Skip to content

Support clientAwareness being undefined in Apollo Link context#212

Merged
jaydenseric merged 1 commit intojaydenseric:masterfrom
danielrearden:client-awareness
Jul 28, 2020
Merged

Support clientAwareness being undefined in Apollo Link context#212
jaydenseric merged 1 commit intojaydenseric:masterfrom
danielrearden:client-awareness

Conversation

@danielrearden
Copy link
Copy Markdown
Contributor

Just a small tweak that ensures createUploadLink doesn't break if clientAwareness is undefined. clientAwareness will always be defined when using a link through an ApolloClient instance, but may be undefined in other contexts (like if calling execute directly instead), so we shouldn't assume it will always be defined.

@jaydenseric
Copy link
Copy Markdown
Owner

jaydenseric commented Jul 28, 2020

Thanks for the PR! I did have it that way originally, but then the when adding code coverage realized it was never necessary when used with Apollo Client. I did try to research instances where it may be undefined in context, but couldn't find any examples so I removed the default.

Copy link
Copy Markdown
Owner

@jaydenseric jaydenseric left a comment

Choose a reason for hiding this comment

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

Could you please add a test for this, so we can get back to 100% code coverage? Also, we need to add a changelog entry under "Patch".

@danielrearden
Copy link
Copy Markdown
Contributor Author

@jaydenseric All set :)

Copy link
Copy Markdown
Owner

@jaydenseric jaydenseric left a comment

Choose a reason for hiding this comment

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

🙌

@jaydenseric jaydenseric changed the title Add default value for clientAwareness Support clientAwareness being undefined in Apollo Link context Jul 28, 2020
@jaydenseric jaydenseric merged commit 8cb3507 into jaydenseric:master Jul 28, 2020
@jaydenseric
Copy link
Copy Markdown
Owner

This change has been published in v14.1.1 🚀

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.

2 participants