Feature: API for providing a path-builder function

[HeyHugo]

For usecases like #67 where you need to handle query string parameters that are optional you can't right now.

I've looked a bit at how bodyBuilder and bodyKey are used to allow custom body handling.

I propose that an optional argument of type function is added to the rest-directive pathBuilder that if set is used to build the path string. The function would take the variables object as input.

We can keep the current path parameter as is. Only if pathBuilder is provided it overrides.

Here is an example using the pathBuilder to handle query strings arg:

const query = gql`
  query FilterItems(
    $priceMin: Int
    $priceMax: Int
    $createPath: any
  ) {
    filterItems(priceMin: $priceMin, priceMax: $priceMax)
      @rest(type: "Item", pathBuilder: $createPath) {
      id
      name
      price
    }
  }
`

function createQueryStringPath(variables) {
  const qs = Object.keys(
    variables,
  ).reduce((acc, key): string => {
    if (variables[key] === null || variables[key] === undefined) {
      return acc;
    }
    if (acc === '') {
      return '?' + key + '=' + encodeURIComponent(variables[key].toString());
    }
    return acc + '&' + key + '=' + encodeURIComponent(variables[key].toString());
  }, '');
  return '/items' + qs;
}

client.query({
  query,
  variables: {
    priceMin: 5,
    priceMax: 20,
    createPath: createQueryStringPath
  }
});

I can try to make a PR if you like the idea.

[jslowack]

I like the idea, but wouldn't it be more appropriate to create a query builder instead of a path builder? In this way we could use existing query builders as a parameter and we keep the path separated from the query string formatting function.

Modified code would look like this:

const query = gql`
  query FilterItems(
    $priceMin: Int
    $priceMax: Int
    $queryBuilder: any
  ) {
    filterItems(priceMin: $priceMin, priceMax: $priceMax)
      @rest(type: "Item", path: "items", queryBuilder: $queryBuilder) {
      id
      name
      price
    }
  }

import queryString from 'query-string';
client.query({
  query,
  variables: {
    priceMin: 5,
    priceMax: 20,
    queryBuilder: queryString.stringify
  }
});

Another (small) remark is that we should exclude our "builder function" out of the incoming variables parameter.

[fbartho]

@jslowack That assumes that the REST API you're working is structured cleanly to take parameters in the Query string. A pathBuilder would be slightly more generic as it could tweak the path or the query string appropriately for your REST API -- and as a fundamental tenet of apollo-link-rest we're assuming you're doing this because you can't necessarily change the backend at this time, and you might not even control the REST API.

[fbartho]

@HeyHugo, remember to use encodeURIComponent in your example code/unit test! (We want to be showing best practices in our examples :) )

[fbartho]

@jslowack Thinking more about what you were saying, would you mind elaborating about "existing query builders"? I don't know what you're referring to, so I don't know what reuse there would be valuable. Thanks!

[HeyHugo]

Good idea @fbartho, thinking bout it maybe we'd want to always run encodeURIComponent on pathWithParams in the library code, or would it be confusing for people having uri-encoding they did not explicitly do themselves?

For the default path arg there is no uri-encoding happening currently.

[HeyHugo]

Ah disregard my previous comment, I was mixing up encodeURIComponent and encodeURI.

If anything we would instead use encodeURI(uri + pathWithParams) but maybe thats not necessary.

[jslowack]

@fbartho for example https://github.com/sindresorhus/query-string. I agree that a pathBuilder is more generic. I personally just don't like the idea of a graphQL query where the path (or endpoint) is generic..

we're assuming you're doing this because you can't necessarily change the backend at this time, and you might not even control the REST API

True! With this in mind, it makes more sense to create a path builder.

[fbartho]

@HeyHugo -- I thought about that, and I think @rest( is generally low level enough that it can't know if it should encode URIs automatically in the general case. Double-encoding would be a bad thing, and I don't think users could protect themselves if we did mangle it, so I'm going to say: treat query-strings as something to document, which is why I made my suggestion about your example calling encodeURIComponent

[fbartho]

Implemented via #70 !