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.

Sign up for GitHub

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

Jump to bottom

Support jsx dev runtime #2045

Merged
merged 5 commits into from
Dec 14, 2022
Merged

Support jsx dev runtime #2045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5d88053
Support jsx dev runtime
remcohaszing May 18, 2022
3ca82dc
Support jsx dev runtime in evaluate
remcohaszing Dec 6, 2022
1cd4700
Merge branch 'main' into jsx-dev-runtime
remcohaszing Dec 6, 2022
7891c37
Move dev runtime compile output test
remcohaszing Dec 6, 2022
6085e65
Revert package-lock.json changes
remcohaszing Dec 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/mdx/lib/core.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export function createProcessor(options = {}) {
.use(recmaJsxRewrite, {development, providerImportSource, outputFormat})

if (!jsx) {
pipeline.use(recmaJsxBuild, {outputFormat})
pipeline.use(recmaJsxBuild, {development, outputFormat})
}

pipeline.use(recmaStringify, {SourceMapGenerator}).use(recmaPlugins || [])
Expand Down
11 changes: 6 additions & 5 deletions packages/mdx/lib/plugin/recma-jsx-build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/**
* @typedef {import('estree-jsx').Program} Program
* @typedef {import('estree-util-build-jsx').BuildJsxOptions} BuildJsxOptions
*
* @typedef RecmaJsxBuildOptions
* @property {'program'|'function-body'} [outputFormat='program']
Expand All @@ -15,13 +16,13 @@ import {toIdOrMemberExpression} from '../util/estree-util-to-id-or-member-expres
* A plugin to build JSX into function calls.
* `estree-util-build-jsx` does all the work for us!
*
* @type {import('unified').Plugin<[RecmaJsxBuildOptions]|[], Program>}
* @type {import('unified').Plugin<[BuildJsxOptions & RecmaJsxBuildOptions?], Program>}
*/
export function recmaJsxBuild(options = {}) {
const {outputFormat} = options
const {development, outputFormat} = options

return (tree) => {
buildJsx(tree)
return (tree, file) => {
buildJsx(tree, {development, filePath: file.history[0]})

// When compiling to a function body, replace the import that was just
// generated, and get `jsx`, `jsxs`, and `Fragment` from `arguments[0]`
Expand All @@ -31,7 +32,7 @@ export function recmaJsxBuild(options = {}) {
tree.body[0] &&
tree.body[0].type === 'ImportDeclaration' &&
typeof tree.body[0].source.value === 'string' &&
/\/jsx-runtime$/.test(tree.body[0].source.value)
/\/jsx-(dev-)?runtime$/.test(tree.body[0].source.value)
) {
tree.body[0] = {
type: 'VariableDeclaration',
Expand Down
24 changes: 16 additions & 8 deletions packages/mdx/lib/util/resolve-evaluate-options.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
* @typedef RunnerOptions
* @property {*} Fragment
* Symbol to use for fragments.
* @property {*} jsx
* Function to generate an element with static children.
* @property {*} jsxs
* Function to generate an element with dynamic children.
* @property {*} [jsx]
* Function to generate an element with static children in production mode.
* @property {*} [jsxs]
* Function to generate an element with dynamic children in production mode.
* @property {*} [jsxDEV]
* Function to generate an element in development mode.
* @property {*} [useMDXComponents]
* Function to get `MDXComponents` from context.
*
Expand All @@ -23,18 +25,24 @@
* @returns {{compiletime: ProcessorOptions, runtime: RunnerOptions}}
*/
export function resolveEvaluateOptions(options) {
const {Fragment, jsx, jsxs, useMDXComponents, ...rest} = options || {}
const {development, Fragment, jsx, jsxs, jsxDEV, useMDXComponents, ...rest} =
options || {}

if (!Fragment) throw new Error('Expected `Fragment` given to `evaluate`')
if (!jsx) throw new Error('Expected `jsx` given to `evaluate`')
if (!jsxs) throw new Error('Expected `jsxs` given to `evaluate`')
if (development) {
if (!jsxDEV) throw new Error('Expected `jsxDEV` given to `evaluate`')
} else {
if (!jsx) throw new Error('Expected `jsx` given to `evaluate`')
if (!jsxs) throw new Error('Expected `jsxs` given to `evaluate`')
}

return {
compiletime: {
...rest,
development,
outputFormat: 'function-body',
providerImportSource: useMDXComponents ? '#' : undefined
},
runtime: {Fragment, jsx, jsxs, useMDXComponents}
runtime: {Fragment, jsx, jsxs, jsxDEV, useMDXComponents}
}
}
50 changes: 50 additions & 0 deletions packages/mdx/test/compile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,22 @@ test('compile', async () => {
)
console.log('\nnote: the preceding warning is expected!\n')

const developmentSourceNode = (
await run(
compileSync(
{value: '<div />', path: 'path/to/file.js'},
{development: true}
).value
)
)({})
assert.equal(
// @ts-expect-error React attaches source information on this property,
// but it’s private and untyped.
developmentSourceNode._source,
{fileName: 'path/to/file.js', lineNumber: 1, columnNumber: 1},
'should expose source information in the automatic jsx dev runtime'
)
Copy link
Member Author

@remcohaszing remcohaszing May 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

The test above uses an undocumented private property of React elements. The test below asserts the generated source code. None of these tests is ideal, but I’m not sure which is worse. I suggest to keep only one of them though.


try {
renderToStaticMarkup(
React.createElement(await run(compileSync('<X />', {development: true})))
Expand Down Expand Up @@ -1503,6 +1519,40 @@ test('MDX (ESM)', async () => {
'<div style="color:red"><p>a</p></div>',
'should support rexporting the default export, and other things, from a source'
)

assert.equal(
compileSync({value: '<X />', path: 'path/to/file.js'}, {development: true})
.value,
[
'/*@jsxRuntime automatic @jsxImportSource react*/',
'import {jsxDEV as _jsxDEV} from "react/jsx-dev-runtime";',
'function _createMdxContent(props) {',
' const {X} = props.components || ({});',
' if (!X) _missingMdxReference("X", true, "1:1-1:6");',
' return _jsxDEV(X, {}, undefined, false, {',
' fileName: "path/to/file.js",',
' lineNumber: 1,',
' columnNumber: 1',
' }, this);',
'}',
'function MDXContent(props = {}) {',
' const {wrapper: MDXLayout} = props.components || ({});',
' return MDXLayout ? _jsxDEV(MDXLayout, Object.assign({}, props, {',
' children: _jsxDEV(_createMdxContent, props, undefined, false, {',
' fileName: "path/to/file.js"',
' }, this)',
' }), undefined, false, {',
' fileName: "path/to/file.js"',
' }, this) : _createMdxContent(props);',
'}',
'export default MDXContent;',
'function _missingMdxReference(id, component, place) {',
' throw new Error("Expected " + (component ? "component" : "object") + " `" + id + "` to be defined: you likely forgot to import, pass, or provide it." + (place ? "\\nIt’s referenced in your code at `" + place + "` in `path/to/file.js`" : ""));',
'}',
''
].join('\n'),
'should support the jsx dev runtime'
)
})

test('source maps', async () => {
Expand Down
33 changes: 31 additions & 2 deletions packages/mdx/test/evaluate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {renderToStaticMarkup as renderToStaticMarkup_} from '../../react/node_mo
// @ts-expect-error: make sure a single react is used.
import * as runtime_ from '../../react/node_modules/react/jsx-runtime.js'
// @ts-expect-error: make sure a single react is used.
import * as devRuntime from '../../react/node_modules/react/jsx-dev-runtime.js'
// @ts-expect-error: make sure a single react is used.
import React_ from '../../react/node_modules/react/index.js'
import * as provider from '../../react/index.js'

Expand All @@ -30,7 +32,6 @@ test('evaluate', async () => {

assert.throws(
() => {
// @ts-expect-error: missing required arguments
evaluateSync('a', {Fragment: runtime.Fragment})
},
/Expected `jsx` given to `evaluate`/,
Expand All @@ -39,13 +40,20 @@ test('evaluate', async () => {

assert.throws(
() => {
// @ts-expect-error: missing required arguments
evaluateSync('a', {Fragment: runtime.Fragment, jsx: runtime.jsx})
},
/Expected `jsxs` given to `evaluate`/,
'should throw on missing `jsxs`'
)

assert.throws(
() => {
evaluateSync('a', {Fragment: runtime.Fragment, development: true})
},
/Expected `jsxDEV` given to `evaluate`/,
'should throw on missing `jsxDEV` in dev mode'
)

assert.equal(
renderToStaticMarkup(
React.createElement((await evaluate('# hi!', runtime)).default)
Expand All @@ -62,6 +70,27 @@ test('evaluate', async () => {
'should evaluate (sync)'
)

assert.equal(
renderToStaticMarkup(
React.createElement(
(await evaluate('# hi dev!', {development: true, ...devRuntime}))
.default
)
),
'<h1>hi dev!</h1>',
'should evaluate (sync)'
)

assert.equal(
renderToStaticMarkup(
React.createElement(
evaluateSync('# hi dev!', {development: true, ...devRuntime}).default
)
),
'<h1>hi dev!</h1>',
'should evaluate (sync)'
)

assert.equal(
renderToStaticMarkup(
React.createElement(
Expand Down