Skip to content

fix: Replace body so fast-refresh function caching works #628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 16, 2024
Merged

fix: Replace body so fast-refresh function caching works #628

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b113275
fix: Replace body so fast-refresh function caching works
Artur- Dec 15, 2024
f9bf1af
Simplify
Artur- Dec 15, 2024
ea534bf
Fix test based on https://github.com/babel/babel/issues/8358#issuecom…
Artur- Dec 16, 2024
99f43af
Merge branch 'main' into fast-refresh
Artur- Dec 16, 2024
d14db48
Create thirty-lies-destroy.md
JoviDeCroock Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 11 additions & 20 deletions packages/react-transform/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,11 +342,10 @@ function wrapInTryFinally(
path: NodePath<FunctionLike>,
state: PluginPass,
hookUsage: HookUsage
): FunctionLike {
): BabelTypes.BlockStatement {
const stopTrackingIdentifier = path.scope.generateUidIdentifier("effect");

const newFunction = t.cloneNode(path.node);
newFunction.body = t.blockStatement(
return t.blockStatement(
tryCatchTemplate({
STORE_IDENTIFIER: stopTrackingIdentifier,
HOOK_IDENTIFIER: get(state, getHookIdentifier)(),
Expand All @@ -356,29 +355,26 @@ function wrapInTryFinally(
: t.returnStatement(path.node.body),
})
);

return newFunction;
}

function prependUseSignals<T extends FunctionLike>(
t: typeof BabelTypes,
path: NodePath<T>,
state: PluginPass
): T {
const newFunction = t.cloneNode(path.node);
newFunction.body = t.blockStatement([
): BabelTypes.BlockStatement {
const body = t.blockStatement([
t.expressionStatement(
t.callExpression(get(state, getHookIdentifier)(), [])
),
]);
if (t.isBlockStatement(path.node.body)) {
// TODO: Is it okay to elide the block statement here?
newFunction.body.body.push(...path.node.body.body);
body.body.push(...path.node.body.body);
} else {
newFunction.body.body.push(t.returnStatement(path.node.body));
body.body.push(t.returnStatement(path.node.body));
}

return newFunction;
return body;
}

function transformFunction(
Expand All @@ -398,20 +394,15 @@ function transformFunction(
? MANAGED_COMPONENT
: UNMANAGED;

let newFunction: FunctionLike;
let newBody: BabelTypes.BlockStatement;
if (hookUsage !== UNMANAGED) {
newFunction = wrapInTryFinally(t, path, state, hookUsage);
newBody = wrapInTryFinally(t, path, state, hookUsage);
} else {
newFunction = prependUseSignals(t, path, state);
newBody = prependUseSignals(t, path, state);
}

// Using replaceWith keeps the existing leading comments already so
// we'll clear our cloned node's leading comments to ensure they
// aren't duplicated in the output.
newFunction.leadingComments = [];

setData(path, alreadyTransformed, true);
path.replaceWith(newFunction);
path.get("body").replaceWith(newBody);
}

function createImportLazily(
Expand Down
1 change: 1 addition & 0 deletions packages/react-transform/test/node/index.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,7 @@ describe("React Signals Babel Transform", () => {
};
`);

scope.path.scope.crawl();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to babel/babel#8358, the scope bindings are not automatically updated when modifying the ast - this seems to update them

const signalsBinding = scope.bindings["_useSignals"];
expect(signalsBinding).to.exist;
expect(signalsBinding.kind).to.equal("module");
Expand Down
Loading