Skip to content

Commit

Permalink
fix(react-signals-transform): Support nested functions accessing sign…
Browse files Browse the repository at this point in the history
…als (#582)
  • Loading branch information
JoviDeCroock authored Jul 6, 2024
1 parent 931404e commit 4fa8603
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .changeset/tame-bobcats-invent.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@preact/signals-react-transform": patch
---

Support nested scopes like a component accessing an array of signals
48 changes: 43 additions & 5 deletions packages/react-transform/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
template,
} from "@babel/core";
import { isModule, addNamed } from "@babel/helper-module-imports";
import type { VisitNodeObject } from "@babel/traverse";
import type { Scope, VisitNodeObject } from "@babel/traverse";
import debug from "debug";

interface PluginArgs {
Expand Down Expand Up @@ -52,8 +52,46 @@ const setData = (node: DataContainer, name: string, value: any) =>
const getData = (node: DataContainer, name: string) =>
node.getData(`${dataNamespace}/${name}`);

function setOnFunctionScope(path: NodePath, key: string, value: any) {
function getComponentFunctionDeclaration(
path: NodePath,
filename: string | undefined,
prev?: Scope
): Scope | null {
const functionScope = path.scope.getFunctionParent();

if (functionScope) {
const parent = functionScope.path.parent;
let functionName = getFunctionName(functionScope.path as any);
if (functionName === DefaultExportSymbol) {
functionName = filename || null;
}
if (isComponentFunction(functionScope.path as any, functionName)) {
return functionScope;
} else if (
parent.type === "CallExpression" &&
parent.callee.type === "Identifier" &&
parent.callee.name.startsWith("use") &&
parent.callee.name[3] === parent.callee.name[3].toUpperCase()
) {
return null;
}
return getComponentFunctionDeclaration(
functionScope.parent.path,
filename,
functionScope
);
} else {
return prev || null;
}
}

function setOnFunctionScope(
path: NodePath,
key: string,
value: any,
filename: string | undefined
) {
const functionScope = getComponentFunctionDeclaration(path, filename);
if (functionScope) {
setData(functionScope, key, value);
}
Expand Down Expand Up @@ -538,15 +576,15 @@ export default function signalsTransform(

MemberExpression(path) {
if (isValueMemberExpression(path)) {
setOnFunctionScope(path, maybeUsesSignal, true);
setOnFunctionScope(path, maybeUsesSignal, true, this.filename);
}
},

JSXElement(path) {
setOnFunctionScope(path, containsJSX, true);
setOnFunctionScope(path, containsJSX, true, this.filename);
},
JSXFragment(path) {
setOnFunctionScope(path, containsJSX, true);
setOnFunctionScope(path, containsJSX, true, this.filename);
},
},
};
Expand Down
20 changes: 20 additions & 0 deletions packages/react-transform/test/node/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,26 @@ describe("React Signals Babel Transform", () => {
experimental: { noTryFinally: true },
});
});

it("recursively propogates `.value` reads to parent component", () => {
const inputCode = `
function MyComponent() {
return <div>{new Array(20).fill(null).map(() => signal.value)}</div>;
}
`;

const expectedOutput = `
import { useSignals as _useSignals } from "@preact/signals-react/runtime";
function MyComponent() {
_useSignals();
return <div>{new Array(20).fill(null).map(() => signal.value)}</div>;
}
`;

runTest(inputCode, expectedOutput, {
experimental: { noTryFinally: true },
});
});
});

describe("importSource option", () => {
Expand Down

0 comments on commit 4fa8603

Please sign in to comment.