Change include to not use decoded values for parsing.

ethanresnick / json-api

Turn your node app into a JSON API server (http://jsonapi.org/)

GNU Lesser General Public License v3.0

268 stars 41 forks source link

Change include to not use decoded values for parsing. #186

Closed martinchau closed 5 years ago

martinchau commented 5 years ago

related conversation

parsing of ?include takes from rawQueryString instead
error thrown, if invalid member name for include
update of query parameter util function to match spec, while leaving parameter handling as is
tests testing functionality for above changes

martinchau commented 5 years ago

From the contributing docs it says to run npm run lint have done so, however none of the errors shown are for the changes that I've created. So im not sure if I am missing something in that regard.

ethanresnick commented 5 years ago

Looking great. PLease just fix the one minor lint error in the Node 8 tests (trailing whitespace in Data.ts), and then this is good to merge.

martinchau commented 5 years ago

I believe i have fixed that trailing white space error now.

ethanresnick commented 5 years ago

@martinchau Thanks. It looks like there's another lint error now, but I'll merge this and fix that error in master

ethanresnick commented 5 years ago

Thanks. Only thing is that there's another lint error now :/

In this case, though, it's not a stupid error and is actually catching a bug. See my comment. Please fix that, and then run npm run lint locally to make sure the lint is all passing. Then this'll be good to merge.

martinchau commented 5 years ago

The node8 tests now pass. Apologies for the back and forth error fixing. Whenever I run npm run lint i would get 80+ errors apparently scattered all over the repository, so theres something wrong with my local set up.

ethanresnick commented 5 years ago

Perfect. Merged! Thanks again.