From d381162cb4601723df59a60bc5f7a5e84f83dad9 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Wed, 8 Jun 2022 11:05:34 -0700 Subject: [PATCH 1/2] Update error transform to allow excluding errors inside subexpressions like ternaries --- .../transform-error-messages.js.snap | 14 ++++++++++ .../__tests__/transform-error-messages.js | 27 +++++++++++++++++++ .../error-codes/transform-error-messages.js | 10 ++++++- 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap b/scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap index 97870a4b31b..6c8ff92c7ab 100644 --- a/scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap +++ b/scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap @@ -5,6 +5,20 @@ exports[`error transform handles escaped backticks in template string 1`] = ` Error(_formatProdErrorMessage(231, listener, type));" `; +exports[`error transform handles ignoring errors that are comment-excluded inside ternary expressions 1`] = ` +"/*! FIXME (minify-errors-in-prod): Unminified error message in production build!*/ + +/*! \\"bar\\"*/ +var val = someBool ? //eslint-disable-next-line react-internal/prod-error-codes +new Error('foo') : someOtherBool ? new Error('bar') : //eslint-disable-next-line react-internal/prod-error-codes +new Error('baz');" +`; + +exports[`error transform handles ignoring errors that are comment-excluded outside ternary expressions 1`] = ` +"//eslint-disable-next-line react-internal/prod-error-codes +var val = someBool ? new Error('foo') : someOtherBool ? new Error('bar') : new Error('baz');" +`; + exports[`error transform should not touch other calls or new expressions 1`] = ` "new NotAnError(); NotAnError();" diff --git a/scripts/error-codes/__tests__/transform-error-messages.js b/scripts/error-codes/__tests__/transform-error-messages.js index 3cf08b69e84..4283428e9fe 100644 --- a/scripts/error-codes/__tests__/transform-error-messages.js +++ b/scripts/error-codes/__tests__/transform-error-messages.js @@ -106,6 +106,33 @@ new Error(\`Expected \${foo} target to \` + \`be an array; got \${bar}\`); expect( transform(` new Error(\`Expected \\\`\$\{listener\}\\\` listener to be a function, instead got a value of \\\`\$\{type\}\\\` type.\`); +`) + ).toMatchSnapshot(); + }); + + it('handles ignoring errors that are comment-excluded inside ternary expressions', () => { + expect( + transform(` +let val = someBool + ? //eslint-disable-next-line react-internal/prod-error-codes + new Error('foo') + : someOtherBool + ? new Error('bar') + : //eslint-disable-next-line react-internal/prod-error-codes + new Error('baz'); +`) + ).toMatchSnapshot(); + }); + + it('handles ignoring errors that are comment-excluded outside ternary expressions', () => { + expect( + transform(` +//eslint-disable-next-line react-internal/prod-error-codes +let val = someBool + ? new Error('foo') + : someOtherBool + ? new Error('bar') + : new Error('baz'); `) ).toMatchSnapshot(); }); diff --git a/scripts/error-codes/transform-error-messages.js b/scripts/error-codes/transform-error-messages.js index a429ed4008b..924a7b14d18 100644 --- a/scripts/error-codes/transform-error-messages.js +++ b/scripts/error-codes/transform-error-messages.js @@ -62,8 +62,16 @@ module.exports = function(babel) { // throw Error(`A ${adj} message that contains ${noun}`); // } + let leadingComments = node.leadingComments; + const statementParent = path.getStatementParent(); - const leadingComments = statementParent.node.leadingComments; + const parentLeadingComments = statementParent.node.leadingComments; + + if (parentLeadingComments) { + leadingComments = leadingComments + ? leadingComments.concat(parentLeadingComments) + : parentLeadingComments; + } if (leadingComments !== undefined) { for (let i = 0; i < leadingComments.length; i++) { // TODO: Since this only detects one of many ways to disable a lint From 9ab80c57142451aa2e27d680042c527420627e55 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Wed, 8 Jun 2022 14:33:38 -0700 Subject: [PATCH 2/2] make leadingcomments aggregation walk the expression stack --- .../transform-error-messages.js.snap | 10 +++++++++ .../__tests__/transform-error-messages.js | 21 +++++++++++++++++++ .../error-codes/transform-error-messages.js | 19 ++++++++++------- 3 files changed, 43 insertions(+), 7 deletions(-) diff --git a/scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap b/scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap index 6c8ff92c7ab..924dcdfa91f 100644 --- a/scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap +++ b/scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap @@ -1,5 +1,15 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`error transform handles deeply nested expressions 1`] = ` +"var val = (a, (b, // eslint-disable-next-line react-internal/prod-error-codes +new Error('foo')));" +`; + +exports[`error transform handles deeply nested expressions 2`] = ` +"var val = (a, ( // eslint-disable-next-line react-internal/prod-error-codes +b, new Error('foo')));" +`; + exports[`error transform handles escaped backticks in template string 1`] = ` "import _formatProdErrorMessage from \\"shared/formatProdErrorMessage\\"; Error(_formatProdErrorMessage(231, listener, type));" diff --git a/scripts/error-codes/__tests__/transform-error-messages.js b/scripts/error-codes/__tests__/transform-error-messages.js index 4283428e9fe..e3f761596a9 100644 --- a/scripts/error-codes/__tests__/transform-error-messages.js +++ b/scripts/error-codes/__tests__/transform-error-messages.js @@ -133,6 +133,27 @@ let val = someBool : someOtherBool ? new Error('bar') : new Error('baz'); +`) + ).toMatchSnapshot(); + }); + + it('handles deeply nested expressions', () => { + expect( + transform(` +let val = + (a, + (b, + // eslint-disable-next-line react-internal/prod-error-codes + new Error('foo'))); +`) + ).toMatchSnapshot(); + + expect( + transform(` +let val = + (a, + // eslint-disable-next-line react-internal/prod-error-codes + (b, new Error('foo'))); `) ).toMatchSnapshot(); }); diff --git a/scripts/error-codes/transform-error-messages.js b/scripts/error-codes/transform-error-messages.js index 924a7b14d18..18cd4e06852 100644 --- a/scripts/error-codes/transform-error-messages.js +++ b/scripts/error-codes/transform-error-messages.js @@ -62,16 +62,21 @@ module.exports = function(babel) { // throw Error(`A ${adj} message that contains ${noun}`); // } - let leadingComments = node.leadingComments; + let leadingComments = []; const statementParent = path.getStatementParent(); - const parentLeadingComments = statementParent.node.leadingComments; - - if (parentLeadingComments) { - leadingComments = leadingComments - ? leadingComments.concat(parentLeadingComments) - : parentLeadingComments; + let nextPath = path; + while (true) { + let nextNode = nextPath.node; + if (nextNode.leadingComments) { + leadingComments.push(...nextNode.leadingComments); + } + if (nextPath === statementParent) { + break; + } + nextPath = nextPath.parentPath; } + if (leadingComments !== undefined) { for (let i = 0; i < leadingComments.length; i++) { // TODO: Since this only detects one of many ways to disable a lint