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

feat: allow symbols as GraphQLSchema extension fields #3511

Closed
wants to merge 1 commit into from

Conversation

n1ru4l
Copy link
Contributor

@n1ru4l n1ru4l commented Mar 15, 2022

No description provided.

@netlify
Copy link

netlify bot commented Mar 15, 2022

✔️ Deploy Preview for compassionate-pike-271cb3 ready!

🔨 Explore the source changes: 10ba0fa

🔍 Inspect the deploy log: https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/6230936702007b00093bc3fa

😎 Browse the preview: https://deploy-preview-3511--compassionate-pike-271cb3.netlify.app

@n1ru4l
Copy link
Contributor Author

n1ru4l commented Apr 26, 2022

@IvanGoncharov supporting TypeScript 4.3 prevents supporting symbols as index keys


(#3511)

Copy link
Member

@IvanGoncharov IvanGoncharov left a comment

Choose a reason for hiding this comment

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

@n1ru4l Symbols as extensions keys totally make sense.
Please add some basic tests.

As for ObjMap I'm don't understand motivation for this change.
Can you please clarify it?

@IvanGoncharov
Copy link
Member

@IvanGoncharov supporting TypeScript 4.3 prevents supporting symbols as index keys

Thanks for clarification 👍

IvanGoncharov added a commit to IvanGoncharov/graphql-js that referenced this pull request Apr 26, 2022
@saihaj saihaj requested a review from IvanGoncharov May 8, 2022 18:35
@yaacovCR
Copy link
Contributor

Flagging for v17

@yaacovCR yaacovCR mentioned this pull request Sep 27, 2024
17 tasks
JoviDeCroock added a commit that referenced this pull request Oct 14, 2024
Supersedes #3511

This adds support for `Symbol` property keys on the extension property
of AST nodes. Had to tweak `toObjMap` here to support symbol keys by
adding `getOwnPropertySymbols` which is the only way to enumerate over
them.

---------

Co-authored-by: n1ru4l <[email protected]>
@JoviDeCroock
Copy link
Member

Part of v17 now

erikwrede pushed a commit to erikwrede/graphql-js that referenced this pull request Oct 17, 2024
Supersedes graphql/graphql-js#3511

This adds support for `Symbol` property keys on the extension property
of AST nodes. Had to tweak `toObjMap` here to support symbol keys by
adding `getOwnPropertySymbols` which is the only way to enumerate over
them.

---------

Co-authored-by: n1ru4l <[email protected]>
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.

4 participants