Skip to content

Commit

Permalink
perf(gatsby): test sync before calling onCreateNode (#27442)
Browse files Browse the repository at this point in the history
* perf(gatsby): test sync before calling onCreateNode

* Dont wrap node in object, fix tests

* This snapshot does not fail locally but does on CI

* Must check if value exists too, not just plugin

* Disarm the footgun

* Apply feedback

* Update version and related snapshot

* Add `pluginOptions` as second callback param. Add note about api.

* shouldOnCreateNode -> unstable_shouldOnCreateNode

* it would help if these snapshots were the same locally.

* Also update the first arg to be `{node}` instead of `node`
  • Loading branch information
pvdz committed Oct 16, 2020
1 parent f227e85 commit 6400383
Show file tree
Hide file tree
Showing 9 changed files with 83 additions and 10 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const Promise = require(`bluebird`)
const _ = require(`lodash`)
const onCreateNode = require(`../on-node-create`)
const { onCreateNode } = require(`../on-node-create`)
const { graphql } = require(`gatsby/graphql`)

const { createContentDigest } = require(`gatsby-core-utils`)
Expand Down
7 changes: 6 additions & 1 deletion packages/gatsby-transformer-remark/src/gatsby-node.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
const {
onCreateNode,
unstable_shouldOnCreateNode,
} = require(`./on-node-create`)
exports.onCreateNode = onCreateNode
exports.unstable_shouldOnCreateNode = unstable_shouldOnCreateNode
exports.createSchemaCustomization = require(`./create-schema-customization`)
exports.onCreateNode = require(`./on-node-create`)
exports.setFieldsOnGraphQLNodeType = require(`./extend-node-type`)

if (process.env.GATSBY_EXPERIMENTAL_PLUGIN_OPTION_VALIDATION) {
Expand Down
16 changes: 11 additions & 5 deletions packages/gatsby-transformer-remark/src/on-node-create.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
const grayMatter = require(`gray-matter`)
const _ = require(`lodash`)

module.exports = async function onCreateNode(
function unstable_shouldOnCreateNode({ node }) {
return (
node.internal.mediaType === `text/markdown` ||
node.internal.mediaType === `text/x-markdown`
)
}

module.exports.onCreateNode = async function onCreateNode(
{
node,
loadNodeContent,
Expand All @@ -15,10 +22,7 @@ module.exports = async function onCreateNode(
const { createNode, createParentChildLink } = actions

// We only care about markdown content.
if (
node.internal.mediaType !== `text/markdown` &&
node.internal.mediaType !== `text/x-markdown`
) {
if (!unstable_shouldOnCreateNode({ node })) {
return {}
}

Expand Down Expand Up @@ -76,3 +80,5 @@ module.exports = async function onCreateNode(
return {} // eslint
}
}

module.exports.unstable_shouldOnCreateNode = unstable_shouldOnCreateNode
7 changes: 6 additions & 1 deletion packages/gatsby-transformer-sharp/src/gatsby-node.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
exports.onCreateNode = require(`./on-node-create`)
const {
onCreateNode,
unstable_shouldOnCreateNode,
} = require(`./on-node-create`)
exports.onCreateNode = onCreateNode
exports.unstable_shouldOnCreateNode = unstable_shouldOnCreateNode
exports.createSchemaCustomization = require(`./customize-schema`)
exports.createResolvers = require(`./create-resolvers`)
14 changes: 12 additions & 2 deletions packages/gatsby-transformer-sharp/src/on-node-create.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
const { supportedExtensions } = require(`./supported-extensions`)

module.exports = async function onCreateNode({ node, actions, createNodeId }) {
function unstable_shouldOnCreateNode({ node }) {
return !!supportedExtensions[node.extension]
}

module.exports.onCreateNode = async function onCreateNode({
node,
actions,
createNodeId,
}) {
const { createNode, createParentChildLink } = actions

if (!supportedExtensions[node.extension]) {
if (!unstable_shouldOnCreateNode({ node })) {
return
}

Expand All @@ -22,3 +30,5 @@ module.exports = async function onCreateNode({ node, actions, createNodeId }) {

return
}

module.exports.unstable_shouldOnCreateNode = unstable_shouldOnCreateNode
14 changes: 14 additions & 0 deletions packages/gatsby/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,20 @@ export interface GatsbyNode {
callback?: PluginCallback
): void

/**
* Called before scheduling a `onCreateNode` callback for a plugin. If it returns falsy
* then Gatsby will not schedule the `onCreateNode` callback for this node for this plugin.
* Note: this API does not receive the regular `api` that other callbacks get as first arg.
*
* @gatsbyVersion 2.24.80
* @example
* exports.unstable_shouldOnCreateNode = ({node}, pluginOptions) => node.internal.type === 'Image'
*/
unstable_shouldOnCreateNode?<TNode extends object = {}>(
args: { node: TNode },
options?: PluginOptions
): boolean

/**
* Called when a new page is created. This extension API is useful
* for programmatically manipulating pages created by other plugins e.g.
Expand Down
3 changes: 3 additions & 0 deletions packages/gatsby/scripts/__tests__/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ it("generates the expected api output", done => {
"resolvableExtensions": Object {},
"setFieldsOnGraphQLNodeType": Object {},
"sourceNodes": Object {},
"unstable_shouldOnCreateNode": Object {
"version": "2.24.80",
},
},
"ssr": Object {
"onPreRenderHTML": Object {},
Expand Down
11 changes: 11 additions & 0 deletions packages/gatsby/src/utils/api-node-docs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,17 @@ export const sourceNodes = true
*/
export const onCreateNode = true

/**
* Called before scheduling a `onCreateNode` callback for a plugin. If it returns falsy
* then Gatsby will not schedule the `onCreateNode` callback for this node for this plugin.
* Note: this API does not receive the regular `api` that other callbacks get as first arg.
*
* @gatsbyVersion 2.24.80
* @example
* exports.unstable_shouldOnCreateNode = ({node}, pluginOptions) => node.internal.type === 'Image'
*/
export const unstable_shouldOnCreateNode = true

/**
* Called when a new page is created. This extension API is useful
* for programmatically manipulating pages created by other plugins e.g.
Expand Down
19 changes: 19 additions & 0 deletions packages/gatsby/src/utils/api-runner-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -527,9 +527,28 @@ module.exports = async (api, args = {}, { pluginSource, activity } = {}) =>
return null
}

let gatsbyNode = pluginNodeCache.get(plugin.name)
if (!gatsbyNode) {
gatsbyNode = require(`${plugin.resolve}/gatsby-node`)
pluginNodeCache.set(plugin.name, gatsbyNode)
}

const pluginName =
plugin.name === `default-site-plugin` ? `gatsby-node.js` : plugin.name

// TODO: rethink createNode API to handle this better
if (
api === `onCreateNode` &&
gatsbyNode?.unstable_shouldOnCreateNode && // Don't bail if this api is not exported
!gatsbyNode.unstable_shouldOnCreateNode(
{ node: args.node },
plugin.pluginOptions
)
) {
// Do not try to schedule an async event for this node for this plugin
return null
}

return new Promise(resolve => {
resolve(runAPI(plugin, api, { ...args, parentSpan: apiSpan }, activity))
}).catch(err => {
Expand Down

0 comments on commit 6400383

Please sign in to comment.