search

graphql / graphql-js

A reference implementation of GraphQL for JavaScript
http://graphql.org/graphql-js/
MIT License
20.04k stars 2.02k forks source link

incremental: lazily create DeferredFragments #4153

Open yaacovCR opened 2 months ago

yaacovCR commented 2 months ago

depends on #4152

goal:

avoid creating or passing around the deferMap

methodology:

note:

for performance, we now save the depth of the path on the path elements themselves. this could always be replaced by a call to pathToArray as path.depth === pathToArray(path).length)

netlify[bot] commented 2 months ago

Deploy Preview for compassionate-pike-271cb3 ready!

Name Link
Latest commit 60f92c0a46a7ac37b62feeb435801169a2fceeaa
Latest deploy log https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/66c624c4f84c460008fa4075
Deploy Preview https://deploy-preview-4153--compassionate-pike-271cb3.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

github-actions[bot] commented 2 months ago

Hi @yaacovCR, I'm @github-actions bot happy to help you with this PR 👋

Supported commands Please post this commands in separate comments and only one per comment: * `@github-actions run-benchmark` - Run benchmark comparing base and merge commits for this PR * `@github-actions publish-pr-on-npm` - Build package from this PR and publish it on NPM
yaacovCR commented 2 months ago

Comparing to #4150, we now annotate the DeferUsage with the actual depth in the response rather than the field depth in the operation. This is simpler and results in no change No Actual Change to the path interface (although technically we still change it by annotating it with the depth, this could always be derived later).

yaacovCR commented 2 months ago

tentative spec changes to go along with this draft implementation PR are at https://github.com/yaacovCR/graphql-spec/pull/3

yaacovCR commented 2 months ago

once we no longer create the deferMap, collectFields also does not need to return the newDeferUsages.

we do still return a boolean if defers were encountered to update the context so that buildExecutionPlan will be called; this allows us to still skip buildExecutionPlan in the wholly non-incremental case, somewhat narrowing the scope of that optimization

yaacovCR commented 1 month ago

@github-actions publish-pr-on-npm

github-actions[bot] commented 1 month ago

@github-actions publish-pr-on-npm

@yaacovCR Something went wrong, please check log.

github-actions[bot] commented 1 month ago

@github-actions publish-pr-on-npm

@yaacovCR The latest changes of this PR are available on NPM as graphql@17.0.0-alpha.5.canary.pr.4153.4ff43175428332c954563050819fcb612e19ca41 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-4153

yaacovCR commented 1 month ago

@github-actions publish-pr-on-npm

github-actions[bot] commented 1 month ago

@github-actions publish-pr-on-npm

@yaacovCR The latest changes of this PR are available on NPM as graphql@17.0.0-alpha.5.canary.pr.4153.d5c18bebb93273daf40fce67daa1babc430a2ce2 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-4153