From a21660de006416018d1fc799be92540417fcb8bb Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 14 Jun 2023 14:04:26 -0700 Subject: [PATCH] Fix calculation of nested function dependencies We were incorrectly calculating the dependencies of nested lambdas, because the "component" scope was incorrectly set to the next closest parent rather than outermost React function (component/hook) being compiled. --- .../src/HIR/BuildHIR.ts | 2 +- .../fixtures/compiler/repro.expect.md | 64 +++++++++++++++++++ .../src/__tests__/fixtures/compiler/repro.js | 14 ++++ 3 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro.expect.md create mode 100644 compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro.js diff --git a/compiler/forget/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts b/compiler/forget/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts index 528b3c8a57..5fa69ae6b5 100644 --- a/compiler/forget/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts +++ b/compiler/forget/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts @@ -2344,7 +2344,7 @@ function lowerFunctionExpression( if (expr.isFunctionExpression()) { name = expr.get("id")?.node?.name ?? null; } - const componentScope: Scope = expr.scope.parent.getFunctionParent()!; + const componentScope: Scope = builder.parentFunction.scope; const captured = gatherCapturedDeps(builder, expr, componentScope); // TODO(gsn): In the future, we could only pass in the context identifiers diff --git a/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro.expect.md b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro.expect.md new file mode 100644 index 0000000000..104f846e05 --- /dev/null +++ b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro.expect.md @@ -0,0 +1,64 @@ + +## Input + +```javascript +function Component(props) { + const item = props.item; + const thumbnails = []; + const baseVideos = getBaseVideos(item); + useMemo(() => { + baseVideos.forEach((video) => { + const baseVideo = video.hasBaseVideo; + if (Boolean(baseVideo)) { + thumbnails.push({ extraVideo: true }); + } + }); + }); + return ; +} + +``` + +## Code + +```javascript +import { unstable_useMemoCache as useMemoCache } from "react"; +function Component(props) { + const $ = useMemoCache(6); + const item = props.item; + const c_0 = $[0] !== item; + let baseVideos; + let thumbnails; + if (c_0) { + thumbnails = []; + baseVideos = getBaseVideos(item); + + baseVideos.forEach((video) => { + const baseVideo = video.hasBaseVideo; + if (Boolean(baseVideo)) { + thumbnails.push({ extraVideo: true }); + } + }); + $[0] = item; + $[1] = baseVideos; + $[2] = thumbnails; + } else { + baseVideos = $[1]; + thumbnails = $[2]; + } + const c_3 = $[3] !== baseVideos; + const c_4 = $[4] !== thumbnails; + let t0; + if (c_3 || c_4) { + t0 = ; + $[3] = baseVideos; + $[4] = thumbnails; + $[5] = t0; + } else { + t0 = $[5]; + } + return t0; +} + +``` + \ No newline at end of file diff --git a/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro.js b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro.js new file mode 100644 index 0000000000..37134b73d0 --- /dev/null +++ b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro.js @@ -0,0 +1,14 @@ +function Component(props) { + const item = props.item; + const thumbnails = []; + const baseVideos = getBaseVideos(item); + useMemo(() => { + baseVideos.forEach((video) => { + const baseVideo = video.hasBaseVideo; + if (Boolean(baseVideo)) { + thumbnails.push({ extraVideo: true }); + } + }); + }); + return ; +}