From 5584e49c532efb285d29aa89d2b32cae117e375e Mon Sep 17 00:00:00 2001 From: Vlad Zhukov Date: Mon, 28 Mar 2022 20:12:39 +0400 Subject: [PATCH 1/4] Make sure _missingMdxReference checks are always added --- packages/mdx/lib/plugin/recma-jsx-rewrite.js | 136 ++++++++++--------- packages/mdx/test/compile.js | 32 +++++ 2 files changed, 101 insertions(+), 67 deletions(-) diff --git a/packages/mdx/lib/plugin/recma-jsx-rewrite.js b/packages/mdx/lib/plugin/recma-jsx-rewrite.js index db2e16110..0858a0be5 100644 --- a/packages/mdx/lib/plugin/recma-jsx-rewrite.js +++ b/packages/mdx/lib/plugin/recma-jsx-rewrite.js @@ -243,25 +243,8 @@ export function recmaJsxRewrite(options = {}) { } } - /** @type {string} */ - let key - - // Add partials (so for `x.y.z` it’d 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 - } - } - } - } - } + /** @type {Array} */ + const statements = [] if (defaults.length > 0 || actual.length > 0) { if (providerImportSource) { @@ -356,60 +339,79 @@ export function recmaJsxRewrite(options = {}) { }) } - // Arrow functions with an implied return: - if (fn.body.type !== 'BlockStatement') { - fn.body = { - type: 'BlockStatement', - body: [{type: 'ReturnStatement', argument: fn.body}] + statements.push({ + type: 'VariableDeclaration', + kind: 'const', + declarations + }) + } + + /** @type {string} */ + let key + + // Add partials (so for `x.y.z` it’d 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 + } + } } } + } - /** @type {Array} */ - 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 {Array} */ + const parameters = [ + {type: 'Literal', value: id}, + {type: 'Literal', value: info.component} ] - 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 {Array} */ - 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}) - } + createErrorHelper = true - 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 - }) + 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 + }) + } + + if (statements.length > 0) { + // Arrow functions with an implied return: + if (fn.body.type !== 'BlockStatement') { + fn.body = { + type: 'BlockStatement', + body: [{type: 'ReturnStatement', argument: fn.body}] + } } fn.body.body.unshift(...statements) diff --git a/packages/mdx/test/compile.js b/packages/mdx/test/compile.js index 9b19012a9..5715f799a 100644 --- a/packages/mdx/test/compile.js +++ b/packages/mdx/test/compile.js @@ -504,6 +504,38 @@ test('compile', async () => { ) } + try { + renderToStaticMarkup( + React.createElement( + await run(compileSync('export const a = {}\n\n')) + ) + ) + assert.unreachable() + } catch (/** @type {unknown} */ error) { + const exception = /** @type {Error} */ (error) + assert.match( + exception.message, + /Expected component `a.b` to be defined/, + 'should throw if a required member is not passed' + ) + } + + try { + renderToStaticMarkup( + React.createElement( + await run(compileSync(' } />')) + ) + ) + assert.unreachable() + } catch (/** @type {unknown} */ error) { + const exception = /** @type {Error} */ (error) + assert.match( + exception.message, + /x is not defined/, + 'should throw if a required member is not passed' + ) + } + try { renderToStaticMarkup( React.createElement(await run(compileSync('', {development: true}))) From e15f403592fa476e70eb6b57d3e3c7a39234fcf3 Mon Sep 17 00:00:00 2001 From: Vlad Zhukov Date: Mon, 28 Mar 2022 19:57:44 +0300 Subject: [PATCH 2/4] Update packages/mdx/test/compile.js Co-authored-by: Titus --- packages/mdx/test/compile.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mdx/test/compile.js b/packages/mdx/test/compile.js index 5715f799a..3a1df0fcd 100644 --- a/packages/mdx/test/compile.js +++ b/packages/mdx/test/compile.js @@ -532,7 +532,7 @@ test('compile', async () => { assert.match( exception.message, /x is not defined/, - 'should throw if a required member is not passed' + 'should throw if a used member is not defined locally (JSX in a function)' ) } From ae53899620950bd2cd0d6ca826aafbb57364379a Mon Sep 17 00:00:00 2001 From: Vlad Zhukov Date: Mon, 28 Mar 2022 19:57:59 +0300 Subject: [PATCH 3/4] Update packages/mdx/test/compile.js Co-authored-by: Titus --- packages/mdx/test/compile.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mdx/test/compile.js b/packages/mdx/test/compile.js index 3a1df0fcd..fcffe4871 100644 --- a/packages/mdx/test/compile.js +++ b/packages/mdx/test/compile.js @@ -516,7 +516,7 @@ test('compile', async () => { assert.match( exception.message, /Expected component `a.b` to be defined/, - 'should throw if a required member is not passed' + 'should throw if a used member is not defined locally' ) } From bf762c876bc4ee7da3797e42141434e60f6d7771 Mon Sep 17 00:00:00 2001 From: Vlad Zhukov Date: Tue, 29 Mar 2022 13:21:11 +0400 Subject: [PATCH 4/4] Add comments that tests show incorrect behavior --- packages/mdx/test/compile.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/mdx/test/compile.js b/packages/mdx/test/compile.js index fcffe4871..b98cf6c16 100644 --- a/packages/mdx/test/compile.js +++ b/packages/mdx/test/compile.js @@ -504,6 +504,7 @@ test('compile', async () => { ) } + // TODO: this is incorrect behavior, will be fixed in GH-1986 try { renderToStaticMarkup( React.createElement( @@ -520,6 +521,7 @@ test('compile', async () => { ) } + // TODO: this is incorrect behavior, will be fixed in GH-1986 try { renderToStaticMarkup( React.createElement(