Update error transform to allow excluding errors inside subexpression…

…s like ternaries (#24693) * Update error transform to allow excluding errors inside subexpressions like ternaries * make leadingcomments aggregation walk the expression stack
facebook · Jun 8, 2022 · 9e3b772 · 9e3b772
1 parent 3e92eb0
commit 9e3b772
Show file tree

Hide file tree

Showing 3 changed files with 86 additions and 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
@@ -1,10 +1,34 @@
 // 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));"
 `;
 
+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!*/
+
+/*! <expected-error-format>\\"bar\\"</expected-error-format>*/
+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
@@ -106,6 +106,54 @@ 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();
+  });
+
+  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
@@ -62,8 +62,21 @@ module.exports = function(babel) {
       //     throw Error(`A ${adj} message that contains ${noun}`);
       //   }
 
+      let leadingComments = [];
+
       const statementParent = path.getStatementParent();
-      const leadingComments = statementParent.node.leadingComments;
+      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