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

don't run debouncedCreatePages on DELETE_NODE #7388

Merged
merged 1 commit into from
Aug 16, 2018

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Aug 16, 2018

Fixes #7300

Problem with running debouncedCreatePages is it assumes we can finish all processing in 100ms which is not always the case. In #7300 it was causing to run createPages before slug was attached, so query inside component would never find actual node because it would try to find node with slug === null

here's my debugy output from before the change:

info changed file at D:\dev\i7300\src\post\tile-layouts.mdx
deleting createMdxNode 32a0c299-deff-5812-894a-6df20be75e2d >>> Mdx
deleting createMdxNode finish 32a0c299-deff-5812-894a-6df20be75e2d >>> Mdx
here's debouncedCreatePages is called --> DELETE_NODE create pages
createMdxNode 32a0c299-deff-5812-894a-6df20be75e2d >>> Mdx
createMdxNode finish 32a0c299-deff-5812-894a-6df20be75e2d >>> Mdx
createPages (slug isn't added, we are passing null in context -> query then can't find node with null slug)
onCreateNode 32a0c299-deff-5812-894a-6df20be75e2d >>> Mdx /post/tile-layouts/
createMdxNode ADD_FIELD_TO_NODE finish 32a0c299-deff-5812-894a-6df20be75e2d >>> Mdx
running query for /post/tile-layouts
running query for /
running query for Finished /post/tile-layouts
running query for Finished /

after the change (we run createPages after all processing was done):

info changed file at D:\dev\i7300\src\post\tile-layouts.mdx
deleting createMdxNode 32a0c299-deff-5812-894a-6df20be75e2d >>> Mdx
deleting createMdxNode finish 32a0c299-deff-5812-894a-6df20be75e2d >>> Mdx
createMdxNode 32a0c299-deff-5812-894a-6df20be75e2d >>> Mdx
createMdxNode finish 32a0c299-deff-5812-894a-6df20be75e2d >>> Mdx
onCreateNode 32a0c299-deff-5812-894a-6df20be75e2d >>> Mdx /post/tile-layouts/
createMdxNode ADD_FIELD_TO_NODE finish 32a0c299-deff-5812-894a-6df20be75e2d >>> Mdx
API_RUNNING_QUEUE_EMPTY create pages
createPages
running query for /post/tile-layouts
running query for /
running query for Finished /post/tile-layouts
running query for Finished /

… for API_RUNNING_QUEUE_EMPTY

in some cases it would introduce race - for example running queries in createPages hook before all fields are attached
@pieh pieh self-assigned this Aug 16, 2018
Copy link
Contributor

@m-allanson m-allanson left a comment

Choose a reason for hiding this comment

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

👍

@KyleAMathews
Copy link
Contributor

KyleAMathews commented Aug 16, 2018

ahhhh!!! Yeah!!!!

All that debouncing stuff was added before I smarted up with API_RUNNING_QUEUE_EMPTY. Guess this got left around :-( and caused 100s of issues of "can't find frontmatter" and tens of thousands of forced restarts of gatsby develop sigh.

@geddski
Copy link
Contributor

geddski commented Aug 16, 2018

There is no gif that accurately portrays my excitement.

Thank you thank you thank you! Can't wait to try it.

@m-allanson m-allanson merged commit 5ae42e9 into gatsbyjs:master Aug 16, 2018
@m-allanson
Copy link
Contributor

Published as gatsby@2.0.0-beta.109

@geddski
Copy link
Contributor

geddski commented Aug 16, 2018

Confirmed that fixed it!!!!

@pieh you are a beast. Thank you so much! Oh my gosh Gatsby dev experience is amazing now.

porfirioribeiro pushed a commit to porfirioribeiro/gatsby that referenced this pull request Aug 22, 2018
… for API_RUNNING_QUEUE_EMPTY (gatsbyjs#7388)

in some cases it would introduce race - for example running queries in createPages hook before all fields are attached
@m-allanson m-allanson deleted the dont-run-debounceCreatePages branch February 20, 2019 10:59
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