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(gatsby-plugin-mdx): drop another babel step during sourcing #25757

Merged
merged 1 commit into from Jul 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 8 additions & 9 deletions packages/gatsby-plugin-mdx/gatsby/on-create-node.js
Expand Up @@ -6,7 +6,6 @@ const { createContentDigest } = require(`gatsby-core-utils`)
const defaultOptions = require(`../utils/default-options`)
const createMDXNode = require(`../utils/create-mdx-node`)
const { MDX_SCOPES_LOCATION } = require(`../constants`)
const { findImports } = require(`../utils/gen-mdx`)

const contentDigest = val => createContentDigest(val)

Expand All @@ -18,6 +17,7 @@ module.exports = async (
createNodeId,
getNode,
getNodes,
getNodesByType,
reporter,
cache,
pathPrefix,
Expand Down Expand Up @@ -46,20 +46,14 @@ module.exports = async (

const content = await loadNodeContent(node)

const mdxNode = await createMDXNode({
const { mdxNode, scopeIdentifiers, scopeImports } = await createMDXNode({
id: createNodeId(`${node.id} >>> Mdx`),
node,
content,
})

createNode(mdxNode)
createParentChildLink({ parent: node, child: mdxNode })

// write scope files into .cache for later consumption
const { scopeImports, scopeIdentifiers } = await findImports({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This call is folded up inside createMDXNode, where it replaces a babel parse step because it gives us the same result (that's the real change in this PR).

node: mdxNode,
getNode,
getNodes,
getNodesByType,
reporter,
cache,
pathPrefix,
Expand All @@ -69,6 +63,11 @@ module.exports = async (
createNodeId,
...helpers,
})

createNode(mdxNode)
createParentChildLink({ parent: node, child: mdxNode })

// write scope files into .cache for later consumption
await cacheScope({
cache,
scopeIdentifiers,
Expand Down
10 changes: 8 additions & 2 deletions packages/gatsby-plugin-mdx/loaders/mdx-loader.js
Expand Up @@ -93,6 +93,7 @@ module.exports = async function (content) {
const {
getNode: rawGetNode,
getNodes,
getNodesByType,
reporter,
cache,
pathPrefix,
Expand Down Expand Up @@ -120,11 +121,15 @@ module.exports = async function (content) {

let mdxNode
try {
mdxNode = await createMDXNode({
// This node attempts to break the chicken-egg problem, where parsing mdx
// allows for custom plugins, which can receive a mdx node
;({ mdxNode } = await createMDXNode({
pvdz marked this conversation as resolved.
Show resolved Hide resolved
id: `fakeNodeIdMDXFileABugIfYouSeeThis`,
node: fileNode,
content,
})
options,
getNodesByType,
}))
} catch (e) {
return callback(e)
}
Expand Down Expand Up @@ -168,6 +173,7 @@ ${contentWithoutFrontmatter}`
node: { ...mdxNode, rawBody: code },
getNode,
getNodes,
getNodesByType,
reporter,
cache,
pathPrefix,
Expand Down
4 changes: 2 additions & 2 deletions packages/gatsby-plugin-mdx/loaders/mdx-loader.test.js
Expand Up @@ -16,9 +16,9 @@ array: [1,2,3]
)`,
namedExports: `export const meta = {author: "chris"}`,
body: `# Some title

a bit of a paragraph

some content`,
}

Expand Down
49 changes: 24 additions & 25 deletions packages/gatsby-plugin-mdx/utils/create-mdx-node.js
@@ -1,20 +1,19 @@
const { createContentDigest } = require(`gatsby-core-utils`)

const mdx = require(`../utils/mdx`)
const extractExports = require(`../utils/extract-exports`)

module.exports = async ({ id, node, content }) => {
let code
try {
code = await mdx(content)
} catch (e) {
// add the path of the file to simplify debugging error messages
e.message += `${node.absolutePath}: ${e.message}`
throw e
}

// extract all the exports
const { frontmatter, ...nodeExports } = extractExports(code)
const { findImportsExports } = require(`../utils/gen-mdx`)

async function createMdxNode({ id, node, content, ...helpers }) {
const {
frontmatter,
scopeImports,
scopeExports,
scopeIdentifiers,
} = await findImportsExports({
mdxNode: node,
rawInput: content,
absolutePath: node.absolutePath,
...helpers,
})

const mdxNode = {
id,
Expand All @@ -24,23 +23,23 @@ module.exports = async ({ id, node, content }) => {
content: content,
type: `Mdx`,
},
excerpt: frontmatter.excerpt,
exports: scopeExports,
rawBody: content,
frontmatter: {
title: ``, // always include a title
...frontmatter,
},
}

mdxNode.frontmatter = {
title: ``, // always include a title
...frontmatter,
}

mdxNode.excerpt = frontmatter.excerpt
mdxNode.exports = nodeExports
mdxNode.rawBody = content

Comment on lines 33 to -37
Copy link
Contributor Author

@pvdz pvdz Jul 16, 2020

Choose a reason for hiding this comment

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

This part is only refactoring, trying to create the object in one go rather than add properties to it later.

// Add path to the markdown file path
if (node.internal.type === `File`) {
mdxNode.fileAbsolutePath = node.absolutePath
}

mdxNode.internal.contentDigest = createContentDigest(mdxNode)

return mdxNode
return { mdxNode, scopeIdentifiers, scopeImports }
}

module.exports = createMdxNode
38 changes: 24 additions & 14 deletions packages/gatsby-plugin-mdx/utils/gen-mdx.js
Expand Up @@ -47,6 +47,7 @@ async function genMDX(
options,
getNode,
getNodes,
getNodesByType,
reporter,
cache,
pathPrefix,
Expand Down Expand Up @@ -114,6 +115,7 @@ export const _frontmatter = ${JSON.stringify(data)}`
// files,
getNode,
getNodes,
getNodesByType,
reporter,
cache,
pathPrefix,
Expand Down Expand Up @@ -194,8 +196,10 @@ ${code}`
module.exports = genMDX // Legacy API, drop in v3 in favor of named export
module.exports.genMDX = genMDX

async function findImports({
node,
async function findImportsExports({
mdxNode,
rawInput,
absolutePath = null,
options,
getNode,
getNodes,
Expand All @@ -205,12 +209,12 @@ async function findImports({
pathPrefix,
...helpers
}) {
const { content } = grayMatter(node.rawBody)
const { data: frontmatter, content } = grayMatter(rawInput)

const gatsbyRemarkPluginsAsremarkPlugins = await getSourcePluginsAsRemarkPlugins(
{
gatsbyRemarkPlugins: options.gatsbyRemarkPlugins,
mdxNode: node,
mdxNode,
getNode,
getNodes,
getNodesByType,
Expand All @@ -226,18 +230,19 @@ async function findImports({
)

const compilerOptions = {
filepath: node.fileAbsolutePath,
filepath: absolutePath,
...options,
remarkPlugins: [
...options.remarkPlugins,
...gatsbyRemarkPluginsAsremarkPlugins,
],
}

const compiler = mdx.createCompiler(compilerOptions)

const fileOpts = { contents: content }
if (node.fileAbsolutePath) {
fileOpts.path = node.fileAbsolutePath
if (absolutePath) {
fileOpts.path = absolutePath
}

const mdast = await compiler.parse(fileOpts)
Expand All @@ -246,16 +251,19 @@ async function findImports({
// we don't need to dedupe the symbols here.
const identifiers = []
const imports = []
const exports = []

mdast.children.forEach(node => {
if (node.type !== `import`) return

const importCode = node.value
if (node.type === `import`) {
const importCode = node.value

imports.push(importCode)
imports.push(importCode)

const bindings = parseImportBindings(importCode)
identifiers.push(...bindings)
const bindings = parseImportBindings(importCode)
identifiers.push(...bindings)
} else if (node.type === `export`) {
exports.push(node.value)
}
})

if (!identifiers.includes(`React`)) {
Expand All @@ -264,9 +272,11 @@ async function findImports({
}

return {
frontmatter,
scopeImports: imports,
scopeExports: exports,
scopeIdentifiers: identifiers,
}
}

module.exports.findImports = findImports
module.exports.findImportsExports = findImportsExports