perf(gatsby): Avoid unnecessary type inference during bootstrap #19781
Conversation
|
Nice! |
| }) | ||
| if (typeNames.length > 0) { | ||
| // Give event-loop a break | ||
| setTimeout(processNextType, 0) |
There was a problem hiding this comment.
Is there need to do that (give up control)? Why this over regular processNextType call?
There was a problem hiding this comment.
This whole process can take seconds (for big sites). And since it is synchronous it will block the event loop and even make CLI "freeze" for that time. So I applied it as a recommended best practice. Do you afraid of some race conditions / conflicts with redux actions in between?
There was a problem hiding this comment.
That was my assumption that's the case, because this is called during bootstrap only so we are (more or less) in control what's being executed, so I didn't see "functional" reason to do that (and letting cli do the "spinner tick" periodically makes sense)
vladar
added
the
bot: merge on green
|
Published in |
|
Seems to work, nice! :D #19512 (comment) |
Description
This PR should fix performance regression introduced in schema rebuilding PR (#19092). The regression only occurred for sites which created, deleted and then re-created nodes during a bootstrap (a typical example is
createPage/deletePagepattern used by some plugins, i.e.gatsby-plugin-intl).With this pattern, you could have 10 nodes by the end of the bootstrap preceded by 30
createNode/deleteNode/createNodeoperations. So before this PR incremental type inference would have performed 30 node traversals.This PR defers initial inference until the first schema build, which means 10 node traversals for the example mentioned above. After the first schema build inference continues incrementally.
Issues with plugins
Schema rebuilding PR also revealed problems in several plugins which mutate data after
createNodecall. When they do so - mutated fields are never registered in the metadata and never added to GraphQL schema.At the moment following plugins seem to be affected:
gatsby-remark-relative-images(see After npm update, suddenly getting GRAPHQL ERROR - "featuredimage" must not have a selection since type "String" has no subfields #19748)gatsby-source-airtable(already fixed)gatsby-plugin-remote-images(see Broken with Gatsby 2.18.0 graysonhicks/gatsby-plugin-remote-images#28)gatsby-source-contentful(see ContentfulAssets localFile node is missing in the graphql query result with gatsby v2.18.0+ #19716)But there may be others. This PR will likely hide those issues back because first inference occurs only on schema build when all mutations are already applied. As a result, those plugins will keep working as they did before 2.18.0. But the issue still exists and must be addressed in every affected plugin. Otherwise, things like future incremental builds simply won't work.