Skip to content
This repository has been archived by the owner on Jul 25, 2024. It is now read-only.

Append graphql to the fetcher URL in template #33

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

asiandrummer
Copy link

This PR changed the fetcher URL to be relative - due to my carelessness this broke GraphiQL example code. I'd like to propose the change that will let you keep the relativeness of the fetcher URL and still have a specific endpoint URL for graphql execution-related requests.

[This PR](graphql/graphiql#146) changed the fetcher URL to be relative - due to my carelessness this broke GraphiQL example code. I'd like to propose the change that will let you keep the relativeness of the fetcher URL and still have a specific endpoint URL for graphql execution-related requests.
@asiandrummer
Copy link
Author

GraphiQL#184 will reflect GraphiQL side of changes.

@aweiker
Copy link
Member

aweiker commented Nov 1, 2016

This will break places where the endpoint does not end with graphql. This used to be hard coded but it was changed in d8fb53d

@asiandrummer
Copy link
Author

It definitely will, and also I understand this change will only act as reflection of the fix I'll make to GraphiQL example code, and is probably not too meaningful if you'd like to keep your graphiql template as-is. The main reason I put up this PR is to communicate that GraphiQL will have this change. I think using GraphiQL without this change (if I understood your code correctly) will still work as expected. Please let me know if that's what you would like to do and I can close this PR (or please feel free to close this if you'd like).

@aweiker
Copy link
Member

aweiker commented Nov 1, 2016

Your interpretation is correct. We use the example in GraphiQL as a template that we update when we take on new updates. We have a strong desire to minimize the amount of code we have to update, so tracking these updates is desirable.

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

Successfully merging this pull request may close these issues.

2 participants