1
1
mirror of https://github.com/mdx-js/mdx.git synced 2024-09-19 03:17:10 +03:00

Add better errors when referencing missing components (#1811)

Previously, React or other JSX runtimes threw rather hard to read errors
when a component was undefined (because it wasn’t imported, passed, or
provided), essentially only pointing to *something* missing.
Now we throw proper errors when a component is missing at runtime,
including what exact component (or object) is undefined.

In addition, this adds a `development` option, which defaults to
`false` but can be configured explicitly or turned on with
`NODE_ENV=development`.
When it’s `true`, the exact place that references the missing component
or object, and which file did that, is included in the error message.

Related-to: mdx-js/mdx#1775.
Backports: wooorm/xdm@62e6f30.
This commit is contained in:
Titus 2021-11-13 10:28:46 +01:00 committed by GitHub
parent e86e9e8ce9
commit 2f96fbae3c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 416 additions and 61 deletions

View File

@ -177,7 +177,8 @@
"react/prop-types": "off",
"unicorn/prefer-node-protocol": "off",
"capitalized-comments": "off",
"complexity": "off"
"complexity": "off",
"max-depth": "off"
},
"overrides": [
{

View File

@ -0,0 +1 @@
export const development = false

View File

@ -0,0 +1,3 @@
import process from 'node:process'
export const development = process.env.NODE_ENV === 'development'

View File

@ -31,6 +31,7 @@ import {rehypeRecma} from './plugin/rehype-recma.js'
import {rehypeRemoveRaw} from './plugin/rehype-remove-raw.js'
import {remarkMarkAndUnravel} from './plugin/remark-mark-and-unravel.js'
import {nodeTypes} from './node-types.js'
import {development as defaultDevelopment} from './condition.js'
const removedOptions = [
'filepath',
@ -53,6 +54,7 @@ const removedOptions = [
*/
export function createProcessor(options = {}) {
const {
development = defaultDevelopment,
jsx,
format,
outputFormat,
@ -102,7 +104,7 @@ export function createProcessor(options = {}) {
pipeline
.use(rehypeRecma)
.use(recmaDocument, {...rest, outputFormat})
.use(recmaJsxRewrite, {providerImportSource, outputFormat})
.use(recmaJsxRewrite, {development, providerImportSource, outputFormat})
if (!jsx) {
pipeline.use(recmaJsxBuild, {outputFormat})

View File

@ -7,6 +7,7 @@
import {buildJsx} from 'estree-util-build-jsx'
import {specifiersToDeclarations} from '../util/estree-util-specifiers-to-declarations.js'
import {toIdOrMemberExpression} from '../util/estree-util-to-id-or-member-expression.js'
/**
* A plugin to build JSX into function calls.
@ -33,13 +34,10 @@ export function recmaJsxBuild(options = {}) {
tree.body[0] = {
type: 'VariableDeclaration',
kind: 'const',
declarations: specifiersToDeclarations(tree.body[0].specifiers, {
type: 'MemberExpression',
object: {type: 'Identifier', name: 'arguments'},
property: {type: 'Literal', value: 0},
computed: true,
optional: false
})
declarations: specifiersToDeclarations(
tree.body[0].specifiers,
toIdOrMemberExpression(['arguments', 0])
)
}
}
}

View File

@ -5,7 +5,6 @@
* @typedef {import('estree-jsx').ImportSpecifier} ImportSpecifier
* @typedef {import('estree-jsx').JSXElement} JSXElement
* @typedef {import('estree-jsx').JSXIdentifier} JSXIdentifier
* @typedef {import('estree-jsx').JSXMemberExpression} JSXMemberExpression
* @typedef {import('estree-jsx').JSXNamespacedName} JSXNamespacedName
* @typedef {import('estree-jsx').ModuleDeclaration} ModuleDeclaration
* @typedef {import('estree-jsx').Program} Program
@ -13,6 +12,7 @@
* @typedef {import('estree-jsx').Statement} Statement
* @typedef {import('estree-jsx').VariableDeclarator} VariableDeclarator
* @typedef {import('estree-jsx').ObjectPattern} ObjectPattern
* @typedef {import('estree-jsx').Identifier} Identifier
*
* @typedef {import('estree-walker').SyncHandler} WalkHandler
*
@ -21,18 +21,29 @@
* @typedef RecmaJsxRewriteOptions
* @property {'program'|'function-body'} [outputFormat='program'] Whether to use an import statement or `arguments[0]` to get the provider
* @property {string} [providerImportSource] Place to import a provider from
* @property {boolean} [development=false] Whether to add extra information to error messages in generated code (can also be passed in Node.js by setting `NODE_ENV=development`)
*
* @typedef StackEntry
* @property {Array.<string>} objects
* @property {Array.<string>} components
* @property {Array.<string>} tags
* @property {Record.<string, {node: JSXElement, component: boolean}>} references
* @property {ESFunction} node
*/
import {stringifyPosition} from 'unist-util-stringify-position'
import {positionFromEstree} from 'unist-util-position-from-estree'
import {name as isIdentifierName} from 'estree-util-is-identifier-name'
import {walk} from 'estree-walker'
import {analyze} from 'periscopic'
import {specifiersToDeclarations} from '../util/estree-util-specifiers-to-declarations.js'
import {
toIdOrMemberExpression,
toJsxIdOrMemberExpression
} from '../util/estree-util-to-id-or-member-expression.js'
import {toBinaryAddition} from '../util/estree-util-to-binary-addition.js'
const own = {}.hasOwnProperty
/**
* A plugin that rewrites JSX in functions to accept components as
@ -44,15 +55,17 @@ import {specifiersToDeclarations} from '../util/estree-util-specifiers-to-declar
* @type {import('unified').Plugin<[RecmaJsxRewriteOptions]|[], Program>}
*/
export function recmaJsxRewrite(options = {}) {
const {providerImportSource, outputFormat} = options
const {development, providerImportSource, outputFormat} = options
return (tree) => {
return (tree, file) => {
// Find everything thats defined in the top-level scope.
const scopeInfo = analyze(tree)
/** @type {Array.<StackEntry>} */
const fnStack = []
/** @type {boolean|undefined} */
let importProvider
/** @type {boolean|undefined} */
let createErrorHelper
/** @type {Scope|null} */
let currentScope
@ -65,7 +78,13 @@ export function recmaJsxRewrite(options = {}) {
node.type === 'FunctionExpression' ||
node.type === 'ArrowFunctionExpression'
) {
fnStack.push({objects: [], components: [], tags: [], node})
fnStack.push({
objects: [],
components: [],
tags: [],
references: {},
node
})
}
let fnScope = fnStack[0]
@ -100,11 +119,23 @@ export function recmaJsxRewrite(options = {}) {
// `<x.y>`, `<Foo.Bar>`, `<x.y.z>`.
if (name.type === 'JSXMemberExpression') {
// Find the left-most identifier.
while (name.type === 'JSXMemberExpression') name = name.object
/** @type {string[]} */
const ids = []
// Find the left-most identifier.
while (name.type === 'JSXMemberExpression') {
ids.unshift(name.property.name)
name = name.object
}
ids.unshift(name.name)
const fullId = ids.join('.')
const id = name.name
if (!own.call(fnScope.references, fullId)) {
fnScope.references[fullId] = {node, component: true}
}
if (!fnScope.objects.includes(id) && !inScope(currentScope, id)) {
fnScope.objects.push(id)
}
@ -120,11 +151,16 @@ export function recmaJsxRewrite(options = {}) {
else if (isIdentifierName(name.name) && !/^[a-z]/.test(name.name)) {
const id = name.name
if (
!fnScope.components.includes(id) &&
!inScope(currentScope, id)
) {
fnScope.components.push(id)
if (!inScope(currentScope, id)) {
// No need to add an error for an undefined layout — we use an
// `if` later.
if (id !== 'MDXLayout' && !own.call(fnScope.references, id)) {
fnScope.references[id] = {node, component: true}
}
if (!fnScope.components.includes(id)) {
fnScope.components.push(id)
}
}
}
// @ts-expect-error Allow fields passed through from mdast through hast to
@ -147,11 +183,10 @@ export function recmaJsxRewrite(options = {}) {
}
if (node.closingElement) {
node.closingElement.name = {
type: 'JSXMemberExpression',
object: {type: 'JSXIdentifier', name: '_components'},
property: {type: 'JSXIdentifier', name: id}
}
node.closingElement.name = toJsxIdOrMemberExpression([
'_components',
id
])
}
}
}
@ -203,6 +238,26 @@ export function recmaJsxRewrite(options = {}) {
}
}
/** @type {string} */
let key
// Add partials (so for `x.y.z` itd generate `x` and `x.y` too).
for (key in scope.references) {
if (own.call(scope.references, key)) {
const parts = key.split('.')
let index = 0
while (++index < parts.length) {
const partial = parts.slice(0, index).join('.')
if (!own.call(scope.references, partial)) {
scope.references[partial] = {
node: scope.references[key].node,
component: false
}
}
}
}
}
if (defaults.length > 0 || actual.length > 0) {
if (providerImportSource) {
importProvider = true
@ -220,13 +275,7 @@ export function recmaJsxRewrite(options = {}) {
isNamedFunction(scope.node, 'MDXContent') ||
isNamedFunction(scope.node, '_createMdxContent')
) {
parameters.push({
type: 'MemberExpression',
object: {type: 'Identifier', name: 'props'},
property: {type: 'Identifier', name: 'components'},
computed: false,
optional: false
})
parameters.push(toIdOrMemberExpression(['props', 'components']))
}
if (defaults.length > 0 || parameters.length > 1) {
@ -242,13 +291,7 @@ export function recmaJsxRewrite(options = {}) {
parameters.length > 1
? {
type: 'CallExpression',
callee: {
type: 'MemberExpression',
object: {type: 'Identifier', name: 'Object'},
property: {type: 'Identifier', name: 'assign'},
computed: false,
optional: false
},
callee: toIdOrMemberExpression(['Object', 'assign']),
arguments: parameters,
optional: false
}
@ -316,11 +359,55 @@ export function recmaJsxRewrite(options = {}) {
}
}
fn.body.body.unshift({
type: 'VariableDeclaration',
kind: 'const',
declarations
})
/** @type {Statement[]} */
const statements = [
{
type: 'VariableDeclaration',
kind: 'const',
declarations
}
]
const references = Object.keys(scope.references).sort()
let index = -1
while (++index < references.length) {
const id = references[index]
const info = scope.references[id]
const place = stringifyPosition(positionFromEstree(info.node))
/** @type {Expression[]} */
const parameters = [
{type: 'Literal', value: id},
{type: 'Literal', value: info.component}
]
createErrorHelper = true
if (development && place !== '1:1-1:1') {
parameters.push({type: 'Literal', value: place})
}
statements.push({
type: 'IfStatement',
test: {
type: 'UnaryExpression',
operator: '!',
prefix: true,
argument: toIdOrMemberExpression(id.split('.'))
},
consequent: {
type: 'ExpressionStatement',
expression: {
type: 'CallExpression',
callee: {type: 'Identifier', name: '_missingMdxReference'},
arguments: parameters,
optional: false
}
},
alternate: null
})
}
fn.body.body.unshift(...statements)
}
fnStack.pop()
@ -334,6 +421,72 @@ export function recmaJsxRewrite(options = {}) {
createImportProvider(providerImportSource, outputFormat)
)
}
// If potentially missing components are used.
if (createErrorHelper) {
/** @type {Expression[]} */
const message = [
{type: 'Literal', value: 'Expected '},
{
type: 'ConditionalExpression',
test: {type: 'Identifier', name: 'component'},
consequent: {type: 'Literal', value: 'component'},
alternate: {type: 'Literal', value: 'object'}
},
{type: 'Literal', value: ' `'},
{type: 'Identifier', name: 'id'},
{
type: 'Literal',
value:
'` to be defined: you likely forgot to import, pass, or provide it.'
}
]
/** @type {Identifier[]} */
const parameters = [
{type: 'Identifier', name: 'id'},
{type: 'Identifier', name: 'component'}
]
if (development) {
message.push({
type: 'ConditionalExpression',
test: {type: 'Identifier', name: 'place'},
consequent: toBinaryAddition([
{type: 'Literal', value: '\nIts referenced in your code at `'},
{type: 'Identifier', name: 'place'},
{
type: 'Literal',
value: (file.path ? '` in `' + file.path : '') + '`'
}
]),
alternate: {type: 'Literal', value: ''}
})
parameters.push({type: 'Identifier', name: 'place'})
}
tree.body.push({
type: 'FunctionDeclaration',
id: {type: 'Identifier', name: '_missingMdxReference'},
generator: false,
async: false,
params: parameters,
body: {
type: 'BlockStatement',
body: [
{
type: 'ThrowStatement',
argument: {
type: 'NewExpression',
callee: {type: 'Identifier', name: 'Error'},
arguments: [toBinaryAddition(message)]
}
}
]
}
})
}
}
}
@ -356,13 +509,10 @@ function createImportProvider(providerImportSource, outputFormat) {
? {
type: 'VariableDeclaration',
kind: 'const',
declarations: specifiersToDeclarations(specifiers, {
type: 'MemberExpression',
object: {type: 'Identifier', name: 'arguments'},
property: {type: 'Literal', value: 0},
computed: true,
optional: false
})
declarations: specifiersToDeclarations(
specifiers,
toIdOrMemberExpression(['arguments', 0])
)
}
: {
type: 'ImportDeclaration',

View File

@ -0,0 +1,23 @@
/**
* @typedef {import('estree-jsx').Expression} Expression
*/
/**
* @param {Expression[]} expressions
*/
export function toBinaryAddition(expressions) {
let index = -1
/** @type {Expression|undefined} */
let left
while (++index < expressions.length) {
const right = expressions[index]
left = left ? {type: 'BinaryExpression', left, operator: '+', right} : right
}
// Just for types.
/* c8 ignore next */
if (!left) throw new Error('Expected non-empty `expressions` to be passed')
return left
}

View File

@ -0,0 +1,64 @@
/**
* @typedef {import('estree-jsx').Identifier} Identifier
* @typedef {import('estree-jsx').Literal} Literal
* @typedef {import('estree-jsx').JSXIdentifier} JSXIdentifier
* @typedef {import('estree-jsx').MemberExpression} MemberExpression
* @typedef {import('estree-jsx').JSXMemberExpression} JSXMemberExpression
*/
import {name as isIdentifierName} from 'estree-util-is-identifier-name'
export const toIdOrMemberExpression = toIdOrMemberExpressionFactory(
'Identifier',
'MemberExpression'
)
export const toJsxIdOrMemberExpression =
// @ts-expect-error: fine
/** @type {(ids: Array.<string|number>) => JSXIdentifier|JSXMemberExpression)} */
(toIdOrMemberExpressionFactory('JSXIdentifier', 'JSXMemberExpression'))
/**
* @param {string} [idType]
* @param {string} [memberType]
*/
function toIdOrMemberExpressionFactory(idType, memberType) {
return toIdOrMemberExpression
/**
* @param {Array.<string|number>} ids
* @returns {Identifier|MemberExpression}
*/
function toIdOrMemberExpression(ids) {
let index = -1
/** @type {Identifier|Literal|MemberExpression|undefined} */
let object
while (++index < ids.length) {
const name = ids[index]
/** @type {Identifier|Literal} */
// @ts-expect-error: JSX is fine.
const id =
typeof name === 'string' && isIdentifierName(name)
? {type: idType, name}
: {type: 'Literal', value: name}
// @ts-expect-error: JSX is fine.
object = object
? {
type: memberType,
object,
property: id,
computed: id.type === 'Literal',
optional: false
}
: id
}
// Just for types.
/* c8 ignore next 3 */
if (!object) throw new Error('Expected non-empty `ids` to be passed')
if (object.type === 'Literal')
throw new Error('Expected identifier as left-most value')
return object
}
}

View File

@ -34,6 +34,12 @@
"sideEffects": false,
"main": "index.js",
"types": "index.d.ts",
"browser": {
"./lib/condition.js": "./lib/condition.browser.js"
},
"react-native": {
"./lib/condition.js": "./lib/condition.browser.js"
},
"files": [
"lib/",
"index.d.ts",

View File

@ -378,6 +378,77 @@ export default MDXContent
</details>
###### `options.development`
Whether to add extra information to error messages in generated code
(`boolean?`, default: `false`).
The default can be set to `true` in Node.js through environment variables: set
`NODE_ENV=development`.
<details>
<summary>Example</summary>
Say we had some MDX that references a component that can be passed or provided
at runtime:
```mdx
**Note**<NoteIcon />: some stuff.
```
And a module to evaluate that:
```js
import {promises as fs} from 'node:fs'
import * as runtime from 'react/jsx-runtime'
import {evaluate} from '@mdx-js/mdx'
main()
async function main() {
const path = 'example.mdx'
const value = await fs.readFile(path)
const MDXContent = (await evaluate({path, value}, runtime)).default
console.log(MDXContent())
}
```
Running that would normally (production) yield:
```txt
Error: Expected component `NoteIcon` to be defined: you likely forgot to import, pass, or provide it.
at _missingMdxReference (eval at run (…/@mdx-js/mdx/lib/run.js:18:10), <anonymous>:27:9)
at _createMdxContent (eval at run (…/@mdx-js/mdx/lib/run.js:18:10), <anonymous>:15:20)
at MDXContent (eval at run (…/@mdx-js/mdx/lib/run.js:18:10), <anonymous>:9:9)
at main (…/example.js:11:15)
```
But if we change add `development: true` to our example:
```diff
@@ -7,6 +7,6 @@ main()
async function main() {
const path = 'example.mdx'
const value = await fs.readFile(path)
- const MDXContent = (await evaluate({path, value}, runtime)).default
+ const MDXContent = (await evaluate({path, value}, {development: true, ...runtime})).default
console.log(MDXContent({}))
}
```
And wed run it again, wed get:
```txt
Error: Expected component `NoteIcon` to be defined: you likely forgot to import, pass, or provide it.
Its referenced in your code at `1:9-1:21` in `example.mdx`
provide it.
at _missingMdxReference (eval at run (…/@mdx-js/mdx/lib/run.js:18:10), <anonymous>:27:9)
at _createMdxContent (eval at run (…/@mdx-js/mdx/lib/run.js:18:10), <anonymous>:15:20)
at MDXContent (eval at run (…/@mdx-js/mdx/lib/run.js:18:10), <anonymous>:9:9)
at main (…/example.js:11:15)
```
</details>
###### `options.SourceMapGenerator`
The `SourceMapGenerator` class from [`source-map`][source-map] (optional).

View File

@ -41,7 +41,6 @@ test('compile', async () => {
assert.unreachable()
} catch (/** @type {unknown} */ error) {
const exception = /** @type {Error} */ (error)
console.log(exception.message)
assert.match(
exception.message,
/`options.filepath` is no longer supported/,
@ -470,8 +469,6 @@ test('compile', async () => {
)
}
console.log('\nnote: the next warning is expected!\n')
try {
renderToStaticMarkup(React.createElement(await run(compileSync('<X />'))))
assert.unreachable()
@ -479,7 +476,7 @@ test('compile', async () => {
const exception = /** @type {Error} */ (error)
assert.match(
exception.message,
/Element type is invalid/,
/Expected component `X` to be defined/,
'should throw if a required component is not passed'
)
}
@ -491,11 +488,46 @@ test('compile', async () => {
const exception = /** @type {Error} */ (error)
assert.match(
exception.message,
/Cannot read propert/,
/Expected object `a` to be defined/,
'should throw if a required member is not passed'
)
}
try {
renderToStaticMarkup(
React.createElement(await run(compileSync('<X />', {development: true})))
)
assert.unreachable()
} catch (/** @type {unknown} */ error) {
const exception = /** @type {Error} */ (error)
assert.match(
exception.message,
/Its referenced in your code at `1:1-1:6/,
'should pass more info to errors w/ `development: true`'
)
}
try {
renderToStaticMarkup(
React.createElement(
await run(
compileSync(
{value: 'asd <a.b />', path: 'folder/example.mdx'},
{development: true}
)
)
)
)
assert.unreachable()
} catch (/** @type {unknown} */ error) {
const exception = /** @type {Error} */ (error)
assert.match(
exception.message,
/Its referenced in your code at `1:5-1:12` in `folder\/example.mdx`/,
'should show what file contains the error w/ `development: true`, and `path`'
)
}
assert.equal(
renderToStaticMarkup(
React.createElement(
@ -516,8 +548,6 @@ test('compile', async () => {
'should support setting components through context with a `providerImportSource`'
)
console.log('\nnote: the next warning is expected!\n')
try {
renderToStaticMarkup(
React.createElement(
@ -529,7 +559,7 @@ test('compile', async () => {
const exception = /** @type {Error} */ (error)
assert.match(
exception.message,
/Element type is invalid/,
/Expected component `X` to be defined/,
'should throw if a required component is not passed or given to `MDXProvider`'
)
}
@ -736,10 +766,15 @@ test('jsx', async () => {
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent /></MDXLayout> : _createMdxContent();',
' function _createMdxContent() {',
' const {c} = props.components || ({});',
' if (!c) _missingMdxReference("c", false);',
' if (!c.d) _missingMdxReference("c.d", true);',
' return <><><a:b /><c.d /></></>;',
' }',
'}',
'export default MDXContent;',
'function _missingMdxReference(id, component) {',
' throw new Error("Expected " + (component ? "component" : "object") + " `" + id + "` to be defined: you likely forgot to import, pass, or provide it.");',
'}',
''
].join('\n'),
'should serialize fragments, namespaces, members'

View File

@ -116,9 +116,10 @@ Create a Node ESM loader to compile MDX to JS.
```js
import {createLoader} from '@mdx-js/node-loader'
const {getFormat, transformSource} = createLoader(/* Options… */)
// Load is for Node 17+, the rest for 12-16.
const {load, getFormat, transformSource} = createLoader(/* Options… */)
export {getFormat, transformSource}
export {load, getFormat, transformSource}
```
This example can then be used with `node --experimental-loader=my-loader.js`.