Further fixes on pull #2 #3
Conversation
|
Nice. I'll take a look at it and get back to you soon |
| var results = self.trigger('request', [req]) | ||
|
|
||
| // The 'request' hook may redefine response when returning something | ||
| for (var i = results.length; i--;) { |
There was a problem hiding this comment.
I'm not sure i like this part very much, it feels odd that a return from a trigger should change any execution. I can accept that you can mutate the req object but the return value should not be handled. What are you aiming for with this ?
There was a problem hiding this comment.
I happened to need such a feature in a previous graphql client (called "lokka") that I used to use. I wanted to setup caching of non-mutating requests, but I already do so in my application level. So I personally don't need this feature at the moment, just felt it needs to be there, might come in use some day. Yet the module becomes full-featured, and now can be even extended by external plugins/modules (using hooks). I updated this code a bit
|
Hm, what happend with the error handling part after query result. When a query returns with errors i wanted it to throw to the catch block so the promise chain stops |
|
In my API I'm throwing custom errors that I need to display in the HTML. So I need to get the response as is, regardless of whether it's an error or not. And since GraphQL is so "strong typed" I cannot return |
|
I also removed everything regarding of how errors should be treated (e.g. useful debugging info), since now everything could be done inside of hooks. So I guess it would be better to move such things apart to different modules and attach them using hooks |
|
|
||
| if (lineErrors.length) { | ||
| var errorHighlight = [] | ||
| var req = self.options.request || {} |
There was a problem hiding this comment.
Maybe we should make this to an actual request object Fetch API Request.
There was a problem hiding this comment.
I had to switch back to plain req object since there were many problems with Request. For example, I couldn't pass headers to it when initializing the client because they could only be modified after the Request object is initialized. Also, I couldn't pass "credentials" to it, because of a bug in isomorphic-fetch. There were some other issues like a difference in behaviour between browser and Node, so it is just not the way to go
| @@ -0,0 +1,15 @@ | |||
| MOCHA_TARGET=test/specs.js | |||
There was a problem hiding this comment.
please remove Makefile
use npm scripts
https://www.keithcirkel.co.uk/how-to-use-npm-as-a-build-tool/
No description provided.