From dcb6549ddae8c10a826503222d6bd7c2dfb770a4 Mon Sep 17 00:00:00 2001 From: Sathya Gunasekaran Date: Tue, 25 Jul 2023 16:08:32 +0100 Subject: [PATCH] [hir] Bailout when reading from React namespace Forget doesn't understand the React namespace object and generates incorrect code when compiling code that loads props from this namespace object. This PR makes Forget bailout when we see a property load from React namespace object. --- .../src/HIR/BuildHIR.ts | 14 +++ .../_bug.hooks-with-React-namespace.expect.md | 30 ------- ...xisting-react-kitchensink-import.expect.md | 88 ++++++++----------- ...babel-existing-react-kitchensink-import.js | 6 +- ...-existing-react-namespace-import.expect.md | 72 --------------- ...-existing-react-namespace-import.expect.md | 32 +++++++ ....babel-existing-react-namespace-import.js} | 0 ...error.hooks-with-React-namespace.expect.md | 19 ++++ ...js => error.hooks-with-React-namespace.js} | 0 9 files changed, 106 insertions(+), 155 deletions(-) delete mode 100644 compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/_bug.hooks-with-React-namespace.expect.md delete mode 100644 compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/babel-existing-react-namespace-import.expect.md create mode 100644 compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.babel-existing-react-namespace-import.expect.md rename compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/{babel-existing-react-namespace-import.js => error.babel-existing-react-namespace-import.js} (100%) create mode 100644 compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.hooks-with-React-namespace.expect.md rename compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/{_bug.hooks-with-React-namespace.js => error.hooks-with-React-namespace.js} (100%) 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 f5244c6aab..dfdd840a21 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 @@ -2215,6 +2215,20 @@ function lowerMemberExpression( const object = loweredObject ?? lowerExpressionToTemporary(builder, objectNode); + if (objectNode.isIdentifier() && objectNode.node.name === "React") { + builder.errors.push({ + reason: `(BuildHIR::lowerMemberExpression) Handle loading properties from React namespace`, + severity: ErrorSeverity.Todo, + loc: propertyNode.node.loc ?? null, + suggestions: null, + }); + return { + object, + property: propertyNode.toString(), + value: { kind: "UnsupportedNode", node: exprNode, loc: exprLoc }, + }; + } + if (!expr.node.computed) { if (!propertyNode.isIdentifier()) { builder.errors.push({ diff --git a/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/_bug.hooks-with-React-namespace.expect.md b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/_bug.hooks-with-React-namespace.expect.md deleted file mode 100644 index 2b2384dede..0000000000 --- a/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/_bug.hooks-with-React-namespace.expect.md +++ /dev/null @@ -1,30 +0,0 @@ - -## Input - -```javascript -function Foo() { - const [x, setX] = React.useState(1); - return x; -} - -``` - -## Code - -```javascript -import { unstable_useMemoCache as useMemoCache } from "react"; -function Foo() { - const $ = useMemoCache(1); - let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - t0 = React.useState(1); - $[0] = t0; - } else { - t0 = $[0]; - } - const [x] = t0; - return x; -} - -``` - \ No newline at end of file diff --git a/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/babel-existing-react-kitchensink-import.expect.md b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/babel-existing-react-kitchensink-import.expect.md index 3ff22edbed..08d7f400a4 100644 --- a/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/babel-existing-react-kitchensink-import.expect.md +++ b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/babel-existing-react-kitchensink-import.expect.md @@ -3,18 +3,18 @@ ```javascript import * as React from "react"; -import { useState } from "react"; +import { useState, useMemo } from "react"; function Component(props) { const [x] = useState(0); - const expensiveNumber = React.useMemo(() => calculateExpensiveNumber(x), [x]); + const expensiveNumber = useMemo(() => calculateExpensiveNumber(x), [x]); return
{expensiveNumber}
; } function Component2(props) { const [x] = useState(0); - const expensiveNumber = React.useMemo(() => calculateExpensiveNumber(x), [x]); + const expensiveNumber = useMemo(() => calculateExpensiveNumber(x), [x]); return
{expensiveNumber}
; } @@ -25,74 +25,62 @@ function Component2(props) { ```javascript import * as React from "react"; -import { useState, unstable_useMemoCache as useMemoCache } from "react"; +import { + useState, + useMemo, + unstable_useMemoCache as useMemoCache, +} from "react"; function Component(props) { - const $ = useMemoCache(6); + const $ = useMemoCache(4); const [x] = useState(0); const c_0 = $[0] !== x; - let t1; + let t0; if (c_0) { - const c_2 = $[2] !== x; - let t0; - if (c_2) { - t0 = () => calculateExpensiveNumber(x); - $[2] = x; - $[3] = t0; - } else { - t0 = $[3]; - } - t1 = React.useMemo(t0, [x]); + t0 = calculateExpensiveNumber(x); $[0] = x; - $[1] = t1; + $[1] = t0; } else { - t1 = $[1]; + t0 = $[1]; } - const expensiveNumber = t1; - const c_4 = $[4] !== expensiveNumber; - let t2; - if (c_4) { - t2 =
{expensiveNumber}
; - $[4] = expensiveNumber; - $[5] = t2; + const t15 = t0; + const expensiveNumber = t15; + const c_2 = $[2] !== expensiveNumber; + let t1; + if (c_2) { + t1 =
{expensiveNumber}
; + $[2] = expensiveNumber; + $[3] = t1; } else { - t2 = $[5]; + t1 = $[3]; } - return t2; + return t1; } function Component2(props) { - const $ = useMemoCache(6); + const $ = useMemoCache(4); const [x] = useState(0); const c_0 = $[0] !== x; - let t1; + let t0; if (c_0) { - const c_2 = $[2] !== x; - let t0; - if (c_2) { - t0 = () => calculateExpensiveNumber(x); - $[2] = x; - $[3] = t0; - } else { - t0 = $[3]; - } - t1 = React.useMemo(t0, [x]); + t0 = calculateExpensiveNumber(x); $[0] = x; - $[1] = t1; + $[1] = t0; } else { - t1 = $[1]; + t0 = $[1]; } - const expensiveNumber = t1; - const c_4 = $[4] !== expensiveNumber; - let t2; - if (c_4) { - t2 =
{expensiveNumber}
; - $[4] = expensiveNumber; - $[5] = t2; + const t15 = t0; + const expensiveNumber = t15; + const c_2 = $[2] !== expensiveNumber; + let t1; + if (c_2) { + t1 =
{expensiveNumber}
; + $[2] = expensiveNumber; + $[3] = t1; } else { - t2 = $[5]; + t1 = $[3]; } - return t2; + return t1; } ``` diff --git a/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/babel-existing-react-kitchensink-import.js b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/babel-existing-react-kitchensink-import.js index 4880c50eb2..39bcae9d69 100644 --- a/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/babel-existing-react-kitchensink-import.js +++ b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/babel-existing-react-kitchensink-import.js @@ -1,16 +1,16 @@ import * as React from "react"; -import { useState } from "react"; +import { useState, useMemo } from "react"; function Component(props) { const [x] = useState(0); - const expensiveNumber = React.useMemo(() => calculateExpensiveNumber(x), [x]); + const expensiveNumber = useMemo(() => calculateExpensiveNumber(x), [x]); return
{expensiveNumber}
; } function Component2(props) { const [x] = useState(0); - const expensiveNumber = React.useMemo(() => calculateExpensiveNumber(x), [x]); + const expensiveNumber = useMemo(() => calculateExpensiveNumber(x), [x]); return
{expensiveNumber}
; } diff --git a/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/babel-existing-react-namespace-import.expect.md b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/babel-existing-react-namespace-import.expect.md deleted file mode 100644 index 8128495d2e..0000000000 --- a/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/babel-existing-react-namespace-import.expect.md +++ /dev/null @@ -1,72 +0,0 @@ - -## Input - -```javascript -import * as React from "react"; - -function Component(props) { - const [x] = React.useState(0); - const expensiveNumber = React.useMemo(() => calculateExpensiveNumber(x), [x]); - - return
{expensiveNumber}
; -} - -function Component2(props) { - const [x] = React.useState(0); - const expensiveNumber = React.useMemo(() => calculateExpensiveNumber(x), [x]); - - return
{expensiveNumber}
; -} - -``` - -## Code - -```javascript -import { unstable_useMemoCache as useMemoCache } from "react"; -import * as React from "react"; - -function Component(props) { - const $ = useMemoCache(2); - let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - const [x] = React.useState(0); - t0 = React.useMemo(() => calculateExpensiveNumber(x), [x]); - $[0] = t0; - } else { - t0 = $[0]; - } - const expensiveNumber = t0; - let t1; - if ($[1] === Symbol.for("react.memo_cache_sentinel")) { - t1 =
{expensiveNumber}
; - $[1] = t1; - } else { - t1 = $[1]; - } - return t1; -} - -function Component2(props) { - const $ = useMemoCache(2); - let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - const [x] = React.useState(0); - t0 = React.useMemo(() => calculateExpensiveNumber(x), [x]); - $[0] = t0; - } else { - t0 = $[0]; - } - const expensiveNumber = t0; - let t1; - if ($[1] === Symbol.for("react.memo_cache_sentinel")) { - t1 =
{expensiveNumber}
; - $[1] = t1; - } else { - t1 = $[1]; - } - return t1; -} - -``` - \ No newline at end of file diff --git a/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.babel-existing-react-namespace-import.expect.md b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.babel-existing-react-namespace-import.expect.md new file mode 100644 index 0000000000..14c60a6ec2 --- /dev/null +++ b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.babel-existing-react-namespace-import.expect.md @@ -0,0 +1,32 @@ + +## Input + +```javascript +import * as React from "react"; + +function Component(props) { + const [x] = React.useState(0); + const expensiveNumber = React.useMemo(() => calculateExpensiveNumber(x), [x]); + + return
{expensiveNumber}
; +} + +function Component2(props) { + const [x] = React.useState(0); + const expensiveNumber = React.useMemo(() => calculateExpensiveNumber(x), [x]); + + return
{expensiveNumber}
; +} + +``` + + +## Error + +``` +[ReactForget] Todo: (BuildHIR::lowerMemberExpression) Handle loading properties from React namespace (4:4) + +[ReactForget] Todo: (BuildHIR::lowerMemberExpression) Handle loading properties from React namespace (5:5) +``` + + \ No newline at end of file diff --git a/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/babel-existing-react-namespace-import.js b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.babel-existing-react-namespace-import.js similarity index 100% rename from compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/babel-existing-react-namespace-import.js rename to compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.babel-existing-react-namespace-import.js diff --git a/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.hooks-with-React-namespace.expect.md b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.hooks-with-React-namespace.expect.md new file mode 100644 index 0000000000..6e9f92625a --- /dev/null +++ b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.hooks-with-React-namespace.expect.md @@ -0,0 +1,19 @@ + +## Input + +```javascript +function Foo() { + const [x, setX] = React.useState(1); + return x; +} + +``` + + +## Error + +``` +[ReactForget] Todo: (BuildHIR::lowerMemberExpression) Handle loading properties from React namespace (2:2) +``` + + \ No newline at end of file diff --git a/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/_bug.hooks-with-React-namespace.js b/compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.hooks-with-React-namespace.js similarity index 100% rename from compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/_bug.hooks-with-React-namespace.js rename to compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.hooks-with-React-namespace.js