Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check Symbol.for exists in addition to Symbol before using it. #6324

Merged
merged 1 commit into from
May 21, 2020

Conversation

ms
Copy link
Contributor

@ms ms commented May 20, 2020

In some cases, Symbol may be shimmed but not Symbol.for. In those cases,
the existing code would throw on platforms with no native Symbol (IE11).

This change ensures Symbol.for exists before using it, preventing the
issue.

We encountered this issue because of a deferred script partially shimming Symbol. After trying to load Apollo code asynchronously, IE11 would occasionally throw.

Checklist:

  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests

In some cases, Symbol may be shimmed but not Symbol.for. In those cases,
the existing code would throw on platforms with no native Symbol (IE11).

This change ensures `Symbol.for` exists before using it, preventing the
issue.
Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

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

Thanks @ms!

@benjamn benjamn merged commit b9097cb into apollographql:master May 21, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023
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