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

perf: implement shouldOnCreateNode for all our plugins/benchmarks #27545

Merged
merged 2 commits into from Oct 20, 2020
Merged

Conversation

pvdz
Copy link
Contributor

@pvdz pvdz commented Oct 19, 2020

This is a followup for #27442 which introduced a shouldOnCreateNode API to test whether the async onCreateNode callback ought to be called.

This PR implements this hook for all the plugins in our monorepo and some benchmark sites.

For all plugins, the behavior should remain the same and should be backwards compatible since the check also exists in the onCreateNode hook itself.

If a plugin happens to break, anyways, please try to fix forward rather than revert this whole PR. Thanks :)

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 19, 2020
@pvdz pvdz added topic: performance Related to runtime & build performance topic: scaling builds and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Oct 19, 2020
@pvdz pvdz force-pushed the addSOCN branch 6 times, most recently from 5d41c9c to d497458 Compare October 19, 2020 10:55
@pvdz pvdz marked this pull request as ready for review October 19, 2020 10:55
@pvdz pvdz requested a review from smthomas as a code owner October 19, 2020 10:55
vladar
vladar previously approved these changes Oct 20, 2020
Copy link
Contributor

@vladar vladar left a comment

Choose a reason for hiding this comment

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

Looks good to me. Left one question but nothing blocking this.

benchmarks/source-agilitycms/gatsby-node.js Outdated Show resolved Hide resolved
LekoArts
LekoArts previously approved these changes Oct 20, 2020
Copy link
Contributor

@LekoArts LekoArts left a comment

Choose a reason for hiding this comment

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

Other than the benchmark inconsistencies (mentioned in Slack) LGTM
Small typo but other than that you consistently moved the exports in the same order, same checks in plugins 👍

packages/gatsby-plugin-mdx/gatsby-node.js Outdated Show resolved Hide resolved
@pvdz pvdz dismissed stale reviews from LekoArts and vladar via b7cbef9 October 20, 2020 12:21
@pvdz pvdz merged commit df914d9 into master Oct 20, 2020
@delete-merged-branch delete-merged-branch bot deleted the addSOCN branch October 20, 2020 12:22
@pvdz
Copy link
Contributor Author

pvdz commented Oct 20, 2020

Published as part of one of the following:

 - gatsby-plugin-feed@2.5.17
 - gatsby-plugin-image@0.0.4
 - gatsby-plugin-mdx@1.2.49
 - gatsby-plugin-netlify@2.3.22
 - gatsby-plugin-offline@3.2.35
 - gatsby-plugin-sass@2.3.19
 - gatsby-plugin-typescript@2.4.23
 - gatsby-plugin-utils@0.2.37
 - gatsby-react-router-scroll@3.0.15
 - gatsby-remark-custom-blocks@2.3.14
 - gatsby-remark-images@3.3.37
 - gatsby-source-contentful@3.0.1
 - gatsby-transformer-asciidoc@1.3.15
 - gatsby-transformer-csv@2.3.14
 - gatsby-transformer-documentationjs@4.3.17
 - gatsby-transformer-excel@2.4.16
 - gatsby-transformer-hjson@2.4.14
 - gatsby-transformer-javascript-frontmatter@2.3.15
 - gatsby-transformer-javascript-static-exports@2.4.14
 - gatsby-transformer-json@2.4.15
 - gatsby-transformer-pdf@1.3.15
 - gatsby-transformer-react-docgen@5.2.15
 - gatsby-transformer-remark@2.8.44
 - gatsby-transformer-screenshot@2.3.20
 - gatsby-transformer-sharp@2.5.19
 - gatsby-transformer-toml@2.4.17
 - gatsby-transformer-xml@2.3.15
 - gatsby-transformer-yaml@2.4.15
 - gatsby@2.24.83

pragmaticpat pushed a commit to pragmaticpat/gatsby that referenced this pull request Apr 28, 2022
…tsbyjs#27545)

* perf: implement shouldOnCreateNode for all our plugins

* Drop changes in /benchmarks, fix typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: performance Related to runtime & build performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants