From 59504e1cb49733d00e4d1613cfd712c5d29c7bfb Mon Sep 17 00:00:00 2001 From: Sathya Gunasekaran Date: Wed, 6 Sep 2023 14:58:04 +0100 Subject: [PATCH] [hir] Traverse function to capture deps, not just body node Technically there is a body node created for implicit return expression in a arrow function, so the existing logic should've worked fine. But there seems to be a Babel bug, so let's work around it by traversing the function. Added a test case that captures a dep as a param -- this is currently unsupported and also something that would've been ignored before this PR. Added a failing test to make sure we think about this case when we add support for default params. --- .../src/HIR/BuildHIR.ts | 2 +- ...ction-with-param-as-captured-dep.expect.md | 29 +++++++++++++++++++ ...ted-function-with-param-as-captured-dep.ts | 14 +++++++++ ....md => lambda-return-expression.expect.md} | 19 +++++++++--- ...ression.ts => lambda-return-expression.ts} | 0 5 files changed, 59 insertions(+), 5 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.nested-function-with-param-as-captured-dep.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.nested-function-with-param-as-captured-dep.ts rename compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/{error.todo-lambda-return-expression.expect.md => lambda-return-expression.expect.md} (52%) rename compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/{error.todo-lambda-return-expression.ts => lambda-return-expression.ts} (100%) diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts index 2edcf18e32..ed19e7ed76 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts @@ -3058,7 +3058,7 @@ function gatherCapturedDeps( } } - fn.get("body").traverse({ + fn.traverse({ Expression(path) { visit(path); }, diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.nested-function-with-param-as-captured-dep.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.nested-function-with-param-as-captured-dep.expect.md new file mode 100644 index 0000000000..0390e8e9eb --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.nested-function-with-param-as-captured-dep.expect.md @@ -0,0 +1,29 @@ + +## Input + +```javascript +function Foo() { + (function t() { + let x = {}; + return function a(x = () => {}) { + return x; + }; + })(); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [], + isComponent: false, +}; + +``` + + +## Error + +``` +[ReactForget] Todo: (BuildHIR::node.lowerReorderableExpression) Expression type 'ArrowFunctionExpression' cannot be safely reordered (4:4) +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.nested-function-with-param-as-captured-dep.ts b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.nested-function-with-param-as-captured-dep.ts new file mode 100644 index 0000000000..c01ab3a084 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.nested-function-with-param-as-captured-dep.ts @@ -0,0 +1,14 @@ +function Foo() { + (function t() { + let x = {}; + return function a(x = () => {}) { + return x; + }; + })(); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [], + isComponent: false, +}; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-lambda-return-expression.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/lambda-return-expression.expect.md similarity index 52% rename from compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-lambda-return-expression.expect.md rename to compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/lambda-return-expression.expect.md index 42607b8aef..443d69faed 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-lambda-return-expression.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/lambda-return-expression.expect.md @@ -18,11 +18,22 @@ export const FIXTURE_ENTRYPOINT = { ``` +## Code -## Error +```javascript +import { invoke } from "shared-runtime"; + +function useFoo() { + const x = {}; + const result = invoke(() => x); + console.log(result); +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [], + isComponent: false, +}; ``` -[ReactForget] Invariant: Expected value for identifier `16` to be initialized. (5:5) -``` - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-lambda-return-expression.ts b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/lambda-return-expression.ts similarity index 100% rename from compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-lambda-return-expression.ts rename to compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/lambda-return-expression.ts