Path builder and query parameters

[pgilad]

Several key points come to mind after having worked with apollo-link-rest for a legacy backend (read: not graphql), also seeing that you guys requested comments on the design of it. I'm not a fan of having both path (backend-uri) and query (search) parameters in the same string. It makes handling more complicated cases too complex. I'm also not a fan of forcing the user to use qs module without his ability to configure it, or use an alternative module (or none at all).

For the following points, I make observations on terms:

• path: the backend-uri, not including search/filter parameters
• query: the search parameters (the ones that come after ?)

The handling of path and query should be:

1. path remains a @rest directive variable. We drop all legacy variations (i.e :id) and support only {} expansions, we also support nested objects expansions off course. The expanded variable is always converted to string (so object will turn in to [object object])
2. The resolved path (without query) is encodeURI by default, configurable from RestLink options as a global behavior, with possible override by @rest directive. If the user turns this off then it's the user responsibility to encode his path and variables correctly.
3. We add a new optional variable to @rest directive, which is named query (or perhaps search). This variable behaves similarly to path with variable expansion - i.e using {}. If the expanded variable is a string it is encodeURIComponent. If the variable is an object it gets stringified using either the default queryStringifiers function, or as a dictionary lookup for a specific queryStringifiers function.

Re: queryStringifiers - it is a dictionary defined at RestLink options. Defining a default function is required if the user has an expanded query variable that is an object (otherwise an error is thrown). Using this strategy allows the user to opt-in to query stringifier, to opt-in for how to stringify the query object and which library to use. Off course we recommend a default function that the user can copy-paste (and install qs on his own, the way he does today because it's a peer-dependency).

Suggested usage:

import qs from 'qs';

const restLink = new RestLink({
  uri: 'https://swapi.co/api',
  queryStringifiers: {
     default(obj): { return qs.stringify(obj); } // this will be used as default if the user doesn't select a specific stringifier
     alt(obj): { return qs.stringify(obj, 
        arrayFormat: 'brackets',
        encodeValuesOnly: true,
        skipNulls: true,
    })
  },
  encodeURIs: true // this is the default as well
});
//...

query postTitle (queryParams: any!) {
  post(id: "kewl id", queryParams: $queryParams) @rest(type: "Post", path: "/posts/{args.id}", search: "id={args.id}&{args.queryParams}", queryStringifier: "alt", encodeURI: false) {
    id
    title
  }
}

We'll need to modify how pathBuilder works, but overall it would be very simple (regex replace).

Off course this change breaks backwards compat, and we'll need to bump major version for this.

Thoughts? Comments?

[zachdixon]

+1 for being able to configure the default qs.stringify somehow, whether it be this method or another. All I needed was to change the arrayFormat and have to pass in a pathBuilder every time I'm passing an array in the query string. Even just a global pathBuilder option for the RestLink would work, but this solution is definitely more robust.

[fbartho]

@pgilad / @zachdixon I'd be happy to merge an API like this one if somebody wants to submit a pull-request! -- Sorry I didn't comment sooner. It was a busy end-of-year!

[okomeworld]

@fbartho @zachdixon @pgilad May I create the pull-request if nobody planning?

[Rennzie]

Hey all, I'm not sure if any work has been done with this yet? I'd certainly +1 the ability to use our own serialiser for the search params. I had a problem today where an array was incorrectly serialised by encodeURIComponent and there don't appear to be any non-hacky was to solve the issue.

In the end I intercepted the variables from teh useQuery and serialised them before passing them as a param to the query itself. Not ideal and would be great to have control over how the serialisation works.

[fbartho]

Checking back in -- if people want to submit a PR it's up for grabs, if no progress happens next time I do an Issue sweep, then I'll close this ticket!