Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit baa0804

Browse files
GatsbyJS Botpieh
GatsbyJS Bot
andauthoredMay 11, 2021
fix(gatsby-plugin-mdx): enable hmr when importing mdx (#31288) (#31370)
* save MDXContent to different file * tmp: skip mdx loader unit tests * test(e2e-mdx): upgrade cypress, setup running dev * test(e2e-mdx): add hmr test case * only apply hmr workaround to develop stage * don't save mdx component to fs, use webpack tricks with query params * wait for hmr in mdx/develop * drop passthrough fs location * revert unneeded change * more reverts * revert devtool debugging change * adjust unit tests * add more e2e test - editing prop in markdown, editing component imported by mdx (cherry picked from commit c8db78f) Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com>

File tree

16 files changed

+1057
-127
lines changed

16 files changed

+1057
-127
lines changed
 

‎e2e-tests/mdx/cypress-dev.json

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"baseUrl": "http://localhost:8000",
3+
"env": {
4+
"GATSBY_COMMAND": "develop"
5+
}
6+
}

‎e2e-tests/mdx/cypress.json

+4-1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
11
{
2-
"baseUrl": "http://localhost:9000"
2+
"baseUrl": "http://localhost:9000",
3+
"env": {
4+
"GATSBY_COMMAND": "build"
5+
}
36
}
+71
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
if (Cypress.env("GATSBY_COMMAND") === `develop`) {
2+
before(() => {
3+
cy.exec(`npm run reset`)
4+
})
5+
6+
after(() => {
7+
cy.exec(`npm run reset`)
8+
})
9+
10+
it(`Can hot-reload markdown content`, () => {
11+
cy.visit(`/hmr`, {
12+
onBeforeLoad: win => {
13+
cy.spy(win.console, "log").as(`hmrConsoleLog`)
14+
},
15+
}).waitForRouteChange()
16+
cy.get(`h2`).invoke(`text`).should(`eq`, `Lorem`)
17+
18+
cy.exec(
19+
`npm run update -- --file src/pages/hmr.mdx --exact --replacements "Lorem:Ipsum"`
20+
)
21+
22+
cy.get(`@hmrConsoleLog`).should(`be.calledWithMatch`, `App is up to date`)
23+
cy.wait(1000)
24+
25+
cy.get(`h2`).invoke(`text`).should(`eq`, `Ipsum`)
26+
})
27+
28+
it(`Can hot-reload react content (i.e. change prop in mdx content)`, () => {
29+
cy.visit(`/hmr`, {
30+
onBeforeLoad: win => {
31+
cy.spy(win.console, "log").as(`hmrConsoleLog`)
32+
},
33+
}).waitForRouteChange()
34+
cy.get(`[data-testid="test-prop-edit"]`)
35+
.invoke(`text`)
36+
.should(`eq`, `prop-before`)
37+
38+
cy.exec(
39+
`npm run update -- --file src/pages/hmr.mdx --exact --replacements "prop-before:prop-after"`
40+
)
41+
42+
cy.get(`@hmrConsoleLog`).should(`be.calledWithMatch`, `App is up to date`)
43+
cy.wait(1000)
44+
45+
cy.get(`[data-testid="test-prop-edit"]`)
46+
.invoke(`text`)
47+
.should(`eq`, `prop-after`)
48+
})
49+
50+
it(`Can hot-reload imported js components`, () => {
51+
cy.visit(`/hmr`, {
52+
onBeforeLoad: win => {
53+
cy.spy(win.console, "log").as(`hmrConsoleLog`)
54+
},
55+
}).waitForRouteChange()
56+
cy.get(`[data-testid="test-imported-edit"]`)
57+
.invoke(`text`)
58+
.should(`eq`, `component-before`)
59+
60+
cy.exec(
61+
`npm run update -- --file src/components/hmr-component-edit.js --exact --replacements "component-before:component-after"`
62+
)
63+
64+
cy.get(`@hmrConsoleLog`).should(`be.calledWithMatch`, `App is up to date`)
65+
cy.wait(1000)
66+
67+
cy.get(`[data-testid="test-imported-edit"]`)
68+
.invoke(`text`)
69+
.should(`eq`, `component-after`)
70+
})
71+
}

‎e2e-tests/mdx/package.json

+14-7
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
"dependencies": {
66
"@mdx-js/mdx": "^1.6.6",
77
"@mdx-js/react": "^1.6.6",
8-
"cypress": "^3.1.0",
8+
"cypress": "^7.2.0",
99
"fs-extra": "^8.1.0",
1010
"gatsby": "^3.0.0",
1111
"gatsby-plugin-mdx": "^2.0.0",
@@ -19,14 +19,21 @@
1919
],
2020
"license": "MIT",
2121
"scripts": {
22-
"build": "gatsby build",
23-
"develop": "gatsby develop",
22+
"build": "cross-env CYPRESS_SUPPORT=y gatsby build",
23+
"develop": "cross-env CYPRESS_SUPPORT=y gatsby develop",
2424
"format": "prettier --write '**/*.js'",
25-
"test": "cross-env CYPRESS_SUPPORT=y npm run build && npm run start-server-and-test",
26-
"start-server-and-test": "start-server-and-test serve http://localhost:9000 cy:run",
25+
"test:build": "cross-env CYPRESS_SUPPORT=y npm run build && npm run start-server-and-test:build",
26+
"test:develop": "npm run start-server-and-test:develop || (npm run reset && exit 1)",
27+
"test": "npm run test:build && npm run test:develop",
28+
"start-server-and-test:develop": "start-server-and-test develop http://localhost:8000 cy:run:develop",
29+
"start-server-and-test:build": "start-server-and-test serve http://localhost:9000 cy:run:build",
2730
"serve": "gatsby serve",
28-
"cy:open": "cypress open",
29-
"cy:run": "node ../../scripts/cypress-run-with-conditional-record-flag.js --browser chrome"
31+
"cy:open:develop": "cypress open --config-file cypress-dev.json",
32+
"cy:open:build": "cypress open",
33+
"cy:run:build": "node ../../scripts/cypress-run-with-conditional-record-flag.js --browser chrome --group production",
34+
"cy:run:develop": "node ../../scripts/cypress-run-with-conditional-record-flag.js --browser chrome --config-file cypress-dev.json --group development",
35+
"reset": "node scripts/reset.js",
36+
"update": "node scripts/update.js"
3037
},
3138
"devDependencies": {
3239
"cross-env": "^5.2.0",

‎e2e-tests/mdx/scripts/history.js

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
const fs = require(`fs-extra`)
2+
3+
const HISTORY_FILE = `__history__.json`
4+
5+
exports.__HISTORY_FILE__ = HISTORY_FILE
6+
7+
exports.getHistory = async (file = HISTORY_FILE) => {
8+
try {
9+
const contents = await fs
10+
.readFile(file, `utf8`)
11+
.then(contents => JSON.parse(contents))
12+
13+
return new Map(contents)
14+
} catch (e) {
15+
return new Map()
16+
}
17+
}
18+
19+
exports.writeHistory = async (contents, file = HISTORY_FILE) => {
20+
try {
21+
await fs.writeFile(file, JSON.stringify([...contents]), `utf8`)
22+
} catch (e) {
23+
console.error(e)
24+
}
25+
}

‎e2e-tests/mdx/scripts/reset.js

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
const fs = require(`fs-extra`)
2+
const path = require(`path`)
3+
4+
const { __HISTORY_FILE__, getHistory } = require(`./history`)
5+
6+
async function reset() {
7+
const history = await getHistory()
8+
9+
await Promise.all(
10+
Array.from(history).map(([filePath, value]) => {
11+
if (typeof value === `string`) {
12+
return fs.writeFile(path.resolve(filePath), value, `utf8`)
13+
}
14+
return fs.remove(path.resolve(filePath))
15+
})
16+
)
17+
18+
await fs.remove(__HISTORY_FILE__)
19+
}
20+
21+
reset()

‎e2e-tests/mdx/scripts/update.js

+103
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
const fs = require(`fs-extra`)
2+
const path = require(`path`)
3+
const yargs = require(`yargs`)
4+
5+
const { getHistory, writeHistory } = require(`./history`)
6+
7+
const args = yargs
8+
.option(`file`, {
9+
demand: true,
10+
type: `string`,
11+
})
12+
.option(`replacements`, {
13+
default: [],
14+
type: `array`,
15+
})
16+
.option(`exact`, {
17+
default: false,
18+
type: `boolean`,
19+
})
20+
.option(`delete`, {
21+
default: false,
22+
type: `boolean`,
23+
})
24+
.option(`fileContent`, {
25+
default: JSON.stringify(
26+
`
27+
import * as React from 'react';
28+
29+
import Layout from '../components/layout';
30+
31+
export default function SomeComponent() {
32+
return (
33+
<Layout>
34+
<h1 data-testid="message">Hello %REPLACEMENT%</h1>
35+
</Layout>
36+
)
37+
}
38+
`
39+
).trim(),
40+
type: `string`,
41+
})
42+
.option(`fileSource`, {
43+
type: `string`,
44+
})
45+
.option(`restore`, {
46+
default: false,
47+
type: `boolean`,
48+
}).argv
49+
50+
async function update() {
51+
const history = await getHistory()
52+
53+
const { file: fileArg, replacements, restore } = args
54+
const filePath = path.resolve(fileArg)
55+
if (restore) {
56+
const original = history.get(filePath)
57+
if (original) {
58+
await fs.writeFile(filePath, original, `utf-8`)
59+
} else if (original === false) {
60+
await fs.remove(filePath)
61+
} else {
62+
console.log(`Didn't make changes to "${fileArg}". Nothing to restore.`)
63+
}
64+
history.delete(filePath)
65+
return
66+
}
67+
let exists = true
68+
if (!fs.existsSync(filePath)) {
69+
exists = false
70+
let fileContent
71+
if (args.fileSource) {
72+
fileContent = await fs.readFile(args.fileSource, `utf8`)
73+
} else if (args.fileContent) {
74+
fileContent = JSON.parse(args.fileContent).replace(/\+n/g, `\n`)
75+
}
76+
await fs.writeFile(filePath, fileContent, `utf8`)
77+
}
78+
const file = await fs.readFile(filePath, `utf8`)
79+
80+
if (!history.has(filePath)) {
81+
history.set(filePath, exists ? file : false)
82+
}
83+
84+
if (args.delete) {
85+
if (exists) {
86+
await fs.remove(filePath)
87+
}
88+
} else {
89+
const contents = replacements.reduce((replaced, pair) => {
90+
const [key, value] = pair.split(`:`)
91+
return replaced.replace(
92+
args.exact ? key : new RegExp(`%${key}%`, `g`),
93+
value
94+
)
95+
}, file)
96+
97+
await fs.writeFile(filePath, contents, `utf8`)
98+
}
99+
100+
await writeHistory(history)
101+
}
102+
103+
update()
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import React from "react"
2+
3+
const HMRImportEditComponent = () => (
4+
<div data-testid="test-imported-edit">component-before</div>
5+
)
6+
7+
export default HMRImportEditComponent
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import React from "react"
2+
3+
const HMRPropEditComponent = ({ test }) => (
4+
<div data-testid="test-prop-edit">{test}</div>
5+
)
6+
7+
export default HMRPropEditComponent

‎e2e-tests/mdx/src/pages/hmr.mdx

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import HMRImportEditComponent from "../components/hmr-component-edit"
2+
import HMRPropEditComponent from "../components/hmr-prop-edit"
3+
4+
## Lorem
5+
6+
<HMRImportEditComponent />
7+
8+
<HMRPropEditComponent test="prop-before" />

‎packages/gatsby-plugin-mdx/gatsby/create-webpack-config.js

+1
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ module.exports = (
7676
options: {
7777
cache: cache,
7878
actions: actions,
79+
isolateMDXComponent: stage === `develop`,
7980
...other,
8081
pluginOptions: options,
8182
},

‎packages/gatsby-plugin-mdx/loaders/__snapshots__/mdx-loader.test.js.snap

+669-53
Large diffs are not rendered by default.

‎packages/gatsby-plugin-mdx/loaders/mdx-loader.js

+23
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
const _ = require(`lodash`)
22
const { getOptions } = require(`loader-utils`)
33
const grayMatter = require(`gray-matter`)
4+
const path = require(`path`)
45
const unified = require(`unified`)
56
const babel = require(`@babel/core`)
67
const { createRequireFromPath, slash } = require(`gatsby-core-utils`)
@@ -95,7 +96,9 @@ const hasDefaultExport = (str, options) => {
9596

9697
module.exports = async function mdxLoader(content) {
9798
const callback = this.async()
99+
98100
const {
101+
isolateMDXComponent,
99102
getNode: rawGetNode,
100103
getNodes,
101104
getNodesByType,
@@ -106,6 +109,25 @@ module.exports = async function mdxLoader(content) {
106109
...helpers
107110
} = getOptions(this)
108111

112+
const resourceQuery = this.resourceQuery || ``
113+
if (isolateMDXComponent && !resourceQuery.includes(`type=component`)) {
114+
const { data } = grayMatter(content)
115+
116+
const requestPath = `/${path.relative(
117+
this.rootContext,
118+
this.resourcePath
119+
)}?type=component`
120+
121+
return callback(
122+
null,
123+
`import MDXContent from "${requestPath}";
124+
export default MDXContent;
125+
export * from "${requestPath}"
126+
127+
export const _frontmatter = ${JSON.stringify(data)};`
128+
)
129+
}
130+
109131
const options = withDefaultOptions(pluginOptions)
110132

111133
let fileNode = getNodes().find(
@@ -214,6 +236,7 @@ ${contentWithoutFrontmatter}`
214236
reporter,
215237
cache,
216238
pathPrefix,
239+
isolateMDXComponent,
217240
})
218241

219242
try {

‎packages/gatsby-plugin-mdx/loaders/mdx-loader.test.js

+52-63
Original file line numberDiff line numberDiff line change
@@ -34,23 +34,27 @@ some content`,
3434
input.namedExports ? code.namedExports : ``,
3535
code.body,
3636
].join(`\n\n`),
37+
isDevelopStage: input.isDevelopStage,
38+
lessBabel: input.lessBabel,
3739
}
3840
}
3941

4042
// generate a table of all possible combinations of genMDXfile input
41-
const fixtures = new BaseN([true, false], 3)
43+
const fixtures = new BaseN([true, false], 5)
4244
.toArray()
43-
.map(([frontmatter, layout, namedExports]) =>
44-
genMDXFile({ frontmatter, layout, namedExports })
45+
.map(([frontmatter, layout, namedExports, isDevelopStage, lessBabel]) =>
46+
genMDXFile({ frontmatter, layout, namedExports, isDevelopStage, lessBabel })
4547
)
46-
.map(({ name, content }) => [
48+
.map(({ name, content, isDevelopStage, lessBabel }) => [
4749
name,
4850
{
4951
internal: { type: `File` },
5052
sourceInstanceName: `webpack-test-fixtures`,
5153
absolutePath: `/fake/${name}`,
5254
},
5355
content,
56+
isDevelopStage,
57+
lessBabel,
5458
])
5559

5660
describe(`mdx-loader`, () => {
@@ -64,71 +68,56 @@ describe(`mdx-loader`, () => {
6468
})
6569
test.each(fixtures)(
6670
`snapshot with %s`,
67-
async (filename, fakeGatsbyNode, content) => {
68-
const loader = mdxLoader.bind({
69-
async() {
70-
return (err, result) => {
71-
expect(err).toBeNull()
72-
expect(result).toMatchSnapshot()
73-
}
74-
},
75-
query: {
76-
getNodes(_type) {
77-
return fixtures.map(([, node]) => node)
78-
},
79-
getNodesByType(_type) {
80-
return fixtures.map(([, node]) => node)
81-
},
82-
pluginOptions: {
83-
lessBabel: false, // default
71+
async (filename, fakeGatsbyNode, content, isDevelopStage, lessBabel) => {
72+
let err
73+
let result
74+
75+
const createLoader = (opts = {}) =>
76+
mdxLoader.bind({
77+
async() {
78+
return (_err, _result) => {
79+
err = _err
80+
result = _result
81+
}
8482
},
85-
cache: {
86-
get() {
87-
return false
83+
query: {
84+
getNodes(_type) {
85+
return fixtures.map(([, node]) => node)
8886
},
89-
set() {
90-
return
87+
getNodesByType(_type) {
88+
return fixtures.map(([, node]) => node)
9189
},
92-
},
93-
},
94-
resourcePath: fakeGatsbyNode.absolutePath,
95-
})
96-
await loader(content)
97-
}
98-
)
99-
100-
test.each(fixtures)(
101-
`snapshot [lessBabel=true] with %s`,
102-
async (filename, fakeGatsbyNode, content) => {
103-
const loader = mdxLoader.bind({
104-
async() {
105-
return (err, result) => {
106-
expect(err).toBeNull()
107-
expect(result).toMatchSnapshot()
108-
}
109-
},
110-
query: {
111-
getNodes(_type) {
112-
return fixtures.map(([, node]) => node)
113-
},
114-
getNodesByType(_type) {
115-
return fixtures.map(([, node]) => node)
116-
},
117-
pluginOptions: {
118-
lessBabel: true,
119-
},
120-
cache: {
121-
get() {
122-
return false
90+
pluginOptions: {
91+
lessBabel,
12392
},
124-
set() {
125-
return
93+
cache: {
94+
get() {
95+
return false
96+
},
97+
set() {
98+
return
99+
},
126100
},
101+
isolateMDXComponent: isDevelopStage,
127102
},
128-
},
129-
resourcePath: fakeGatsbyNode.absolutePath,
130-
})
131-
await loader(content)
103+
resourcePath: fakeGatsbyNode.absolutePath,
104+
resourceQuery: fakeGatsbyNode.absolutePath,
105+
rootContext: `/fake/`,
106+
...opts,
107+
})
108+
109+
await createLoader()(content)
110+
expect(err).toBeNull()
111+
expect(result).toMatchSnapshot()
112+
err = result = undefined
113+
114+
if (isDevelopStage) {
115+
await createLoader({
116+
resourceQuery: `${fakeGatsbyNode.absolutePath}?type=component`,
117+
})(content)
118+
expect(err).toBeNull()
119+
expect(result).toMatchSnapshot()
120+
}
132121
}
133122
)
134123
})

‎packages/gatsby-plugin-mdx/utils/gen-mdx.js

+6-2
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,14 @@ async function genMDX(
5151
reporter,
5252
cache,
5353
pathPrefix,
54+
isolateMDXComponent,
5455
...helpers
5556
},
5657
{ forceDisableCache = false } = {}
5758
) {
5859
const pathPrefixCacheStr = pathPrefix || ``
5960
const payloadCacheKey = node =>
60-
`gatsby-plugin-mdx-entire-payload-${node.internal.contentDigest}-${pathPrefixCacheStr}`
61+
`gatsby-plugin-mdx-entire-payload-${node.internal.contentDigest}-${pathPrefixCacheStr}-${isolateMDXComponent}`
6162

6263
if (!forceDisableCache) {
6364
const cachedPayload = await cache.get(payloadCacheKey(node))
@@ -89,7 +90,10 @@ async function genMDX(
8990
// pull classic style frontmatter off the raw MDX body
9091
debug(`processing classic frontmatter`)
9192
const { data, content: frontMatterCodeResult } = grayMatter(node.rawBody)
92-
const content = `${frontMatterCodeResult}
93+
94+
const content = isolateMDXComponent
95+
? frontMatterCodeResult
96+
: `${frontMatterCodeResult}
9397
9498
export const _frontmatter = ${JSON.stringify(data)}`
9599

‎packages/gatsby/src/utils/webpack.config.js

+40-1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ module.exports = async (
3838
port,
3939
{ parentSpan } = {}
4040
) => {
41+
let fastRefreshPlugin
4142
const modulesThatUseGatsby = await getGatsbyDependents()
4243
const directoryPath = withBasePath(directory)
4344

@@ -219,7 +220,7 @@ module.exports = async (
219220
case `develop`: {
220221
configPlugins = configPlugins
221222
.concat([
222-
plugins.fastRefresh({ modulesThatUseGatsby }),
223+
(fastRefreshPlugin = plugins.fastRefresh({ modulesThatUseGatsby })),
223224
new ForceCssHMRForEdgeCases(),
224225
plugins.hotModuleReplacement(),
225226
plugins.noEmitOnErrors(),
@@ -810,5 +811,43 @@ module.exports = async (
810811
parentSpan,
811812
})
812813

814+
if (fastRefreshPlugin) {
815+
// Fast refresh plugin has `include` option that determines
816+
// wether HMR code gets injected. We need to make sure all custom loaders
817+
// (like .ts or .mdx) that use our babel-loader will be taken into account
818+
// when deciding which modules get fast-refresh HMR addition.
819+
const fastRefreshIncludes = []
820+
const babelLoaderLoc = require.resolve(`./babel-loader`)
821+
for (const rule of getConfig().module.rules) {
822+
if (!rule.use) {
823+
continue
824+
}
825+
826+
const hasBabelLoader = (Array.isArray(rule.use)
827+
? rule.use
828+
: [rule.use]
829+
).some(loaderConfig => loaderConfig.loader === babelLoaderLoc)
830+
831+
if (hasBabelLoader) {
832+
fastRefreshIncludes.push(rule.test)
833+
}
834+
}
835+
836+
// start with default include of fast refresh plugin
837+
const includeRegex = /\.([jt]sx?|flow)$/i
838+
includeRegex.test = modulePath => {
839+
// drop query param from request (i.e. ?type=component for mdx-loader)
840+
// so loader rule test work well
841+
const queryParamStartIndex = modulePath.indexOf(`?`)
842+
if (queryParamStartIndex !== -1) {
843+
modulePath = modulePath.substr(0, queryParamStartIndex)
844+
}
845+
846+
return fastRefreshIncludes.some(re => re.test(modulePath))
847+
}
848+
849+
fastRefreshPlugin.options.include = includeRegex
850+
}
851+
813852
return getConfig()
814853
}

0 commit comments

Comments
 (0)
Please sign in to comment.