Skip to content

Commit

Permalink
fix: react-cons-elem should not hoist router comp (#14828)
Browse files Browse the repository at this point in the history
  • Loading branch information
JLHwung committed Aug 4, 2022
1 parent bee081d commit 7fce737
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 7 deletions.
@@ -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[];
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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;
Expand All @@ -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`
Expand Down
@@ -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>;
}
@@ -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>;
}

0 comments on commit 7fce737

Please sign in to comment.