Skip to content

Conversation

krinoid
Copy link
Contributor

@krinoid krinoid commented May 14, 2018

It's possible to set "credentials" option in the GraphQL Playground settings. But it seems that this option is not used when HttpLink is created. This causes problems when user relies on the cookies, e.g. uses cookies for authentication.

Related issues:
#656
#681
#664

Previous PR with the fix, when regular fetch was used:
#470

I've encountered this issue myself, since I'm using cookies for storing session ID. I verified that my API works as expected with GraphiQL (i.e. cookies are being sent). Today was the first time that I've read the Playground's code, so I'm not aware of the conventions etc. I see that there's some refactor going on so tell me how to adapt the code to adhere to the standards.

Also, in the ideal case I think that this PR needs a unit test, since it regressed, but I don't have enough time today.

@CLAassistant
Copy link

CLAassistant commented May 14, 2018

CLA assistant check
All committers have signed the CLA.

@huv1k
Copy link
Collaborator

huv1k commented May 14, 2018

Good catch :) and yea we need a lot of unit tests

Copy link
Contributor

@vincenzo vincenzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!

@krinoid
Copy link
Contributor Author

krinoid commented May 15, 2018

What should I add to this PR so it can be merged?

@huv1k
Copy link
Collaborator

huv1k commented May 16, 2018

@timsuchanek

@timsuchanek
Copy link
Member

Thanks for the PR @krinoid ! And sorry for the delay!

@timsuchanek timsuchanek merged commit ccab601 into graphql:master May 16, 2018
@timsuchanek
Copy link
Member

We will do a release in the next 1-2 days to include this change!

@capaj
Copy link
Contributor

capaj commented May 18, 2018

@timsuchanek how about that new release?

@krinoid krinoid deleted the fix/set-credentials-when-creating-httplink branch May 18, 2018 11:02
RenovZ pushed a commit to RenovZ/graphql-playground that referenced this pull request Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants