Skip to content
This repository has been archived by the owner on Apr 14, 2023. It is now read-only.

apollo-link-http does not respect a GET method override #236

Closed
szdc opened this issue Nov 12, 2017 · 20 comments
Closed

apollo-link-http does not respect a GET method override #236

szdc opened this issue Nov 12, 2017 · 20 comments
Assignees

Comments

@szdc
Copy link

szdc commented Nov 12, 2017

As per the docs, the fetchOptions option allows us to set the method of the request to GET. I'd expect that after doing so, the query would be included as a query string parameter rather than the request body; Chrome throws an error if you try to make a GET request with a body.

Intended outcome:
A query string should be constructed if the request method is GET.

Actual outcome:
The request body is used.

How to reproduce the issue:

client.query({
  context: {
    fetchOptions: {
      method: 'GET',
    },
  },
  query: QUERY,
  variables: { query },
})
@jbaxleyiii
Copy link
Contributor

@szdc yep this definitely doesn't work! Would you be open to adding it? Something like #257 may be a good solution as well to transform the request before its sent?

@jbaxleyiii
Copy link
Contributor

cc @arackaf @outlaw11a @puttyman who all have interest in this! Should be a pretty small change to add in!

@arackaf
Copy link

arackaf commented Nov 29, 2017

I'll add it if nobody else wants to.

@jbaxleyiii
Copy link
Contributor

@szdc @arackaf so turns out this is already possible! I'll push up a test and docs soon showing how it can be done without internal changes

@jbaxleyiii jbaxleyiii self-assigned this Nov 30, 2017
@jbaxleyiii
Copy link
Contributor

@szdc @Outlaw11A @arackaf here is how to do it:

const customFetch = (uri, options) => {
  const { body, ...newOptions } = options;
  const queryString = objectToQuery(JSON.parse(body));
  requestedString = uri + queryString;
  return fetch(requestedString, newOptions);
};
const link = createHttpLink({
  uri: "data",
  fetchOptions: { method: "GET" },
  fetch: customFetch
});

@arackaf
Copy link

arackaf commented Nov 30, 2017

@jbaxleyiii that looks awesome! Just curious from where objectToQuery is coming? Is there a util on npm for that? I imagine it'd be pretty easy to just roll your own if needed - basically map Object.keys onto

escapeUriComponnt(`${k}=${obj[k]}`).join("&");

or something similar

@ghost
Copy link

ghost commented Nov 30, 2017

https://www.apollographql.com/docs/link/links/http.html#Sending-GET-requests-custom-fetching

@arackaf see the comment:

// turn the object into a query string, try object-to-querystring package

https://github.com/Cyberlane/object-to-querystring

@ghost
Copy link

ghost commented Dec 1, 2017

@jbaxleyiii it works! It's amazing!

I just have a problem.

The querystring created:

http://localhost/graphql?operationName=AllPlayersQuery&variables=%5Bobject%20Object%5D&query=query%20All...

contains this: variables=%5Bobject%20Object%5D which is a problem.

How to fix this?

@arackaf
Copy link

arackaf commented Dec 1, 2017

That looks like standard URI serialization - don't think you can avoid it - why's it a problem?

@ghost
Copy link

ghost commented Dec 1, 2017

Standard URI? This: %5Bobject%20Object%5D?

@arackaf
Copy link

arackaf commented Dec 1, 2017

encodeURIComponent('books(title:"Hello World"){Book{title}}')

books(title%3A%22Hello%20World%22)%7BBook%7Btitle%7D%7D

@ghost
Copy link

ghost commented Dec 1, 2017

My query in Apollo is:

query AllPlayers {
  players {
    id
    name
    surname
}

and so I have the problem with [Object object] variables empty with the code posted by @jbaxleyiii.

@ianserlin
Copy link

Here's a barebones react app (created using create-react-app) that fetches via GET and works - see index.js: https://github.com/usefulio/example-graphql-via-get-react-app

@jankaszel
Copy link

jankaszel commented Jan 7, 2018

@johnunclesam obviously variables isn't serialized before encoding. i simply tuned my custom fetch method (obtained from the example repo) by adding a conditional JSON.stringify:

const customFetch = (uri, options) => {
  const {body, ...newOptions} = options;
  const parsedBody = JSON.parse(body);
  const command = omitBy(parsedBody, isEmpty);

  if (command.variables) {
    command.variables = JSON.stringify(command.variables);
  }

  const requestedString = uri + '?' + queryString.stringify(command);

  return fetch(requestedString, newOptions);
};

@ivawzh
Copy link

ivawzh commented Feb 16, 2019

So what is useGETForQueries used for? Why the custom fetch is not builtin when useGETForQueries=true?
How do I differentiate query and mutation? I only want to use GET for query (for DNS cache).

@glasser
Copy link
Member

glasser commented Feb 21, 2019

This is an old issue that predates useGETForQueries, which was added a bit after this was fixed by #510

@jharris4
Copy link

jharris4 commented Apr 4, 2019

umm, useGETForQueries seems to not be working.

Specifically on this line there seems to be a bug:

let {
    uri = '/graphql',
    // use default global fetch if nothing passed in
    fetch: fetcher,
    includeExtensions,
    useGETForQueries,
    ...requestOptions
  } = linkOptions;

should be something like:

let {
    uri = '/graphql',
    // use default global fetch if nothing passed in
    fetch: fetcher,
    includeExtensions,
    fetchOptions,
    ...requestOptions
  } = linkOptions;

let {
  useGETForQueries
} = fetchOptions;

@glasser
Copy link
Member

glasser commented Jul 26, 2019

What makes you say that? How are you calling createHttpLink?

@jharris4
Copy link

My observation above was from stepping through the code with a debugger, but I'd have to do it over again to reproduce.

The code we ended up settling on is this:

import omitBy from 'lodash.omitby';
import isEmpty from 'lodash.isempty';

const graphQLUrl = 'http://the.server.com/graphql';

const customFetch = (uri, options) => {
  const { body, credentials, headers, ...newOptions } = options;
  let fetchUri = uri;
  if (body) {
    const parsedBody = JSON.parse(body);
    const command = omitBy(parsedBody, isEmpty);
    fetchUri = uri + "?" + queryString.stringify(command);
  }
  return fetch(fetchUri, newOptions);
};

const link = createHttpLink({
  uri: graphQLUrl,
  fetchOptions: { method: "GET" },
  fetch: customFetch
});

@glasser
Copy link
Member

glasser commented Jul 27, 2019

I guess my question is, if you were passing useGETForQueries inside fetchOptions instead of directly inside createHttpLink, then that would be the behavior you would observe, but it would also not be how the API works. I'd suggest giving it a shot again!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants