Skip to content
New issue

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

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

Already on GitHub? # to your account

feat(mergeAST): add mergeAST utility #4359

Open
wants to merge 1 commit into
base: 16.x.x
Choose a base branch
from

Conversation

metrue
Copy link

@metrue metrue commented Mar 22, 2025

This PR is adding utility of merge AST by simply migrating it from graphiql

As first step to resolve this issue #1428 (comment)

@metrue metrue requested a review from a team as a code owner March 22, 2025 19:50
@metrue metrue changed the title WIP: feat(mergeAST): add mergeAST util feat(mergeAST): add mergeAST util Mar 23, 2025
@metrue metrue changed the title feat(mergeAST): add mergeAST util feat(mergeAST): add mergeAST utility Mar 23, 2025
Copy link

github-actions bot commented Apr 3, 2025

@github-actions publish-pr-on-npm

@0042834736 The latest changes of this PR are available on NPM as
graphql@16.10.0-canary.pr.4359.9fe5229b2fc30d3e5b07a6b00693fac42b649fdd
Note: no gurantees provided so please use your own discretion.

Also you can depend on latest version built from this PR:
npm install --save graphql@canary-pr-4359

@benjie
Copy link
Member

benjie commented Apr 10, 2025

One thing we should very much keep in mind with mergeAST is that it's a potential DOS vector. Having it in the frontend (i.e. GraphiQL) is one thing, but putting it in GraphQL.js means that we will need to have a lot stronger protection against things that could cause explosion of complexity or even infinite loops. If we could require that the document was already validated (and thus had passed all our checks) before calling into mergeAST then that would give us some confidence, but I imagine that people will want to mergeAST separately and we need to protect them from bad inputs. I'm not sure the solution to this, just wanted to surface the concern.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants