From 7fce7373181789b4d417e6b0bdf28becf856d230 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hu=C3=A1ng=20J=C3=B9nli=C3=A0ng?= <jlhwung@gmail.com>
Date: Thu, 4 Aug 2022 17:28:05 -0400
Subject: [PATCH] fix: react-cons-elem should not hoist router comp (#14828)

---
 .../src/index.ts                              | 14 ++++++-------
 .../regression-14363-2/input.mjs              | 21 +++++++++++++++++++
 .../regression-14363-2/output.mjs             | 21 +++++++++++++++++++
 3 files changed, 49 insertions(+), 7 deletions(-)
 create mode 100644 packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/regression-14363-2/input.mjs
 create mode 100644 packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/regression-14363-2/output.mjs

diff --git a/packages/babel-plugin-transform-react-constant-elements/src/index.ts b/packages/babel-plugin-transform-react-constant-elements/src/index.ts
index e83dc03bc9dd..dd664dca7544 100644
--- a/packages/babel-plugin-transform-react-constant-elements/src/index.ts
+++ b/packages/babel-plugin-transform-react-constant-elements/src/index.ts
@@ -1,6 +1,6 @@
 import { declare } from "@babel/helper-plugin-utils";
 import { types as t, template } from "@babel/core";
-import type { Visitor, Scope } from "@babel/traverse";
+import type { Visitor, Scope, NodePath } from "@babel/traverse";
 
 export interface Options {
   allowMutablePropsOnTags?: null | string[];
@@ -165,8 +165,6 @@ export default declare((api, options: Options) => {
     visitor: {
       JSXElement(path) {
         if (HOISTED.has(path.node)) return;
-        HOISTED.set(path.node, path.scope);
-
         const name = path.node.openingElement.name;
 
         // This transform takes the option `allowMutablePropsOnTags`, which is an array
@@ -191,13 +189,15 @@ export default declare((api, options: Options) => {
         // current element has already been hoisted, we can consider its target
         // scope as the base scope for the current element.
         let jsxScope;
-        let current = path;
+        let current: NodePath<t.JSX> = path;
         while (!jsxScope && current.parentPath.isJSX()) {
-          // @ts-expect-error current is a search pointer
           current = current.parentPath;
           jsxScope = HOISTED.get(current.node);
         }
         jsxScope ??= path.scope;
+        // The initial HOISTED is set to jsxScope, s.t.
+        // if the element's JSX ancestor has been hoisted, it will be skipped
+        HOISTED.set(path.node, jsxScope);
 
         const visitorState: VisitorState = {
           isImmutable: true,
@@ -209,8 +209,6 @@ export default declare((api, options: Options) => {
         if (!visitorState.isImmutable) return;
 
         const { targetScope } = visitorState;
-        HOISTED.set(path.node, targetScope);
-
         // Only hoist if it would give us an advantage.
         for (let currentScope = jsxScope; ; ) {
           if (targetScope === currentScope) return;
@@ -228,6 +226,8 @@ export default declare((api, options: Options) => {
 
         const id = path.scope.generateUidBasedOnNode(name);
         targetScope.push({ id: t.identifier(id) });
+        // If the element is to be hoisted, update HOISTED to be the target scope
+        HOISTED.set(path.node, targetScope);
 
         let replacement: t.Expression | t.JSXExpressionContainer = template
           .expression.ast`
diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/regression-14363-2/input.mjs b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/regression-14363-2/input.mjs
new file mode 100644
index 000000000000..12292f44d732
--- /dev/null
+++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/regression-14363-2/input.mjs
@@ -0,0 +1,21 @@
+import { Routes, Route } from "react-router";
+import { router } from "common/router";
+
+function RoutesComponent() {
+  return <Routes>
+      {Object.keys(router).map(routerKey => {
+      const route = router[routerKey];
+
+      if (route && route.element) {
+        const {
+          path,
+          element: Component
+        } = route;
+        // Component should not be hoisted
+        return <Route key={routerKey} path={path} element={<Component />} />;
+      } else {
+        return null;
+      }
+    }).filter(Boolean)}
+    </Routes>;
+}
diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/regression-14363-2/output.mjs b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/regression-14363-2/output.mjs
new file mode 100644
index 000000000000..fdef8dce5fa2
--- /dev/null
+++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/regression-14363-2/output.mjs
@@ -0,0 +1,21 @@
+import { Routes, Route } from "react-router";
+import { router } from "common/router";
+
+function RoutesComponent() {
+  return <Routes>
+      {Object.keys(router).map(routerKey => {
+      const route = router[routerKey];
+
+      if (route && route.element) {
+        const {
+          path,
+          element: Component
+        } = route; // Component should not be hoisted
+
+        return <Route key={routerKey} path={path} element={<Component />} />;
+      } else {
+        return null;
+      }
+    }).filter(Boolean)}
+    </Routes>;
+}