Avoid reusing AST nodes (#4309)

Summary: Reusing AST nodes can result in subtle bugs, for example [this issue](#3856) is caused by AST node reuse breaking [memoization](https://github.com/babel/babel/blob/main/packages/babel-helper-module-transforms/src/rewrite-live-references.ts#L275-L276) in the `babel/plugin-transform-modules-commonjs` plugin. The first commit here introduces [babel/helper-check-duplicated-nodes](https://github.com/babel/babel/tree/main/packages/babel-helper-check-duplicate-nodes) which breaks a number of tests. The second commit fixes those tests. Fixes #3856. Pull Request resolved: #4309 Reviewed By: voideanvalue Differential Revision: D45750523 Pulled By: captbaritone fbshipit-source-id: 7b962a4fed309a1cc85e00fc6a9d0442d2bcd8e2
facebook · May 22, 2023 · 39ebc1c · 39ebc1c
1 parent d1dfe1f
commit 39ebc1c
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 13 deletions.
diff --git a/package.json b/package.json
@@ -26,6 +26,7 @@
   "dependencies": {
     "@babel/core": "^7.18.6",
     "@babel/generator": "^7.18.6",
+    "@babel/helper-check-duplicate-nodes": "^7.18.6",
     "@babel/parser": "^7.18.6",
     "@babel/plugin-proposal-nullish-coalescing-operator": "^7.0.0",
     "@babel/plugin-proposal-optional-catch-binding": "^7.0.0",

diff --git a/packages/babel-plugin-relay/__tests__/transformerWithOptions.js b/packages/babel-plugin-relay/__tests__/transformerWithOptions.js
@@ -12,6 +12,8 @@
 
 const BabelPluginRelay = require('../BabelPluginRelay');
 const babel = require('@babel/core');
+const checkDuplicatedNodes =
+  require('@babel/helper-check-duplicate-nodes').default;
 const prettier = require('prettier');
 
 function transformerWithOptions(
@@ -23,14 +25,16 @@ function transformerWithOptions(
     const previousEnv = process.env.BABEL_ENV;
     try {
       process.env.BABEL_ENV = environment;
-      const code = babel.transform(text, {
+      const {code, ast} = babel.transformSync(text, {
         compact: false,
         cwd: '/',
         filename: filename || providedFileName || 'test.js',
         highlightCode: false,
         parserOpts: {plugins: ['jsx']},
         plugins: [[BabelPluginRelay, options]],
-      }).code;
+        ast: true,
+      });
+      checkDuplicatedNodes(ast);
       return prettier.format(code, {
         bracketSameLine: true,
         bracketSpacing: false,

diff --git a/packages/babel-plugin-relay/compileGraphQLTag.js b/packages/babel-plugin-relay/compileGraphQLTag.js
@@ -125,11 +125,11 @@ function createNode(
 
   const id = topScope.generateUidIdentifier(definitionName);
 
-  const expHash = t.MemberExpression(id, t.Identifier('hash'));
+  const expHash = t.MemberExpression(t.cloneNode(id), t.Identifier('hash'));
   const expWarn = warnNeedsRebuild(t, definitionName, options.buildCommand);
   const expWarnIfOutdated = t.LogicalExpression(
     '&&',
-    expHash,
+    t.cloneNode(expHash),
     t.LogicalExpression(
       '&&',
       t.BinaryExpression('!==', expHash, t.StringLiteral(hash)),
@@ -145,34 +145,41 @@ function createNode(
     const program = path.findParent(parent => parent.isProgram());
     program.unshiftContainer('body', importDeclaration);
 
-    const expAssignAndCheck = t.SequenceExpression([expWarnIfOutdated, id]);
+    const expAssignAndCheck = t.SequenceExpression([
+      expWarnIfOutdated,
+      t.cloneNode(id),
+    ]);
 
     let expAssign;
     if (options.isDevVariable != null) {
       expAssign = t.ConditionalExpression(
         t.Identifier(options.isDevVariable),
         expAssignAndCheck,
-        id,
+        t.cloneNode(id),
       );
     } else if (options.isDevelopment) {
       expAssign = expAssignAndCheck;
     } else {
-      expAssign = id;
+      expAssign = t.cloneNode(id);
     }
 
     path.replaceWith(expAssign);
   } else {
-    topScope.push({id});
+    topScope.push({id: t.cloneNode(id)});
 
     const requireGraphQLModule = t.CallExpression(t.Identifier('require'), [
       t.StringLiteral(requiredPath),
     ]);
 
-    const expAssignProd = t.AssignmentExpression('=', id, requireGraphQLModule);
+    const expAssignProd = t.AssignmentExpression(
+      '=',
+      t.cloneNode(id),
+      requireGraphQLModule,
+    );
     const expAssignAndCheck = t.SequenceExpression([
       expAssignProd,
       expWarnIfOutdated,
-      id,
+      t.cloneNode(id),
     ]);
 
     let expAssign;
@@ -191,9 +198,9 @@ function createNode(
     const expVoid0 = t.UnaryExpression('void', t.NumericLiteral(0));
     path.replaceWith(
       t.ConditionalExpression(
-        t.BinaryExpression('!==', id, expVoid0),
-        id,
-        expAssign,
+        t.BinaryExpression('!==', t.cloneNode(id), expVoid0),
+        t.cloneNode(id),
+        t.cloneNode(expAssign),
       ),
     );
   }

diff --git a/yarn.lock b/yarn.lock
@@ -59,6 +59,13 @@
   dependencies:
     "@babel/types" "^7.18.6"
 
+"@babel/helper-check-duplicate-nodes@^7.18.6":
+  version "7.18.6"
+  resolved "https://registry.yarnpkg.com/@babel/helper-check-duplicate-nodes/-/helper-check-duplicate-nodes-7.18.6.tgz#776dcf6834a9ea1162530069000218bc44e1c11e"
+  integrity sha512-gQkMniJ8+GfcRk98xGNlBXSEuWOJpEb7weoHDJpw4xxQrikPRvO1INlqbCM6LynKYKVZ/suN869xudV5DzAkzQ==
+  dependencies:
+    "@babel/types" "^7.18.6"
+
 "@babel/helper-compilation-targets@^7.13.0", "@babel/helper-compilation-targets@^7.18.9":
   version "7.18.9"
   resolved "https://registry.yarnpkg.com/@babel/helper-compilation-targets/-/helper-compilation-targets-7.18.9.tgz#69e64f57b524cde3e5ff6cc5a9f4a387ee5563bf"