From 9bdcce14e61c0a38adb0a1acdde7b405c10149d4 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 14 Jun 2024 08:38:57 -0700 Subject: [PATCH 1/2] [compiler] Support method-call version of macro functions [ghstack-poisoned] --- .../src/Entrypoint/Pipeline.ts | 2 +- .../MemoizeFbtAndMacroOperandsInSameScope.ts | 7 +- ...-namespace-assigned-to-temporary.expect.md | 100 ++++++++++++++++++ ...epro-cx-namespace-assigned-to-temporary.js | 39 +++++++ .../repro-cx-namespace-nesting.expect.md | 68 ++++++++++++ .../meta-isms/repro-cx-namespace-nesting.js | 22 ++++ 6 files changed, 233 insertions(+), 5 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-assigned-to-temporary.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-assigned-to-temporary.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-nesting.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-nesting.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index 6ba3b5dcc3e60..c2dc1302f3996 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -248,7 +248,7 @@ function* runWithEnvironment( memoizeFbtOperandsInSameScope(hir); yield log({ kind: "hir", - name: "MemoizeFbtOperandsInSameScope", + name: "MemoizeFbtAndMacroOperandsInSameScope", value: hir, }); diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts index e0f38c0eee547..564c635c8b5ba 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts @@ -15,7 +15,6 @@ import { import { eachReactiveValueOperand } from "./visitors"; /** - * This pass supports the * This pass supports the `fbt` translation system (https://facebook.github.io/fbt/) * as well as similar user-configurable macro-like APIs where it's important that * the name of the function not be changed, and it's literal arguments not be turned @@ -75,7 +74,7 @@ function visit( for (const instruction of block.instructions) { const { lvalue, value } = instruction; if (lvalue === null) { - return; + continue; } if ( value.kind === "Primitive" && @@ -96,7 +95,7 @@ function visit( } else if (isFbtCallExpression(fbtValues, value)) { const fbtScope = lvalue.identifier.scope; if (fbtScope === null) { - return; + continue; } /* @@ -121,7 +120,7 @@ function visit( ) { const fbtScope = lvalue.identifier.scope; if (fbtScope === null) { - return; + continue; } /* diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-assigned-to-temporary.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-assigned-to-temporary.expect.md new file mode 100644 index 0000000000000..9b55426f4e057 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-assigned-to-temporary.expect.md @@ -0,0 +1,100 @@ + +## Input + +```javascript +// @compilationMode(infer) @enableAssumeHooksFollowRulesOfReact:false @customMacros(cx) +import { identity } from "shared-runtime"; + +const DARK = "dark"; + +function Component() { + const theme = useTheme(); + return ( +
+ ); +} + +function cx(obj) { + const classes = []; + for (const [key, value] of Object.entries(obj)) { + if (value) { + classes.push(key); + } + } + return classes.join(" "); +} + +function useTheme() { + return { + getTheme() { + return DARK; + }, + }; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @compilationMode(infer) @enableAssumeHooksFollowRulesOfReact:false @customMacros(cx) +import { identity } from "shared-runtime"; + +const DARK = "dark"; + +function Component() { + const $ = _c(2); + const theme = useTheme(); + + const t0 = cx.foo({ + "styles/light": true, + "styles/dark": identity([theme.getTheme()]), + }); + let t1; + if ($[0] !== t0) { + t1 =
; + $[0] = t0; + $[1] = t1; + } else { + t1 = $[1]; + } + return t1; +} + +function cx(obj) { + const classes = []; + for (const [key, value] of Object.entries(obj)) { + if (value) { + classes.push(key); + } + } + return classes.join(" "); +} + +function useTheme() { + return { + getTheme() { + return DARK; + }, + }; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +### Eval output +(kind: exception) cx.foo is not a function \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-assigned-to-temporary.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-assigned-to-temporary.js new file mode 100644 index 0000000000000..b002d99db6e70 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-assigned-to-temporary.js @@ -0,0 +1,39 @@ +// @compilationMode(infer) @enableAssumeHooksFollowRulesOfReact:false @customMacros(cx) +import { identity } from "shared-runtime"; + +const DARK = "dark"; + +function Component() { + const theme = useTheme(); + return ( +
+ ); +} + +function cx(obj) { + const classes = []; + for (const [key, value] of Object.entries(obj)) { + if (value) { + classes.push(key); + } + } + return classes.join(" "); +} + +function useTheme() { + return { + getTheme() { + return DARK; + }, + }; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-nesting.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-nesting.expect.md new file mode 100644 index 0000000000000..4400a550cee08 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-nesting.expect.md @@ -0,0 +1,68 @@ + +## Input + +```javascript +import { makeArray } from "shared-runtime"; + +function Component() { + const items = makeArray("foo", "bar", "", null, "baz", false, "merp"); + const classname = cx.namespace(...items.filter(isNonEmptyString)); + return
Ok
; +} + +function isNonEmptyString(s) { + return typeof s === "string" && s.trim().length !== 0; +} + +const cx = { + namespace(...items) { + return items.join(" "); + }, +}; + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { makeArray } from "shared-runtime"; + +function Component() { + const $ = _c(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + const items = makeArray("foo", "bar", "", null, "baz", false, "merp"); + const classname = cx.namespace(...items.filter(isNonEmptyString)); + t0 =
Ok
; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +function isNonEmptyString(s) { + return typeof s === "string" && s.trim().length !== 0; +} + +const cx = { + namespace(...items) { + return items.join(" "); + }, +}; + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + +### Eval output +(kind: ok)
Ok
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-nesting.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-nesting.js new file mode 100644 index 0000000000000..7f44985e9b590 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/meta-isms/repro-cx-namespace-nesting.js @@ -0,0 +1,22 @@ +import { makeArray } from "shared-runtime"; + +function Component() { + const items = makeArray("foo", "bar", "", null, "baz", false, "merp"); + const classname = cx.namespace(...items.filter(isNonEmptyString)); + return
Ok
; +} + +function isNonEmptyString(s) { + return typeof s === "string" && s.trim().length !== 0; +} + +const cx = { + namespace(...items) { + return items.join(" "); + }, +}; + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; From b223c3a00eae3bbecc919170ed221ef82d8b80bd Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 14 Jun 2024 08:44:40 -0700 Subject: [PATCH 2/2] Update on "[compiler] Support method-call version of macro functions" Adds fixtures for `macro.namespace(...)` style invocations which we use internally in some cases instead of just `macro(...)`. I tried every example i could think of that could possibly break it (including basing one off of another fixture where we hit an invariant related due to a temporary being emitted for a method call), and they all worked. I just had to fix an existing bug where we early return in some cases instead of continuing, which is a holdover from when this pass was originally written as a ReactiveFunction visitor. [ghstack-poisoned]