From d06196c1cd2dc0bb2288f3a3efa30669d4a7ea61 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Tue, 30 Jul 2024 16:55:41 -0400 Subject: [PATCH] [compiler] Visit nested scopes in pruned scopes in PromoteUsedTemporaries While debugging #30536 I happened to notice that the bug only reproduced when there was interleaving scopes, and observed that an unpruned scope nested inside of a pruned one was not being visited by CollectPromotableTemporaries, which keeps track of which identifiers should be promoted later. Therefore when actually promoting temporaries we were skipping over the identifiers in children of pruned scopes ghstack-source-id: d805f62f22fda04beedb6c7063312451f36d678c Pull Request resolved: https://github.com/facebook/react/pull/30537 --- .../ReactiveScopes/PromoteUsedTemporaries.ts | 1 + .../bug-renaming-jsx-tag-lowercase.expect.md | 107 ------------------ .../bug-renaming-jsx-tag-lowercase.tsx | 30 ----- .../renaming-jsx-tag-lowercase.expect.md | 85 ++++++++++++++ .../compiler/renaming-jsx-tag-lowercase.tsx | 18 +++ 5 files changed, 104 insertions(+), 137 deletions(-) delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-renaming-jsx-tag-lowercase.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-renaming-jsx-tag-lowercase.tsx create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/renaming-jsx-tag-lowercase.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/renaming-jsx-tag-lowercase.tsx diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts index 0f31dade2d..65b27fe639 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts @@ -138,6 +138,7 @@ class CollectPromotableTemporaries extends ReactiveFunctionVisitor { usedOutsideScope: false, }); } + this.visitBlock(scopeBlock.instructions, state); } override visitScope(scopeBlock: ReactiveScopeBlock, state: State): void { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-renaming-jsx-tag-lowercase.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-renaming-jsx-tag-lowercase.expect.md deleted file mode 100644 index ed0b39ce9a..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-renaming-jsx-tag-lowercase.expect.md +++ /dev/null @@ -1,107 +0,0 @@ - -## Input - -```javascript -import {Stringify, identity, useIdentity} from 'shared-runtime'; - -/** - * Currently, we're passing a lower-case jsx tag `t0`. - * We should either reorder Stringify or rename the local to `T0`. - * - * See evaluator error: - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok)
{"value":{}}
{"value":{}}
- * Forget: - * (kind: ok)
{"value":{}}
- * logs: ['Warning: The tag <%s> is unrecognized in this browser. If you meant to render a React component, start its name with an uppercase letter.%s','t1'] - */ -function Foo({}) { - const x = {}; - const y = {}; - useIdentity(0); - return ( - <> - - - - ); -} - -export const FIXTURE_ENTRYPOINT = { - fn: Foo, - params: [{}], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; -import { Stringify, identity, useIdentity } from "shared-runtime"; - -/** - * Currently, we're passing a lower-case jsx tag `t0`. - * We should either reorder Stringify or rename the local to `T0`. - * - * See evaluator error: - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok)
{"value":{}}
{"value":{}}
- * Forget: - * (kind: ok)
{"value":{}}
- * logs: ['Warning: The tag <%s> is unrecognized in this browser. If you meant to render a React component, start its name with an uppercase letter.%s','t1'] - */ -function Foo(t0) { - const $ = _c(9); - const x = {}; - const y = {}; - useIdentity(0); - - const t1 = Stringify; - const t2 = identity(y); - let t3; - if ($[0] !== t1 || $[1] !== t2) { - t3 = ; - $[0] = t1; - $[1] = t2; - $[2] = t3; - } else { - t3 = $[2]; - } - const T0 = Stringify; - const t4 = identity(x); - let t5; - if ($[3] !== T0 || $[4] !== t4) { - t5 = ; - $[3] = T0; - $[4] = t4; - $[5] = t5; - } else { - t5 = $[5]; - } - let t6; - if ($[6] !== t3 || $[7] !== t5) { - t6 = ( - <> - {t3} - {t5} - - ); - $[6] = t3; - $[7] = t5; - $[8] = t6; - } else { - t6 = $[8]; - } - return t6; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Foo, - params: [{}], -}; - -``` - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-renaming-jsx-tag-lowercase.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-renaming-jsx-tag-lowercase.tsx deleted file mode 100644 index 5a77309471..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-renaming-jsx-tag-lowercase.tsx +++ /dev/null @@ -1,30 +0,0 @@ -import {Stringify, identity, useIdentity} from 'shared-runtime'; - -/** - * Currently, we're passing a lower-case jsx tag `t0`. - * We should either reorder Stringify or rename the local to `T0`. - * - * See evaluator error: - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok)
{"value":{}}
{"value":{}}
- * Forget: - * (kind: ok)
{"value":{}}
- * logs: ['Warning: The tag <%s> is unrecognized in this browser. If you meant to render a React component, start its name with an uppercase letter.%s','t1'] - */ -function Foo({}) { - const x = {}; - const y = {}; - useIdentity(0); - return ( - <> - - - - ); -} - -export const FIXTURE_ENTRYPOINT = { - fn: Foo, - params: [{}], -}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/renaming-jsx-tag-lowercase.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/renaming-jsx-tag-lowercase.expect.md new file mode 100644 index 0000000000..61414ae895 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/renaming-jsx-tag-lowercase.expect.md @@ -0,0 +1,85 @@ + +## Input + +```javascript +import {Stringify, identity, useIdentity} from 'shared-runtime'; + +function Foo({}) { + const x = {}; + const y = {}; + useIdentity(0); + return ( + <> + + + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { Stringify, identity, useIdentity } from "shared-runtime"; + +function Foo(t0) { + const $ = _c(9); + const x = {}; + const y = {}; + useIdentity(0); + + const T0 = Stringify; + const t1 = identity(y); + let t2; + if ($[0] !== T0 || $[1] !== t1) { + t2 = ; + $[0] = T0; + $[1] = t1; + $[2] = t2; + } else { + t2 = $[2]; + } + const T1 = Stringify; + const t3 = identity(x); + let t4; + if ($[3] !== T1 || $[4] !== t3) { + t4 = ; + $[3] = T1; + $[4] = t3; + $[5] = t4; + } else { + t4 = $[5]; + } + let t5; + if ($[6] !== t2 || $[7] !== t4) { + t5 = ( + <> + {t2} + {t4} + + ); + $[6] = t2; + $[7] = t4; + $[8] = t5; + } else { + t5 = $[8]; + } + return t5; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{}], +}; + +``` + +### Eval output +(kind: ok)
{"value":{}}
{"value":{}}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/renaming-jsx-tag-lowercase.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/renaming-jsx-tag-lowercase.tsx new file mode 100644 index 0000000000..35995a862b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/renaming-jsx-tag-lowercase.tsx @@ -0,0 +1,18 @@ +import {Stringify, identity, useIdentity} from 'shared-runtime'; + +function Foo({}) { + const x = {}; + const y = {}; + useIdentity(0); + return ( + <> + + + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{}], +};