From 2d16326d9a3f45260aa80bcae78745ab2f199138 Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Mon, 30 Sep 2024 17:07:54 +0100 Subject: [PATCH 1/8] fix[scripts/devtools/publish-release]: parse version list instead of handling 404 (#31087) Discovered yesterday while was publishing a new release. NPM `10.x.x` changed the text for 404 errors, so this check was failing. Instead of handling 404 as a signal, I think its better to just parse the whole list of versions and check if the new one is already there. --- scripts/devtools/publish-release.js | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/scripts/devtools/publish-release.js b/scripts/devtools/publish-release.js index 495fddefc2cd6..6a2ab4f79d2ef 100755 --- a/scripts/devtools/publish-release.js +++ b/scripts/devtools/publish-release.js @@ -82,18 +82,13 @@ async function publishToNPM() { // If so we might be resuming from a previous run. // We could infer this by comparing the build-info.json, // But for now the easiest way is just to ask if this is expected. - const info = await execRead(`npm view ${npmPackage}@${version}`) - // Early versions of npm view gives empty response, but newer versions give 404 error. - // Catch the error to keep it consistent. - .catch(childProcessError => { - if (childProcessError.stderr.startsWith('npm ERR! code E404')) { - return null; - } - - throw childProcessError; - }); + const versionListJSON = await execRead( + `npm view ${npmPackage} versions --json` + ); + const versionList = JSON.parse(versionListJSON); + const versionIsAlreadyPublished = versionList.includes(version); - if (info) { + if (versionIsAlreadyPublished) { console.log(''); console.log( `${npmPackage} version ${chalk.bold( From 943e45e910d1a125f2be431c2b66f22a035ea0c9 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Mon, 30 Sep 2024 12:24:19 -0400 Subject: [PATCH 2/8] [compiler][test fixtures] Fork more fixtures for hir-rewrite Followup from #30894 , not sure how these got missed. Note that this PR just copies the fixtures without adding `@enablePropagateDepsInHIR`. #31032 follows and actually enables the HIR-version of propagateScopeDeps to run. I split this out into two PRs to make snapshot differences easier to review, but also happy to merge Fixtures found from locally setting snap test runner to default to `enablePropagateDepsInHIR: 'enabled_baseline'` and forking fixtures files with different output. ghstack-source-id: 7d7cf41aa923d83ad49f89079171b0411923ce6b Pull Request resolved: https://github.com/facebook/react/pull/31030 --- .../conditional-break-labeled.expect.md | 65 ++++++ .../conditional-break-labeled.js | 21 ++ .../conditional-early-return.expect.md | 197 ++++++++++++++++++ .../conditional-early-return.js | 58 ++++++ .../conditional-on-mutable.expect.md | 88 ++++++++ .../conditional-on-mutable.js | 26 +++ ...rly-return-within-reactive-scope.expect.md | 89 ++++++++ ...sted-early-return-within-reactive-scope.js | 21 ++ ...rly-return-within-reactive-scope.expect.md | 112 ++++++++++ .../early-return-within-reactive-scope.js | 33 +++ ...-optional-call-chain-in-optional.expect.md | 34 +++ ...or.todo-optional-call-chain-in-optional.ts | 13 ++ .../iife-return-modified-later-phi.expect.md | 57 +++++ .../iife-return-modified-later-phi.js | 16 ++ ...consequent-alternate-both-return.expect.md | 66 ++++++ ...ted-in-consequent-alternate-both-return.js | 17 ++ ...rly-return-within-reactive-scope.expect.md | 80 +++++++ ...tial-early-return-within-reactive-scope.js | 20 ++ ...ence-array-push-consecutive-phis.expect.md | 110 ++++++++++ ...e-inference-array-push-consecutive-phis.js | 37 ++++ .../phi-type-inference-array-push.expect.md | 83 ++++++++ .../phi-type-inference-array-push.js | 26 +++ ...hi-type-inference-property-store.expect.md | 72 +++++++ .../phi-type-inference-property-store.js | 22 ++ ...properties-inside-optional-chain.expect.md | 31 +++ ...tional-properties-inside-optional-chain.js | 3 + ...ming-unconditional-with-mutation.expect.md | 79 +++++++ ...sa-renaming-unconditional-with-mutation.js | 27 +++ .../packages/snap/src/SproutTodoFilter.ts | 1 + 29 files changed, 1504 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-break-labeled.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-break-labeled.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-early-return.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-early-return.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-on-mutable.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-on-mutable.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-nested-early-return-within-reactive-scope.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-nested-early-return-within-reactive-scope.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-within-reactive-scope.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-within-reactive-scope.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/error.todo-optional-call-chain-in-optional.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/error.todo-optional-call-chain-in-optional.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/iife-return-modified-later-phi.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/iife-return-modified-later-phi.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/object-mutated-in-consequent-alternate-both-return.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/object-mutated-in-consequent-alternate-both-return.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/partial-early-return-within-reactive-scope.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/partial-early-return-within-reactive-scope.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push-consecutive-phis.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push-consecutive-phis.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-property-store.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-property-store.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reactive-dependencies-non-optional-properties-inside-optional-chain.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reactive-dependencies-non-optional-properties-inside-optional-chain.js create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/ssa-renaming-unconditional-with-mutation.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/ssa-renaming-unconditional-with-mutation.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-break-labeled.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-break-labeled.expect.md new file mode 100644 index 0000000000000..76648c251a101 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-break-labeled.expect.md @@ -0,0 +1,65 @@ + +## Input + +```javascript +/** + * props.b *does* influence `a` + */ +function Component(props) { + const a = []; + a.push(props.a); + label: { + if (props.b) { + break label; + } + a.push(props.c); + } + a.push(props.d); + return a; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: ['TodoAdd'], + isComponent: 'TodoAdd', +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; /** + * props.b *does* influence `a` + */ +function Component(props) { + const $ = _c(2); + let a; + if ($[0] !== props) { + a = []; + a.push(props.a); + bb0: { + if (props.b) { + break bb0; + } + + a.push(props.c); + } + + a.push(props.d); + $[0] = props; + $[1] = a; + } else { + a = $[1]; + } + return a; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: ["TodoAdd"], + isComponent: "TodoAdd", +}; + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-break-labeled.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-break-labeled.js new file mode 100644 index 0000000000000..ccf983189cf71 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-break-labeled.js @@ -0,0 +1,21 @@ +/** + * props.b *does* influence `a` + */ +function Component(props) { + const a = []; + a.push(props.a); + label: { + if (props.b) { + break label; + } + a.push(props.c); + } + a.push(props.d); + return a; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: ['TodoAdd'], + isComponent: 'TodoAdd', +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-early-return.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-early-return.expect.md new file mode 100644 index 0000000000000..82537902bfa19 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-early-return.expect.md @@ -0,0 +1,197 @@ + +## Input + +```javascript +/** + * props.b does *not* influence `a` + */ +function ComponentA(props) { + const a_DEBUG = []; + a_DEBUG.push(props.a); + if (props.b) { + return null; + } + a_DEBUG.push(props.d); + return a_DEBUG; +} + +/** + * props.b *does* influence `a` + */ +function ComponentB(props) { + const a = []; + a.push(props.a); + if (props.b) { + a.push(props.c); + } + a.push(props.d); + return a; +} + +/** + * props.b *does* influence `a`, but only in a way that is never observable + */ +function ComponentC(props) { + const a = []; + a.push(props.a); + if (props.b) { + a.push(props.c); + return null; + } + a.push(props.d); + return a; +} + +/** + * props.b *does* influence `a` + */ +function ComponentD(props) { + const a = []; + a.push(props.a); + if (props.b) { + a.push(props.c); + return a; + } + a.push(props.d); + return a; +} + +export const FIXTURE_ENTRYPOINT = { + fn: ComponentA, + params: [{a: 1, b: false, d: 3}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; /** + * props.b does *not* influence `a` + */ +function ComponentA(props) { + const $ = _c(3); + let a_DEBUG; + let t0; + if ($[0] !== props) { + t0 = Symbol.for("react.early_return_sentinel"); + bb0: { + a_DEBUG = []; + a_DEBUG.push(props.a); + if (props.b) { + t0 = null; + break bb0; + } + + a_DEBUG.push(props.d); + } + $[0] = props; + $[1] = a_DEBUG; + $[2] = t0; + } else { + a_DEBUG = $[1]; + t0 = $[2]; + } + if (t0 !== Symbol.for("react.early_return_sentinel")) { + return t0; + } + return a_DEBUG; +} + +/** + * props.b *does* influence `a` + */ +function ComponentB(props) { + const $ = _c(2); + let a; + if ($[0] !== props) { + a = []; + a.push(props.a); + if (props.b) { + a.push(props.c); + } + + a.push(props.d); + $[0] = props; + $[1] = a; + } else { + a = $[1]; + } + return a; +} + +/** + * props.b *does* influence `a`, but only in a way that is never observable + */ +function ComponentC(props) { + const $ = _c(3); + let a; + let t0; + if ($[0] !== props) { + t0 = Symbol.for("react.early_return_sentinel"); + bb0: { + a = []; + a.push(props.a); + if (props.b) { + a.push(props.c); + t0 = null; + break bb0; + } + + a.push(props.d); + } + $[0] = props; + $[1] = a; + $[2] = t0; + } else { + a = $[1]; + t0 = $[2]; + } + if (t0 !== Symbol.for("react.early_return_sentinel")) { + return t0; + } + return a; +} + +/** + * props.b *does* influence `a` + */ +function ComponentD(props) { + const $ = _c(3); + let a; + let t0; + if ($[0] !== props) { + t0 = Symbol.for("react.early_return_sentinel"); + bb0: { + a = []; + a.push(props.a); + if (props.b) { + a.push(props.c); + t0 = a; + break bb0; + } + + a.push(props.d); + } + $[0] = props; + $[1] = a; + $[2] = t0; + } else { + a = $[1]; + t0 = $[2]; + } + if (t0 !== Symbol.for("react.early_return_sentinel")) { + return t0; + } + return a; +} + +export const FIXTURE_ENTRYPOINT = { + fn: ComponentA, + params: [{ a: 1, b: false, d: 3 }], +}; + +``` + +### Eval output +(kind: ok) [1,3] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-early-return.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-early-return.js new file mode 100644 index 0000000000000..e0ed101409252 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-early-return.js @@ -0,0 +1,58 @@ +/** + * props.b does *not* influence `a` + */ +function ComponentA(props) { + const a_DEBUG = []; + a_DEBUG.push(props.a); + if (props.b) { + return null; + } + a_DEBUG.push(props.d); + return a_DEBUG; +} + +/** + * props.b *does* influence `a` + */ +function ComponentB(props) { + const a = []; + a.push(props.a); + if (props.b) { + a.push(props.c); + } + a.push(props.d); + return a; +} + +/** + * props.b *does* influence `a`, but only in a way that is never observable + */ +function ComponentC(props) { + const a = []; + a.push(props.a); + if (props.b) { + a.push(props.c); + return null; + } + a.push(props.d); + return a; +} + +/** + * props.b *does* influence `a` + */ +function ComponentD(props) { + const a = []; + a.push(props.a); + if (props.b) { + a.push(props.c); + return a; + } + a.push(props.d); + return a; +} + +export const FIXTURE_ENTRYPOINT = { + fn: ComponentA, + params: [{a: 1, b: false, d: 3}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-on-mutable.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-on-mutable.expect.md new file mode 100644 index 0000000000000..24c565b088d2f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-on-mutable.expect.md @@ -0,0 +1,88 @@ + +## Input + +```javascript +function ComponentA(props) { + const a = []; + const b = []; + if (b) { + a.push(props.p0); + } + if (props.p1) { + b.push(props.p2); + } + return ; +} + +function ComponentB(props) { + const a = []; + const b = []; + if (mayMutate(b)) { + a.push(props.p0); + } + if (props.p1) { + b.push(props.p2); + } + return ; +} + +function Foo() {} +function mayMutate() {} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function ComponentA(props) { + const $ = _c(2); + let t0; + if ($[0] !== props) { + const a = []; + const b = []; + if (b) { + a.push(props.p0); + } + if (props.p1) { + b.push(props.p2); + } + + t0 = ; + $[0] = props; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +function ComponentB(props) { + const $ = _c(2); + let t0; + if ($[0] !== props) { + const a = []; + const b = []; + if (mayMutate(b)) { + a.push(props.p0); + } + if (props.p1) { + b.push(props.p2); + } + + t0 = ; + $[0] = props; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +function Foo() {} +function mayMutate() {} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-on-mutable.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-on-mutable.js new file mode 100644 index 0000000000000..0378ab655c1ef --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-on-mutable.js @@ -0,0 +1,26 @@ +function ComponentA(props) { + const a = []; + const b = []; + if (b) { + a.push(props.p0); + } + if (props.p1) { + b.push(props.p2); + } + return ; +} + +function ComponentB(props) { + const a = []; + const b = []; + if (mayMutate(b)) { + a.push(props.p0); + } + if (props.p1) { + b.push(props.p2); + } + return ; +} + +function Foo() {} +function mayMutate() {} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-nested-early-return-within-reactive-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-nested-early-return-within-reactive-scope.expect.md new file mode 100644 index 0000000000000..2d33981f73fd1 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-nested-early-return-within-reactive-scope.expect.md @@ -0,0 +1,89 @@ + +## Input + +```javascript +function Component(props) { + let x = []; + if (props.cond) { + x.push(props.a); + if (props.b) { + const y = [props.b]; + x.push(y); + // oops no memo! + return x; + } + // oops no memo! + return x; + } else { + return foo(); + } +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{cond: true, a: 42, b: 3.14}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function Component(props) { + const $ = _c(5); + let t0; + if ($[0] !== props) { + t0 = Symbol.for("react.early_return_sentinel"); + bb0: { + const x = []; + if (props.cond) { + x.push(props.a); + if (props.b) { + let t1; + if ($[2] !== props.b) { + t1 = [props.b]; + $[2] = props.b; + $[3] = t1; + } else { + t1 = $[3]; + } + const y = t1; + x.push(y); + t0 = x; + break bb0; + } + + t0 = x; + break bb0; + } else { + let t1; + if ($[4] === Symbol.for("react.memo_cache_sentinel")) { + t1 = foo(); + $[4] = t1; + } else { + t1 = $[4]; + } + t0 = t1; + break bb0; + } + } + $[0] = props; + $[1] = t0; + } else { + t0 = $[1]; + } + if (t0 !== Symbol.for("react.early_return_sentinel")) { + return t0; + } +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ cond: true, a: 42, b: 3.14 }], +}; + +``` + +### Eval output +(kind: ok) [42,[3.14]] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-nested-early-return-within-reactive-scope.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-nested-early-return-within-reactive-scope.js new file mode 100644 index 0000000000000..53eb06bc591e4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-nested-early-return-within-reactive-scope.js @@ -0,0 +1,21 @@ +function Component(props) { + let x = []; + if (props.cond) { + x.push(props.a); + if (props.b) { + const y = [props.b]; + x.push(y); + // oops no memo! + return x; + } + // oops no memo! + return x; + } else { + return foo(); + } +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{cond: true, a: 42, b: 3.14}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-within-reactive-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-within-reactive-scope.expect.md new file mode 100644 index 0000000000000..6c3525e9e77eb --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-within-reactive-scope.expect.md @@ -0,0 +1,112 @@ + +## Input + +```javascript +import {makeArray} from 'shared-runtime'; + +function Component(props) { + let x = []; + if (props.cond) { + x.push(props.a); + // oops no memo! + return x; + } else { + return makeArray(props.b); + } +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], + sequentialRenders: [ + // pattern 1 + {cond: true, a: 42}, + {cond: true, a: 42}, + // pattern 2 + {cond: false, b: 3.14}, + {cond: false, b: 3.14}, + // pattern 1 + {cond: true, a: 42}, + // pattern 2 + {cond: false, b: 3.14}, + // pattern 1 + {cond: true, a: 42}, + // pattern 2 + {cond: false, b: 3.14}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { makeArray } from "shared-runtime"; + +function Component(props) { + const $ = _c(4); + let t0; + if ($[0] !== props) { + t0 = Symbol.for("react.early_return_sentinel"); + bb0: { + const x = []; + if (props.cond) { + x.push(props.a); + t0 = x; + break bb0; + } else { + let t1; + if ($[2] !== props.b) { + t1 = makeArray(props.b); + $[2] = props.b; + $[3] = t1; + } else { + t1 = $[3]; + } + t0 = t1; + break bb0; + } + } + $[0] = props; + $[1] = t0; + } else { + t0 = $[1]; + } + if (t0 !== Symbol.for("react.early_return_sentinel")) { + return t0; + } +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], + sequentialRenders: [ + // pattern 1 + { cond: true, a: 42 }, + { cond: true, a: 42 }, + // pattern 2 + { cond: false, b: 3.14 }, + { cond: false, b: 3.14 }, + // pattern 1 + { cond: true, a: 42 }, + // pattern 2 + { cond: false, b: 3.14 }, + // pattern 1 + { cond: true, a: 42 }, + // pattern 2 + { cond: false, b: 3.14 }, + ], +}; + +``` + +### Eval output +(kind: ok) [42] +[42] +[3.14] +[3.14] +[42] +[3.14] +[42] +[3.14] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-within-reactive-scope.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-within-reactive-scope.js new file mode 100644 index 0000000000000..7446d76fb320c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-within-reactive-scope.js @@ -0,0 +1,33 @@ +import {makeArray} from 'shared-runtime'; + +function Component(props) { + let x = []; + if (props.cond) { + x.push(props.a); + // oops no memo! + return x; + } else { + return makeArray(props.b); + } +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], + sequentialRenders: [ + // pattern 1 + {cond: true, a: 42}, + {cond: true, a: 42}, + // pattern 2 + {cond: false, b: 3.14}, + {cond: false, b: 3.14}, + // pattern 1 + {cond: true, a: 42}, + // pattern 2 + {cond: false, b: 3.14}, + // pattern 1 + {cond: true, a: 42}, + // pattern 2 + {cond: false, b: 3.14}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/error.todo-optional-call-chain-in-optional.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/error.todo-optional-call-chain-in-optional.expect.md new file mode 100644 index 0000000000000..75c5d61d407f0 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/error.todo-optional-call-chain-in-optional.expect.md @@ -0,0 +1,34 @@ + +## Input + +```javascript +function useFoo(props: {value: {x: string; y: string} | null}) { + const value = props.value; + return createArray(value?.x, value?.y)?.join(', '); +} + +function createArray(...args: Array): Array { + return args; +} + +export const FIXTURE_ENTRYPONT = { + fn: useFoo, + props: [{value: null}], +}; + +``` + + +## Error + +``` + 1 | function useFoo(props: {value: {x: string; y: string} | null}) { + 2 | const value = props.value; +> 3 | return createArray(value?.x, value?.y)?.join(', '); + | ^^^^^^^^ Todo: Unexpected terminal kind `optional` for optional test block (3:3) + 4 | } + 5 | + 6 | function createArray(...args: Array): Array { +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/error.todo-optional-call-chain-in-optional.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/error.todo-optional-call-chain-in-optional.ts new file mode 100644 index 0000000000000..7ee50e0380288 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/error.todo-optional-call-chain-in-optional.ts @@ -0,0 +1,13 @@ +function useFoo(props: {value: {x: string; y: string} | null}) { + const value = props.value; + return createArray(value?.x, value?.y)?.join(', '); +} + +function createArray(...args: Array): Array { + return args; +} + +export const FIXTURE_ENTRYPONT = { + fn: useFoo, + props: [{value: null}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/iife-return-modified-later-phi.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/iife-return-modified-later-phi.expect.md new file mode 100644 index 0000000000000..bed1c329f0d10 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/iife-return-modified-later-phi.expect.md @@ -0,0 +1,57 @@ + +## Input + +```javascript +function Component(props) { + const items = (() => { + if (props.cond) { + return []; + } else { + return null; + } + })(); + items?.push(props.a); + return items; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: {}}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function Component(props) { + const $ = _c(2); + let items; + if ($[0] !== props) { + let t0; + if (props.cond) { + t0 = []; + } else { + t0 = null; + } + items = t0; + + items?.push(props.a); + $[0] = props; + $[1] = items; + } else { + items = $[1]; + } + return items; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ a: {} }], +}; + +``` + +### Eval output +(kind: ok) null \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/iife-return-modified-later-phi.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/iife-return-modified-later-phi.js new file mode 100644 index 0000000000000..f4f953d294e6f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/iife-return-modified-later-phi.js @@ -0,0 +1,16 @@ +function Component(props) { + const items = (() => { + if (props.cond) { + return []; + } else { + return null; + } + })(); + items?.push(props.a); + return items; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: {}}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/object-mutated-in-consequent-alternate-both-return.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/object-mutated-in-consequent-alternate-both-return.expect.md new file mode 100644 index 0000000000000..8a20f9186b447 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/object-mutated-in-consequent-alternate-both-return.expect.md @@ -0,0 +1,66 @@ + +## Input + +```javascript +import {makeObject_Primitives} from 'shared-runtime'; + +function Component(props) { + const object = makeObject_Primitives(); + if (props.cond) { + object.value = 1; + return object; + } else { + object.value = props.value; + return object; + } +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{cond: false, value: [0, 1, 2]}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { makeObject_Primitives } from "shared-runtime"; + +function Component(props) { + const $ = _c(2); + let t0; + if ($[0] !== props) { + t0 = Symbol.for("react.early_return_sentinel"); + bb0: { + const object = makeObject_Primitives(); + if (props.cond) { + object.value = 1; + t0 = object; + break bb0; + } else { + object.value = props.value; + t0 = object; + break bb0; + } + } + $[0] = props; + $[1] = t0; + } else { + t0 = $[1]; + } + if (t0 !== Symbol.for("react.early_return_sentinel")) { + return t0; + } +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ cond: false, value: [0, 1, 2] }], +}; + +``` + +### Eval output +(kind: ok) {"a":0,"b":"value1","c":true,"value":[0,1,2]} \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/object-mutated-in-consequent-alternate-both-return.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/object-mutated-in-consequent-alternate-both-return.js new file mode 100644 index 0000000000000..94a142d033708 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/object-mutated-in-consequent-alternate-both-return.js @@ -0,0 +1,17 @@ +import {makeObject_Primitives} from 'shared-runtime'; + +function Component(props) { + const object = makeObject_Primitives(); + if (props.cond) { + object.value = 1; + return object; + } else { + object.value = props.value; + return object; + } +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{cond: false, value: [0, 1, 2]}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/partial-early-return-within-reactive-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/partial-early-return-within-reactive-scope.expect.md new file mode 100644 index 0000000000000..398161f0c6a24 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/partial-early-return-within-reactive-scope.expect.md @@ -0,0 +1,80 @@ + +## Input + +```javascript +function Component(props) { + let x = []; + let y = null; + if (props.cond) { + x.push(props.a); + // oops no memo! + return x; + } else { + y = foo(); + if (props.b) { + return; + } + } + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{cond: true, a: 42}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function Component(props) { + const $ = _c(4); + let y; + let t0; + if ($[0] !== props) { + t0 = Symbol.for("react.early_return_sentinel"); + bb0: { + const x = []; + if (props.cond) { + x.push(props.a); + t0 = x; + break bb0; + } else { + let t1; + if ($[3] === Symbol.for("react.memo_cache_sentinel")) { + t1 = foo(); + $[3] = t1; + } else { + t1 = $[3]; + } + y = t1; + if (props.b) { + t0 = undefined; + break bb0; + } + } + } + $[0] = props; + $[1] = y; + $[2] = t0; + } else { + y = $[1]; + t0 = $[2]; + } + if (t0 !== Symbol.for("react.early_return_sentinel")) { + return t0; + } + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ cond: true, a: 42 }], +}; + +``` + +### Eval output +(kind: ok) [42] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/partial-early-return-within-reactive-scope.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/partial-early-return-within-reactive-scope.js new file mode 100644 index 0000000000000..66d68242b8bb6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/partial-early-return-within-reactive-scope.js @@ -0,0 +1,20 @@ +function Component(props) { + let x = []; + let y = null; + if (props.cond) { + x.push(props.a); + // oops no memo! + return x; + } else { + y = foo(); + if (props.b) { + return; + } + } + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{cond: true, a: 42}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push-consecutive-phis.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push-consecutive-phis.expect.md new file mode 100644 index 0000000000000..f17bcc92cb7ce --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push-consecutive-phis.expect.md @@ -0,0 +1,110 @@ + +## Input + +```javascript +import {makeArray} from 'shared-runtime'; + +function Component(props) { + const x = {}; + let y; + if (props.cond) { + if (props.cond2) { + y = [props.value]; + } else { + y = [props.value2]; + } + } else { + y = []; + } + // This should be inferred as ` y` s.t. `x` can still + // be independently memoized. *But* this also must properly + // extend the mutable range of the array literals in the + // if/else branches + y.push(x); + + return [x, y]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{cond: true, cond2: true, value: 42}], + sequentialRenders: [ + {cond: true, cond2: true, value: 3.14}, + {cond: true, cond2: true, value: 42}, + {cond: true, cond2: true, value: 3.14}, + {cond: true, cond2: false, value2: 3.14}, + {cond: true, cond2: false, value2: 42}, + {cond: true, cond2: false, value2: 3.14}, + {cond: false}, + {cond: false}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { makeArray } from "shared-runtime"; + +function Component(props) { + const $ = _c(3); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = {}; + $[0] = t0; + } else { + t0 = $[0]; + } + const x = t0; + let t1; + if ($[1] !== props) { + let y; + if (props.cond) { + if (props.cond2) { + y = [props.value]; + } else { + y = [props.value2]; + } + } else { + y = []; + } + + y.push(x); + + t1 = [x, y]; + $[1] = props; + $[2] = t1; + } else { + t1 = $[2]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ cond: true, cond2: true, value: 42 }], + sequentialRenders: [ + { cond: true, cond2: true, value: 3.14 }, + { cond: true, cond2: true, value: 42 }, + { cond: true, cond2: true, value: 3.14 }, + { cond: true, cond2: false, value2: 3.14 }, + { cond: true, cond2: false, value2: 42 }, + { cond: true, cond2: false, value2: 3.14 }, + { cond: false }, + { cond: false }, + ], +}; + +``` + +### Eval output +(kind: ok) [{},[3.14,"[[ cyclic ref *1 ]]"]] +[{},[42,"[[ cyclic ref *1 ]]"]] +[{},[3.14,"[[ cyclic ref *1 ]]"]] +[{},[3.14,"[[ cyclic ref *1 ]]"]] +[{},[42,"[[ cyclic ref *1 ]]"]] +[{},[3.14,"[[ cyclic ref *1 ]]"]] +[{},["[[ cyclic ref *1 ]]"]] +[{},["[[ cyclic ref *1 ]]"]] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push-consecutive-phis.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push-consecutive-phis.js new file mode 100644 index 0000000000000..0a9aa39defea4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push-consecutive-phis.js @@ -0,0 +1,37 @@ +import {makeArray} from 'shared-runtime'; + +function Component(props) { + const x = {}; + let y; + if (props.cond) { + if (props.cond2) { + y = [props.value]; + } else { + y = [props.value2]; + } + } else { + y = []; + } + // This should be inferred as ` y` s.t. `x` can still + // be independently memoized. *But* this also must properly + // extend the mutable range of the array literals in the + // if/else branches + y.push(x); + + return [x, y]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{cond: true, cond2: true, value: 42}], + sequentialRenders: [ + {cond: true, cond2: true, value: 3.14}, + {cond: true, cond2: true, value: 42}, + {cond: true, cond2: true, value: 3.14}, + {cond: true, cond2: false, value2: 3.14}, + {cond: true, cond2: false, value2: 42}, + {cond: true, cond2: false, value2: 3.14}, + {cond: false}, + {cond: false}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push.expect.md new file mode 100644 index 0000000000000..f58eed10fda5a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push.expect.md @@ -0,0 +1,83 @@ + +## Input + +```javascript +function Component(props) { + const x = {}; + let y; + if (props.cond) { + y = [props.value]; + } else { + y = []; + } + // This should be inferred as ` y` s.t. `x` can still + // be independently memoized. *But* this also must properly + // extend the mutable range of the array literals in the + // if/else branches + y.push(x); + + return [x, y]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{cond: true, value: 42}], + sequentialRenders: [ + {cond: true, value: 3.14}, + {cond: false, value: 3.14}, + {cond: true, value: 42}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function Component(props) { + const $ = _c(3); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = {}; + $[0] = t0; + } else { + t0 = $[0]; + } + const x = t0; + let t1; + if ($[1] !== props) { + let y; + if (props.cond) { + y = [props.value]; + } else { + y = []; + } + + y.push(x); + + t1 = [x, y]; + $[1] = props; + $[2] = t1; + } else { + t1 = $[2]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ cond: true, value: 42 }], + sequentialRenders: [ + { cond: true, value: 3.14 }, + { cond: false, value: 3.14 }, + { cond: true, value: 42 }, + ], +}; + +``` + +### Eval output +(kind: ok) [{},[3.14,"[[ cyclic ref *1 ]]"]] +[{},["[[ cyclic ref *1 ]]"]] +[{},[42,"[[ cyclic ref *1 ]]"]] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push.js new file mode 100644 index 0000000000000..732a1524cba2d --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push.js @@ -0,0 +1,26 @@ +function Component(props) { + const x = {}; + let y; + if (props.cond) { + y = [props.value]; + } else { + y = []; + } + // This should be inferred as ` y` s.t. `x` can still + // be independently memoized. *But* this also must properly + // extend the mutable range of the array literals in the + // if/else branches + y.push(x); + + return [x, y]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{cond: true, value: 42}], + sequentialRenders: [ + {cond: true, value: 3.14}, + {cond: false, value: 3.14}, + {cond: true, value: 42}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-property-store.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-property-store.expect.md new file mode 100644 index 0000000000000..70551c8e9d7b0 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-property-store.expect.md @@ -0,0 +1,72 @@ + +## Input + +```javascript +// @debug +function Component(props) { + const x = {}; + let y; + if (props.cond) { + y = {}; + } else { + y = {a: props.a}; + } + // This should be inferred as ` y` s.t. `x` can still + // be independently memoized. *But* this also must properly + // extend the mutable range of the object literals in the + // if/else branches + y.x = x; + + return [x, y]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{cond: false, a: 'a!'}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @debug +function Component(props) { + const $ = _c(3); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + t0 = {}; + $[0] = t0; + } else { + t0 = $[0]; + } + const x = t0; + let t1; + if ($[1] !== props) { + let y; + if (props.cond) { + y = {}; + } else { + y = { a: props.a }; + } + + y.x = x; + + t1 = [x, y]; + $[1] = props; + $[2] = t1; + } else { + t1 = $[2]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ cond: false, a: "a!" }], +}; + +``` + +### Eval output +(kind: ok) [{},{"a":"a!","x":"[[ cyclic ref *1 ]]"}] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-property-store.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-property-store.js new file mode 100644 index 0000000000000..ba7133ff808df --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-property-store.js @@ -0,0 +1,22 @@ +// @debug +function Component(props) { + const x = {}; + let y; + if (props.cond) { + y = {}; + } else { + y = {a: props.a}; + } + // This should be inferred as ` y` s.t. `x` can still + // be independently memoized. *But* this also must properly + // extend the mutable range of the object literals in the + // if/else branches + y.x = x; + + return [x, y]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{cond: false, a: 'a!'}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reactive-dependencies-non-optional-properties-inside-optional-chain.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reactive-dependencies-non-optional-properties-inside-optional-chain.expect.md new file mode 100644 index 0000000000000..6574bd1966ed9 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reactive-dependencies-non-optional-properties-inside-optional-chain.expect.md @@ -0,0 +1,31 @@ + +## Input + +```javascript +function Component(props) { + return props.post.feedback.comments?.edges?.map(render); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function Component(props) { + const $ = _c(2); + let t0; + if ($[0] !== props.post.feedback.comments) { + t0 = props.post.feedback.comments?.edges?.map(render); + $[0] = props.post.feedback.comments; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reactive-dependencies-non-optional-properties-inside-optional-chain.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reactive-dependencies-non-optional-properties-inside-optional-chain.js new file mode 100644 index 0000000000000..e8e0da392e9e7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reactive-dependencies-non-optional-properties-inside-optional-chain.js @@ -0,0 +1,3 @@ +function Component(props) { + return props.post.feedback.comments?.edges?.map(render); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/ssa-renaming-unconditional-with-mutation.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/ssa-renaming-unconditional-with-mutation.expect.md new file mode 100644 index 0000000000000..f4689e5795552 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/ssa-renaming-unconditional-with-mutation.expect.md @@ -0,0 +1,79 @@ + +## Input + +```javascript +import {mutate} from 'shared-runtime'; + +function useFoo(props) { + let x = []; + x.push(props.bar); + if (props.cond) { + x = {}; + x = []; + x.push(props.foo); + } else { + x = []; + x = []; + x.push(props.bar); + } + mutate(x); + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{bar: 'bar', foo: 'foo', cond: true}], + sequentialRenders: [ + {bar: 'bar', foo: 'foo', cond: true}, + {bar: 'bar', foo: 'foo', cond: true}, + {bar: 'bar', foo: 'foo', cond: false}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { mutate } from "shared-runtime"; + +function useFoo(props) { + const $ = _c(2); + let x; + if ($[0] !== props) { + x = []; + x.push(props.bar); + if (props.cond) { + x = []; + x.push(props.foo); + } else { + x = []; + x.push(props.bar); + } + + mutate(x); + $[0] = props; + $[1] = x; + } else { + x = $[1]; + } + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ bar: "bar", foo: "foo", cond: true }], + sequentialRenders: [ + { bar: "bar", foo: "foo", cond: true }, + { bar: "bar", foo: "foo", cond: true }, + { bar: "bar", foo: "foo", cond: false }, + ], +}; + +``` + +### Eval output +(kind: ok) ["foo","joe"] +["foo","joe"] +["bar","joe"] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/ssa-renaming-unconditional-with-mutation.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/ssa-renaming-unconditional-with-mutation.js new file mode 100644 index 0000000000000..3e7078cfc7999 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/ssa-renaming-unconditional-with-mutation.js @@ -0,0 +1,27 @@ +import {mutate} from 'shared-runtime'; + +function useFoo(props) { + let x = []; + x.push(props.bar); + if (props.cond) { + x = {}; + x = []; + x.push(props.foo); + } else { + x = []; + x = []; + x.push(props.bar); + } + mutate(x); + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{bar: 'bar', foo: 'foo', cond: true}], + sequentialRenders: [ + {bar: 'bar', foo: 'foo', cond: true}, + {bar: 'bar', foo: 'foo', cond: true}, + {bar: 'bar', foo: 'foo', cond: false}, + ], +}; diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index 7d7476dc74739..7140cad2f74bc 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -50,6 +50,7 @@ const skipFilter = new Set([ 'component', 'cond-deps-conditional-member-expr', 'conditional-break-labeled', + 'propagate-scope-deps-hir-fork/conditional-break-labeled', 'conditional-set-state-in-render', 'constant-computed', 'constant-propagation-phi', From 1a779207a7b85314e16d410b185d427702f22ebc Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Mon, 30 Sep 2024 12:24:20 -0400 Subject: [PATCH 3/8] [compiler][test fixtures] Add enablePropagateDepsInHIR to forked tests Annotates fixtures added in #31030 with `@enablePropagateDepsInHIR` to fork behavior (and commit snapshot differences) ghstack-source-id: e423e8c42db62f1bb87562b770761be09fc8ffc6 Pull Request resolved: https://github.com/facebook/react/pull/31031 --- .../conditional-break-labeled.expect.md | 22 +++-- .../conditional-break-labeled.js | 1 + .../conditional-early-return.expect.md | 82 +++++++++++++------ .../conditional-early-return.js | 1 + .../conditional-on-mutable.expect.md | 27 +++--- .../conditional-on-mutable.js | 1 + ...rly-return-within-reactive-scope.expect.md | 29 ++++--- ...sted-early-return-within-reactive-scope.js | 1 + ...rly-return-within-reactive-scope.expect.md | 23 +++--- .../early-return-within-reactive-scope.js | 1 + ...-optional-call-chain-in-optional.expect.md | 15 ++-- ...or.todo-optional-call-chain-in-optional.ts | 1 + .../iife-return-modified-later-phi.expect.md | 14 ++-- .../iife-return-modified-later-phi.js | 1 + ...consequent-alternate-both-return.expect.md | 14 ++-- ...ted-in-consequent-alternate-both-return.js | 1 + ...rly-return-within-reactive-scope.expect.md | 25 +++--- ...tial-early-return-within-reactive-scope.js | 1 + ...ence-array-push-consecutive-phis.expect.md | 21 +++-- ...e-inference-array-push-consecutive-phis.js | 1 + .../phi-type-inference-array-push.expect.md | 14 ++-- .../phi-type-inference-array-push.js | 1 + ...hi-type-inference-property-store.expect.md | 15 ++-- .../phi-type-inference-property-store.js | 2 +- ...properties-inside-optional-chain.expect.md | 3 +- ...tional-properties-inside-optional-chain.js | 1 + ...ming-unconditional-with-mutation.expect.md | 15 ++-- ...sa-renaming-unconditional-with-mutation.js | 1 + 28 files changed, 210 insertions(+), 124 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-break-labeled.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-break-labeled.expect.md index 76648c251a101..f778c74037d32 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-break-labeled.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-break-labeled.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +// @enablePropagateDepsInHIR /** * props.b *does* influence `a` */ @@ -29,13 +30,19 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; /** +import { c as _c } from "react/compiler-runtime"; // @enablePropagateDepsInHIR +/** * props.b *does* influence `a` */ function Component(props) { - const $ = _c(2); + const $ = _c(5); let a; - if ($[0] !== props) { + if ( + $[0] !== props.a || + $[1] !== props.b || + $[2] !== props.c || + $[3] !== props.d + ) { a = []; a.push(props.a); bb0: { @@ -47,10 +54,13 @@ function Component(props) { } a.push(props.d); - $[0] = props; - $[1] = a; + $[0] = props.a; + $[1] = props.b; + $[2] = props.c; + $[3] = props.d; + $[4] = a; } else { - a = $[1]; + a = $[4]; } return a; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-break-labeled.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-break-labeled.js index ccf983189cf71..770a4794fe075 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-break-labeled.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-break-labeled.js @@ -1,3 +1,4 @@ +// @enablePropagateDepsInHIR /** * props.b *does* influence `a` */ diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-early-return.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-early-return.expect.md index 82537902bfa19..602859389e3a1 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-early-return.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-early-return.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +// @enablePropagateDepsInHIR /** * props.b does *not* influence `a` */ @@ -66,14 +67,15 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; /** +import { c as _c } from "react/compiler-runtime"; // @enablePropagateDepsInHIR +/** * props.b does *not* influence `a` */ function ComponentA(props) { - const $ = _c(3); + const $ = _c(5); let a_DEBUG; let t0; - if ($[0] !== props) { + if ($[0] !== props.a || $[1] !== props.b || $[2] !== props.d) { t0 = Symbol.for("react.early_return_sentinel"); bb0: { a_DEBUG = []; @@ -85,12 +87,14 @@ function ComponentA(props) { a_DEBUG.push(props.d); } - $[0] = props; - $[1] = a_DEBUG; - $[2] = t0; + $[0] = props.a; + $[1] = props.b; + $[2] = props.d; + $[3] = a_DEBUG; + $[4] = t0; } else { - a_DEBUG = $[1]; - t0 = $[2]; + a_DEBUG = $[3]; + t0 = $[4]; } if (t0 !== Symbol.for("react.early_return_sentinel")) { return t0; @@ -102,9 +106,14 @@ function ComponentA(props) { * props.b *does* influence `a` */ function ComponentB(props) { - const $ = _c(2); + const $ = _c(5); let a; - if ($[0] !== props) { + if ( + $[0] !== props.a || + $[1] !== props.b || + $[2] !== props.c || + $[3] !== props.d + ) { a = []; a.push(props.a); if (props.b) { @@ -112,10 +121,13 @@ function ComponentB(props) { } a.push(props.d); - $[0] = props; - $[1] = a; + $[0] = props.a; + $[1] = props.b; + $[2] = props.c; + $[3] = props.d; + $[4] = a; } else { - a = $[1]; + a = $[4]; } return a; } @@ -124,10 +136,15 @@ function ComponentB(props) { * props.b *does* influence `a`, but only in a way that is never observable */ function ComponentC(props) { - const $ = _c(3); + const $ = _c(6); let a; let t0; - if ($[0] !== props) { + if ( + $[0] !== props.a || + $[1] !== props.b || + $[2] !== props.c || + $[3] !== props.d + ) { t0 = Symbol.for("react.early_return_sentinel"); bb0: { a = []; @@ -140,12 +157,15 @@ function ComponentC(props) { a.push(props.d); } - $[0] = props; - $[1] = a; - $[2] = t0; + $[0] = props.a; + $[1] = props.b; + $[2] = props.c; + $[3] = props.d; + $[4] = a; + $[5] = t0; } else { - a = $[1]; - t0 = $[2]; + a = $[4]; + t0 = $[5]; } if (t0 !== Symbol.for("react.early_return_sentinel")) { return t0; @@ -157,10 +177,15 @@ function ComponentC(props) { * props.b *does* influence `a` */ function ComponentD(props) { - const $ = _c(3); + const $ = _c(6); let a; let t0; - if ($[0] !== props) { + if ( + $[0] !== props.a || + $[1] !== props.b || + $[2] !== props.c || + $[3] !== props.d + ) { t0 = Symbol.for("react.early_return_sentinel"); bb0: { a = []; @@ -173,12 +198,15 @@ function ComponentD(props) { a.push(props.d); } - $[0] = props; - $[1] = a; - $[2] = t0; + $[0] = props.a; + $[1] = props.b; + $[2] = props.c; + $[3] = props.d; + $[4] = a; + $[5] = t0; } else { - a = $[1]; - t0 = $[2]; + a = $[4]; + t0 = $[5]; } if (t0 !== Symbol.for("react.early_return_sentinel")) { return t0; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-early-return.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-early-return.js index e0ed101409252..02edefc77f571 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-early-return.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-early-return.js @@ -1,3 +1,4 @@ +// @enablePropagateDepsInHIR /** * props.b does *not* influence `a` */ diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-on-mutable.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-on-mutable.expect.md index 24c565b088d2f..17ff728051ddb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-on-mutable.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-on-mutable.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +// @enablePropagateDepsInHIR function ComponentA(props) { const a = []; const b = []; @@ -34,11 +35,11 @@ function mayMutate() {} ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; +import { c as _c } from "react/compiler-runtime"; // @enablePropagateDepsInHIR function ComponentA(props) { - const $ = _c(2); + const $ = _c(4); let t0; - if ($[0] !== props) { + if ($[0] !== props.p0 || $[1] !== props.p1 || $[2] !== props.p2) { const a = []; const b = []; if (b) { @@ -49,18 +50,20 @@ function ComponentA(props) { } t0 = ; - $[0] = props; - $[1] = t0; + $[0] = props.p0; + $[1] = props.p1; + $[2] = props.p2; + $[3] = t0; } else { - t0 = $[1]; + t0 = $[3]; } return t0; } function ComponentB(props) { - const $ = _c(2); + const $ = _c(4); let t0; - if ($[0] !== props) { + if ($[0] !== props.p0 || $[1] !== props.p1 || $[2] !== props.p2) { const a = []; const b = []; if (mayMutate(b)) { @@ -71,10 +74,12 @@ function ComponentB(props) { } t0 = ; - $[0] = props; - $[1] = t0; + $[0] = props.p0; + $[1] = props.p1; + $[2] = props.p2; + $[3] = t0; } else { - t0 = $[1]; + t0 = $[3]; } return t0; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-on-mutable.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-on-mutable.js index 0378ab655c1ef..d03e36aaab56c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-on-mutable.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/conditional-on-mutable.js @@ -1,3 +1,4 @@ +// @enablePropagateDepsInHIR function ComponentA(props) { const a = []; const b = []; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-nested-early-return-within-reactive-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-nested-early-return-within-reactive-scope.expect.md index 2d33981f73fd1..476e1c017c655 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-nested-early-return-within-reactive-scope.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-nested-early-return-within-reactive-scope.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +// @enablePropagateDepsInHIR function Component(props) { let x = []; if (props.cond) { @@ -29,11 +30,11 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; +import { c as _c } from "react/compiler-runtime"; // @enablePropagateDepsInHIR function Component(props) { - const $ = _c(5); + const $ = _c(7); let t0; - if ($[0] !== props) { + if ($[0] !== props.cond || $[1] !== props.a || $[2] !== props.b) { t0 = Symbol.for("react.early_return_sentinel"); bb0: { const x = []; @@ -41,12 +42,12 @@ function Component(props) { x.push(props.a); if (props.b) { let t1; - if ($[2] !== props.b) { + if ($[4] !== props.b) { t1 = [props.b]; - $[2] = props.b; - $[3] = t1; + $[4] = props.b; + $[5] = t1; } else { - t1 = $[3]; + t1 = $[5]; } const y = t1; x.push(y); @@ -58,20 +59,22 @@ function Component(props) { break bb0; } else { let t1; - if ($[4] === Symbol.for("react.memo_cache_sentinel")) { + if ($[6] === Symbol.for("react.memo_cache_sentinel")) { t1 = foo(); - $[4] = t1; + $[6] = t1; } else { - t1 = $[4]; + t1 = $[6]; } t0 = t1; break bb0; } } - $[0] = props; - $[1] = t0; + $[0] = props.cond; + $[1] = props.a; + $[2] = props.b; + $[3] = t0; } else { - t0 = $[1]; + t0 = $[3]; } if (t0 !== Symbol.for("react.early_return_sentinel")) { return t0; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-nested-early-return-within-reactive-scope.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-nested-early-return-within-reactive-scope.js index 53eb06bc591e4..c8c24172d79e6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-nested-early-return-within-reactive-scope.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-nested-early-return-within-reactive-scope.js @@ -1,3 +1,4 @@ +// @enablePropagateDepsInHIR function Component(props) { let x = []; if (props.cond) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-within-reactive-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-within-reactive-scope.expect.md index 6c3525e9e77eb..bf54b52b59289 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-within-reactive-scope.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-within-reactive-scope.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +// @enablePropagateDepsInHIR import {makeArray} from 'shared-runtime'; function Component(props) { @@ -41,13 +42,13 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; +import { c as _c } from "react/compiler-runtime"; // @enablePropagateDepsInHIR import { makeArray } from "shared-runtime"; function Component(props) { - const $ = _c(4); + const $ = _c(6); let t0; - if ($[0] !== props) { + if ($[0] !== props.cond || $[1] !== props.a || $[2] !== props.b) { t0 = Symbol.for("react.early_return_sentinel"); bb0: { const x = []; @@ -57,21 +58,23 @@ function Component(props) { break bb0; } else { let t1; - if ($[2] !== props.b) { + if ($[4] !== props.b) { t1 = makeArray(props.b); - $[2] = props.b; - $[3] = t1; + $[4] = props.b; + $[5] = t1; } else { - t1 = $[3]; + t1 = $[5]; } t0 = t1; break bb0; } } - $[0] = props; - $[1] = t0; + $[0] = props.cond; + $[1] = props.a; + $[2] = props.b; + $[3] = t0; } else { - t0 = $[1]; + t0 = $[3]; } if (t0 !== Symbol.for("react.early_return_sentinel")) { return t0; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-within-reactive-scope.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-within-reactive-scope.js index 7446d76fb320c..256eb46bc9280 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-within-reactive-scope.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/early-return-within-reactive-scope.js @@ -1,3 +1,4 @@ +// @enablePropagateDepsInHIR import {makeArray} from 'shared-runtime'; function Component(props) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/error.todo-optional-call-chain-in-optional.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/error.todo-optional-call-chain-in-optional.expect.md index 75c5d61d407f0..8b52187920cbe 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/error.todo-optional-call-chain-in-optional.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/error.todo-optional-call-chain-in-optional.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +// @enablePropagateDepsInHIR function useFoo(props: {value: {x: string; y: string} | null}) { const value = props.value; return createArray(value?.x, value?.y)?.join(', '); @@ -22,13 +23,13 @@ export const FIXTURE_ENTRYPONT = { ## Error ``` - 1 | function useFoo(props: {value: {x: string; y: string} | null}) { - 2 | const value = props.value; -> 3 | return createArray(value?.x, value?.y)?.join(', '); - | ^^^^^^^^ Todo: Unexpected terminal kind `optional` for optional test block (3:3) - 4 | } - 5 | - 6 | function createArray(...args: Array): Array { + 2 | function useFoo(props: {value: {x: string; y: string} | null}) { + 3 | const value = props.value; +> 4 | return createArray(value?.x, value?.y)?.join(', '); + | ^^^^^^^^ Todo: Unexpected terminal kind `optional` for optional test block (4:4) + 5 | } + 6 | + 7 | function createArray(...args: Array): Array { ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/error.todo-optional-call-chain-in-optional.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/error.todo-optional-call-chain-in-optional.ts index 7ee50e0380288..0031bc770c678 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/error.todo-optional-call-chain-in-optional.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/error.todo-optional-call-chain-in-optional.ts @@ -1,3 +1,4 @@ +// @enablePropagateDepsInHIR function useFoo(props: {value: {x: string; y: string} | null}) { const value = props.value; return createArray(value?.x, value?.y)?.join(', '); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/iife-return-modified-later-phi.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/iife-return-modified-later-phi.expect.md index bed1c329f0d10..744824d0bd2c3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/iife-return-modified-later-phi.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/iife-return-modified-later-phi.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +// @enablePropagateDepsInHIR function Component(props) { const items = (() => { if (props.cond) { @@ -24,11 +25,11 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; +import { c as _c } from "react/compiler-runtime"; // @enablePropagateDepsInHIR function Component(props) { - const $ = _c(2); + const $ = _c(3); let items; - if ($[0] !== props) { + if ($[0] !== props.cond || $[1] !== props.a) { let t0; if (props.cond) { t0 = []; @@ -38,10 +39,11 @@ function Component(props) { items = t0; items?.push(props.a); - $[0] = props; - $[1] = items; + $[0] = props.cond; + $[1] = props.a; + $[2] = items; } else { - items = $[1]; + items = $[2]; } return items; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/iife-return-modified-later-phi.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/iife-return-modified-later-phi.js index f4f953d294e6f..4e8eb097de8a7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/iife-return-modified-later-phi.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/iife-return-modified-later-phi.js @@ -1,3 +1,4 @@ +// @enablePropagateDepsInHIR function Component(props) { const items = (() => { if (props.cond) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/object-mutated-in-consequent-alternate-both-return.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/object-mutated-in-consequent-alternate-both-return.expect.md index 8a20f9186b447..142fc9cefe64c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/object-mutated-in-consequent-alternate-both-return.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/object-mutated-in-consequent-alternate-both-return.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +// @enablePropagateDepsInHIR import {makeObject_Primitives} from 'shared-runtime'; function Component(props) { @@ -25,13 +26,13 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; +import { c as _c } from "react/compiler-runtime"; // @enablePropagateDepsInHIR import { makeObject_Primitives } from "shared-runtime"; function Component(props) { - const $ = _c(2); + const $ = _c(3); let t0; - if ($[0] !== props) { + if ($[0] !== props.cond || $[1] !== props.value) { t0 = Symbol.for("react.early_return_sentinel"); bb0: { const object = makeObject_Primitives(); @@ -45,10 +46,11 @@ function Component(props) { break bb0; } } - $[0] = props; - $[1] = t0; + $[0] = props.cond; + $[1] = props.value; + $[2] = t0; } else { - t0 = $[1]; + t0 = $[2]; } if (t0 !== Symbol.for("react.early_return_sentinel")) { return t0; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/object-mutated-in-consequent-alternate-both-return.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/object-mutated-in-consequent-alternate-both-return.js index 94a142d033708..2386448adfbbc 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/object-mutated-in-consequent-alternate-both-return.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/object-mutated-in-consequent-alternate-both-return.js @@ -1,3 +1,4 @@ +// @enablePropagateDepsInHIR import {makeObject_Primitives} from 'shared-runtime'; function Component(props) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/partial-early-return-within-reactive-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/partial-early-return-within-reactive-scope.expect.md index 398161f0c6a24..324eb714fc89b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/partial-early-return-within-reactive-scope.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/partial-early-return-within-reactive-scope.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +// @enablePropagateDepsInHIR function Component(props) { let x = []; let y = null; @@ -28,12 +29,12 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; +import { c as _c } from "react/compiler-runtime"; // @enablePropagateDepsInHIR function Component(props) { - const $ = _c(4); + const $ = _c(6); let y; let t0; - if ($[0] !== props) { + if ($[0] !== props.cond || $[1] !== props.a || $[2] !== props.b) { t0 = Symbol.for("react.early_return_sentinel"); bb0: { const x = []; @@ -43,11 +44,11 @@ function Component(props) { break bb0; } else { let t1; - if ($[3] === Symbol.for("react.memo_cache_sentinel")) { + if ($[5] === Symbol.for("react.memo_cache_sentinel")) { t1 = foo(); - $[3] = t1; + $[5] = t1; } else { - t1 = $[3]; + t1 = $[5]; } y = t1; if (props.b) { @@ -56,12 +57,14 @@ function Component(props) { } } } - $[0] = props; - $[1] = y; - $[2] = t0; + $[0] = props.cond; + $[1] = props.a; + $[2] = props.b; + $[3] = y; + $[4] = t0; } else { - y = $[1]; - t0 = $[2]; + y = $[3]; + t0 = $[4]; } if (t0 !== Symbol.for("react.early_return_sentinel")) { return t0; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/partial-early-return-within-reactive-scope.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/partial-early-return-within-reactive-scope.js index 66d68242b8bb6..d54f650c36bbe 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/partial-early-return-within-reactive-scope.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/partial-early-return-within-reactive-scope.js @@ -1,3 +1,4 @@ +// @enablePropagateDepsInHIR function Component(props) { let x = []; let y = null; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push-consecutive-phis.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push-consecutive-phis.expect.md index f17bcc92cb7ce..4ae80006678c6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push-consecutive-phis.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push-consecutive-phis.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +// @enablePropagateDepsInHIR import {makeArray} from 'shared-runtime'; function Component(props) { @@ -45,11 +46,11 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; +import { c as _c } from "react/compiler-runtime"; // @enablePropagateDepsInHIR import { makeArray } from "shared-runtime"; function Component(props) { - const $ = _c(3); + const $ = _c(6); let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { t0 = {}; @@ -59,7 +60,12 @@ function Component(props) { } const x = t0; let t1; - if ($[1] !== props) { + if ( + $[1] !== props.cond || + $[2] !== props.cond2 || + $[3] !== props.value || + $[4] !== props.value2 + ) { let y; if (props.cond) { if (props.cond2) { @@ -74,10 +80,13 @@ function Component(props) { y.push(x); t1 = [x, y]; - $[1] = props; - $[2] = t1; + $[1] = props.cond; + $[2] = props.cond2; + $[3] = props.value; + $[4] = props.value2; + $[5] = t1; } else { - t1 = $[2]; + t1 = $[5]; } return t1; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push-consecutive-phis.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push-consecutive-phis.js index 0a9aa39defea4..9173848b49b90 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push-consecutive-phis.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push-consecutive-phis.js @@ -1,3 +1,4 @@ +// @enablePropagateDepsInHIR import {makeArray} from 'shared-runtime'; function Component(props) { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push.expect.md index f58eed10fda5a..0b756a648ec23 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +// @enablePropagateDepsInHIR function Component(props) { const x = {}; let y; @@ -34,9 +35,9 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; +import { c as _c } from "react/compiler-runtime"; // @enablePropagateDepsInHIR function Component(props) { - const $ = _c(3); + const $ = _c(4); let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { t0 = {}; @@ -46,7 +47,7 @@ function Component(props) { } const x = t0; let t1; - if ($[1] !== props) { + if ($[1] !== props.cond || $[2] !== props.value) { let y; if (props.cond) { y = [props.value]; @@ -57,10 +58,11 @@ function Component(props) { y.push(x); t1 = [x, y]; - $[1] = props; - $[2] = t1; + $[1] = props.cond; + $[2] = props.value; + $[3] = t1; } else { - t1 = $[2]; + t1 = $[3]; } return t1; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push.js index 732a1524cba2d..0b60f4e44315d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-array-push.js @@ -1,3 +1,4 @@ +// @enablePropagateDepsInHIR function Component(props) { const x = {}; let y; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-property-store.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-property-store.expect.md index 70551c8e9d7b0..1bfaeb1c84a57 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-property-store.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-property-store.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @debug +// @debug @enablePropagateDepsInHIR function Component(props) { const x = {}; let y; @@ -30,9 +30,9 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; // @debug +import { c as _c } from "react/compiler-runtime"; // @debug @enablePropagateDepsInHIR function Component(props) { - const $ = _c(3); + const $ = _c(4); let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { t0 = {}; @@ -42,7 +42,7 @@ function Component(props) { } const x = t0; let t1; - if ($[1] !== props) { + if ($[1] !== props.cond || $[2] !== props.a) { let y; if (props.cond) { y = {}; @@ -53,10 +53,11 @@ function Component(props) { y.x = x; t1 = [x, y]; - $[1] = props; - $[2] = t1; + $[1] = props.cond; + $[2] = props.a; + $[3] = t1; } else { - t1 = $[2]; + t1 = $[3]; } return t1; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-property-store.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-property-store.js index ba7133ff808df..3611da08d2fbd 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-property-store.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/phi-type-inference-property-store.js @@ -1,4 +1,4 @@ -// @debug +// @debug @enablePropagateDepsInHIR function Component(props) { const x = {}; let y; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reactive-dependencies-non-optional-properties-inside-optional-chain.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reactive-dependencies-non-optional-properties-inside-optional-chain.expect.md index 6574bd1966ed9..6db3983d10751 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reactive-dependencies-non-optional-properties-inside-optional-chain.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reactive-dependencies-non-optional-properties-inside-optional-chain.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +// @enablePropagateDepsInHIR function Component(props) { return props.post.feedback.comments?.edges?.map(render); } @@ -11,7 +12,7 @@ function Component(props) { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; +import { c as _c } from "react/compiler-runtime"; // @enablePropagateDepsInHIR function Component(props) { const $ = _c(2); let t0; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reactive-dependencies-non-optional-properties-inside-optional-chain.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reactive-dependencies-non-optional-properties-inside-optional-chain.js index e8e0da392e9e7..58ad2a210fab5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reactive-dependencies-non-optional-properties-inside-optional-chain.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/reactive-dependencies-non-optional-properties-inside-optional-chain.js @@ -1,3 +1,4 @@ +// @enablePropagateDepsInHIR function Component(props) { return props.post.feedback.comments?.edges?.map(render); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/ssa-renaming-unconditional-with-mutation.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/ssa-renaming-unconditional-with-mutation.expect.md index f4689e5795552..1cb3cf2474b6a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/ssa-renaming-unconditional-with-mutation.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/ssa-renaming-unconditional-with-mutation.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +// @enablePropagateDepsInHIR import {mutate} from 'shared-runtime'; function useFoo(props) { @@ -35,13 +36,13 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; +import { c as _c } from "react/compiler-runtime"; // @enablePropagateDepsInHIR import { mutate } from "shared-runtime"; function useFoo(props) { - const $ = _c(2); + const $ = _c(4); let x; - if ($[0] !== props) { + if ($[0] !== props.bar || $[1] !== props.cond || $[2] !== props.foo) { x = []; x.push(props.bar); if (props.cond) { @@ -53,10 +54,12 @@ function useFoo(props) { } mutate(x); - $[0] = props; - $[1] = x; + $[0] = props.bar; + $[1] = props.cond; + $[2] = props.foo; + $[3] = x; } else { - x = $[1]; + x = $[3]; } return x; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/ssa-renaming-unconditional-with-mutation.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/ssa-renaming-unconditional-with-mutation.js index 3e7078cfc7999..99e28942268bd 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/ssa-renaming-unconditional-with-mutation.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/ssa-renaming-unconditional-with-mutation.js @@ -1,3 +1,4 @@ +// @enablePropagateDepsInHIR import {mutate} from 'shared-runtime'; function useFoo(props) { From 8c89fa76430b3d9fbecd6d535c93171d22f0377f Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Mon, 30 Sep 2024 12:24:20 -0400 Subject: [PATCH 4/8] [compiler][hir-rewrite] Infer non-null props, destructure source Followup from #30894. This adds a new flagged mode `enablePropagateScopeDepsInHIR: "enabled_with_optimizations"`, under which we infer more hoistable loads: - it's always safe to evaluate loads from `props` (i.e. first parameter of a `component`) - destructuring sources are safe to evaluate loads from (e.g. given `{x} = obj`, we infer that it's safe to evaluate obj.y) - computed load sources are safe to evaluate loads from (e.g. given `arr[0]`, we can infer that it's safe to evaluate arr.length) ghstack-source-id: 32f3bb72e9f85922825579bd785d636f4ccf724d Pull Request resolved: https://github.com/facebook/react/pull/31033 --- .../src/HIR/CollectHoistablePropertyLoads.ts | 139 ++++++++++++------ .../infer-component-props-non-null.expect.md | 60 ++++++++ .../infer-component-props-non-null.tsx | 20 +++ .../infer-non-null-destructure.expect.md | 91 ++++++++++++ .../infer-non-null-destructure.ts | 23 +++ 5 files changed, 289 insertions(+), 44 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/infer-component-props-non-null.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/infer-component-props-non-null.tsx create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/infer-non-null-destructure.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/infer-non-null-destructure.ts diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts index 941c60dea9d4f..ff50113b6660c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -8,6 +8,7 @@ import { HIRFunction, Identifier, IdentifierId, + InstructionId, Place, ReactiveScopeDependency, ScopeId, @@ -66,7 +67,7 @@ export function collectHoistablePropertyLoads( fn: HIRFunction, temporaries: ReadonlyMap, ): ReadonlyMap { - const nodes = collectPropertyLoadsInBlocks(fn, temporaries); + const nodes = collectNonNullsInBlocks(fn, temporaries); propagateNonNull(fn, nodes); const nodesKeyedByScopeId = new Map(); @@ -165,7 +166,7 @@ type PropertyLoadNode = class Tree { roots: Map = new Map(); - #getOrCreateRoot(identifier: Identifier): PropertyLoadNode { + getOrCreateRoot(identifier: Identifier): PropertyLoadNode { /** * Reads from a statically scoped variable are always safe in JS, * with the exception of TDZ (not addressed by this pass). @@ -207,17 +208,15 @@ class Tree { } getPropertyLoadNode(n: ReactiveScopeDependency): PropertyLoadNode { - CompilerError.invariant(n.path.length > 0, { - reason: - '[CollectHoistablePropertyLoads] Expected property node, found root node', - loc: GeneratedSource, - }); /** * We add ReactiveScopeDependencies according to instruction ordering, * so all subpaths of a PropertyLoad should already exist * (e.g. a.b is added before a.b.c), */ - let currNode = this.#getOrCreateRoot(n.identifier); + let currNode = this.getOrCreateRoot(n.identifier); + if (n.path.length === 0) { + return currNode; + } for (let i = 0; i < n.path.length - 1; i++) { currNode = assertNonNull(currNode.properties.get(n.path[i].property)); } @@ -226,10 +225,44 @@ class Tree { } } -function collectPropertyLoadsInBlocks( +function pushPropertyLoadNode( + loadSource: Identifier, + loadSourceNode: PropertyLoadNode, + instrId: InstructionId, + knownImmutableIdentifiers: Set, + result: Set, +): void { + /** + * Since this runs *after* buildReactiveScopeTerminals, identifier mutable ranges + * are not valid with respect to current instruction id numbering. + * We use attached reactive scope ranges as a proxy for mutable range, but this + * is an overestimate as (1) scope ranges merge and align to form valid program + * blocks and (2) passes like MemoizeFbtAndMacroOperands may assign scopes to + * non-mutable identifiers. + * + * See comment at top of function for why we track known immutable identifiers. + */ + const isMutableAtInstr = + loadSource.mutableRange.end > loadSource.mutableRange.start + 1 && + loadSource.scope != null && + inRange({id: instrId}, loadSource.scope.range); + if ( + !isMutableAtInstr || + knownImmutableIdentifiers.has(loadSourceNode.fullPath.identifier.id) + ) { + let curr: PropertyLoadNode | null = loadSourceNode; + while (curr != null) { + result.add(curr); + curr = curr.parent; + } + } +} + +function collectNonNullsInBlocks( fn: HIRFunction, temporaries: ReadonlyMap, ): ReadonlyMap { + const tree = new Tree(); /** * Due to current limitations of mutable range inference, there are edge cases in * which we infer known-immutable values (e.g. props or hook params) to have a @@ -238,53 +271,70 @@ function collectPropertyLoadsInBlocks( * We track known immutable identifiers to reduce regressions (as PropagateScopeDeps * is being rewritten to HIR). */ - const knownImmutableIdentifiers = new Set(); + const knownImmutableIdentifiers = new Set(); if (fn.fnType === 'Component' || fn.fnType === 'Hook') { for (const p of fn.params) { if (p.kind === 'Identifier') { - knownImmutableIdentifiers.add(p.identifier); + knownImmutableIdentifiers.add(p.identifier.id); } } } - const tree = new Tree(); + /** + * Known non-null objects such as functional component props can be safely + * read from any block. + */ + const knownNonNullIdentifiers = new Set(); + if ( + fn.fnType === 'Component' && + fn.params.length > 0 && + fn.params[0].kind === 'Identifier' + ) { + const identifier = fn.params[0].identifier; + knownNonNullIdentifiers.add(tree.getOrCreateRoot(identifier)); + } const nodes = new Map(); for (const [_, block] of fn.body.blocks) { - const assumedNonNullObjects = new Set(); + const assumedNonNullObjects = new Set( + knownNonNullIdentifiers, + ); for (const instr of block.instructions) { if (instr.value.kind === 'PropertyLoad') { - const property = getProperty( - instr.value.object, - instr.value.property, - temporaries, + const source = temporaries.get(instr.value.object.identifier.id) ?? { + identifier: instr.value.object.identifier, + path: [], + }; + pushPropertyLoadNode( + instr.value.object.identifier, + tree.getPropertyLoadNode(source), + instr.id, + knownImmutableIdentifiers, + assumedNonNullObjects, ); - const propertyNode = tree.getPropertyLoadNode(property); - const object = instr.value.object.identifier; - /** - * Since this runs *after* buildReactiveScopeTerminals, identifier mutable ranges - * are not valid with respect to current instruction id numbering. - * We use attached reactive scope ranges as a proxy for mutable range, but this - * is an overestimate as (1) scope ranges merge and align to form valid program - * blocks and (2) passes like MemoizeFbtAndMacroOperands may assign scopes to - * non-mutable identifiers. - * - * See comment at top of function for why we track known immutable identifiers. - */ - const isMutableAtInstr = - object.mutableRange.end > object.mutableRange.start + 1 && - object.scope != null && - inRange(instr, object.scope.range); - if ( - !isMutableAtInstr || - knownImmutableIdentifiers.has(propertyNode.fullPath.identifier) - ) { - let curr = propertyNode.parent; - while (curr != null) { - assumedNonNullObjects.add(curr); - curr = curr.parent; - } + } else if (instr.value.kind === 'Destructure') { + const source = instr.value.value.identifier.id; + const sourceNode = temporaries.get(source); + if (sourceNode != null) { + pushPropertyLoadNode( + instr.value.value.identifier, + tree.getPropertyLoadNode(sourceNode), + instr.id, + knownImmutableIdentifiers, + assumedNonNullObjects, + ); + } + } else if (instr.value.kind === 'ComputedLoad') { + const source = instr.value.object.identifier.id; + const sourceNode = temporaries.get(source); + if (sourceNode != null) { + pushPropertyLoadNode( + instr.value.object.identifier, + tree.getPropertyLoadNode(sourceNode), + instr.id, + knownImmutableIdentifiers, + assumedNonNullObjects, + ); } } - // TODO handle destructuring } nodes.set(block.id, { @@ -449,10 +499,11 @@ function propagateNonNull( ); for (const [id, node] of nodes) { - node.assumedNonNullObjects = Set_union( + const assumedNonNullObjects = Set_union( assertNonNull(fromEntry.get(id)), assertNonNull(fromExit.get(id)), ); + node.assumedNonNullObjects = assumedNonNullObjects; } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/infer-component-props-non-null.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/infer-component-props-non-null.expect.md new file mode 100644 index 0000000000000..da9ab95fb6b53 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/infer-component-props-non-null.expect.md @@ -0,0 +1,60 @@ + +## Input + +```javascript +// @enablePropagateDepsInHIR +import {identity, Stringify} from 'shared-runtime'; + +function Foo(props) { + /** + * props.value should be inferred as the dependency of this scope + * since we know that props is safe to read from (i.e. non-null) + * as it is arg[0] of a component function + */ + const arr = []; + if (cond) { + arr.push(identity(props.value)); + } + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{value: 2}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enablePropagateDepsInHIR +import { identity, Stringify } from "shared-runtime"; + +function Foo(props) { + const $ = _c(2); + let t0; + if ($[0] !== props.value) { + const arr = []; + if (cond) { + arr.push(identity(props.value)); + } + + t0 = ; + $[0] = props.value; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{ value: 2 }], +}; + +``` + +### Eval output +(kind: exception) cond is not defined \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/infer-component-props-non-null.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/infer-component-props-non-null.tsx new file mode 100644 index 0000000000000..d5aa3534d4dc3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/infer-component-props-non-null.tsx @@ -0,0 +1,20 @@ +// @enablePropagateDepsInHIR +import {identity, Stringify} from 'shared-runtime'; + +function Foo(props) { + /** + * props.value should be inferred as the dependency of this scope + * since we know that props is safe to read from (i.e. non-null) + * as it is arg[0] of a component function + */ + const arr = []; + if (cond) { + arr.push(identity(props.value)); + } + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{value: 2}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/infer-non-null-destructure.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/infer-non-null-destructure.expect.md new file mode 100644 index 0000000000000..faed26bc8aef4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/infer-non-null-destructure.expect.md @@ -0,0 +1,91 @@ + +## Input + +```javascript +// @enablePropagateDepsInHIR +import {identity, useIdentity} from 'shared-runtime'; + +function useFoo({arg, cond}: {arg: number; cond: boolean}) { + const maybeObj = useIdentity({value: arg}); + const {value} = maybeObj; + useIdentity(null); + /** + * maybeObj.value should be inferred as the dependency of this scope + * since we know that maybeObj is safe to read from (i.e. non-null) + * due to the above destructuring instruction + */ + const arr = []; + if (cond) { + arr.push(identity(maybeObj.value)); + } + return {arr, value}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{arg: 2, cond: false}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enablePropagateDepsInHIR +import { identity, useIdentity } from "shared-runtime"; + +function useFoo(t0) { + const $ = _c(10); + const { arg, cond } = t0; + let t1; + if ($[0] !== arg) { + t1 = { value: arg }; + $[0] = arg; + $[1] = t1; + } else { + t1 = $[1]; + } + const maybeObj = useIdentity(t1); + const { value } = maybeObj; + useIdentity(null); + let arr; + if ($[2] !== cond || $[3] !== maybeObj.value) { + arr = []; + if (cond) { + let t2; + if ($[5] !== maybeObj.value) { + t2 = identity(maybeObj.value); + $[5] = maybeObj.value; + $[6] = t2; + } else { + t2 = $[6]; + } + arr.push(t2); + } + $[2] = cond; + $[3] = maybeObj.value; + $[4] = arr; + } else { + arr = $[4]; + } + let t2; + if ($[7] !== arr || $[8] !== value) { + t2 = { arr, value }; + $[7] = arr; + $[8] = value; + $[9] = t2; + } else { + t2 = $[9]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ arg: 2, cond: false }], +}; + +``` + +### Eval output +(kind: ok) {"arr":[],"value":2} \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/infer-non-null-destructure.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/infer-non-null-destructure.ts new file mode 100644 index 0000000000000..f67991df7254e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/infer-non-null-destructure.ts @@ -0,0 +1,23 @@ +// @enablePropagateDepsInHIR +import {identity, useIdentity} from 'shared-runtime'; + +function useFoo({arg, cond}: {arg: number; cond: boolean}) { + const maybeObj = useIdentity({value: arg}); + const {value} = maybeObj; + useIdentity(null); + /** + * maybeObj.value should be inferred as the dependency of this scope + * since we know that maybeObj is safe to read from (i.e. non-null) + * due to the above destructuring instruction + */ + const arr = []; + if (cond) { + arr.push(identity(maybeObj.value)); + } + return {arr, value}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{arg: 2, cond: false}], +}; From 58a3ca3b47f6a51cea48ea95ded26c9887baca38 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Mon, 30 Sep 2024 12:24:21 -0400 Subject: [PATCH 5/8] [compiler][hir-rewrite] Cleanup Identifier -> IdentifierId Since removing ExitSSA, Identifier and IdentifierId should mean the same thing ghstack-source-id: 076cacbe8360e716b0555088043502823f9ee72e Pull Request resolved: https://github.com/facebook/react/pull/31034 --- .../src/HIR/CollectHoistablePropertyLoads.ts | 66 ++----------------- .../src/HIR/PropagateScopeDependenciesHIR.ts | 49 +++++++++++++- 2 files changed, 53 insertions(+), 62 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts index ff50113b6660c..4f642e57a65f3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -9,7 +9,6 @@ import { Identifier, IdentifierId, InstructionId, - Place, ReactiveScopeDependency, ScopeId, } from './HIR'; @@ -88,61 +87,6 @@ export type BlockInfo = { assumedNonNullObjects: ReadonlySet; }; -export function getProperty( - object: Place, - propertyName: string, - temporaries: ReadonlyMap, -): ReactiveScopeDependency { - /* - * (1) Get the base object either from the temporary sidemap (e.g. a LoadLocal) - * or a deep copy of an existing property dependency. - * Example 1: - * $0 = LoadLocal x - * $1 = PropertyLoad $0.y - * getProperty($0, ...) -> resolvedObject = x, resolvedDependency = null - * - * Example 2: - * $0 = LoadLocal x - * $1 = PropertyLoad $0.y - * $2 = PropertyLoad $1.z - * getProperty($1, ...) -> resolvedObject = null, resolvedDependency = x.y - * - * Example 3: - * $0 = Call(...) - * $1 = PropertyLoad $0.y - * getProperty($0, ...) -> resolvedObject = null, resolvedDependency = null - */ - const resolvedDependency = temporaries.get(object.identifier.id); - - /** - * (2) Push the last PropertyLoad - * TODO(mofeiZ): understand optional chaining - */ - let property: ReactiveScopeDependency; - if (resolvedDependency == null) { - property = { - identifier: object.identifier, - path: [{property: propertyName, optional: false}], - }; - } else { - property = { - identifier: resolvedDependency.identifier, - path: [ - ...resolvedDependency.path, - {property: propertyName, optional: false}, - ], - }; - } - return property; -} - -export function resolveTemporary( - place: Place, - temporaries: ReadonlyMap, -): Identifier { - return temporaries.get(place.identifier.id) ?? place.identifier; -} - /** * Tree data structure to dedupe property loads (e.g. a.b.c) * and make computing sets intersections simpler. @@ -152,7 +96,7 @@ type RootNode = { parent: null; // Recorded to make later computations simpler fullPath: ReactiveScopeDependency; - root: Identifier; + root: IdentifierId; }; type PropertyLoadNode = @@ -164,18 +108,18 @@ type PropertyLoadNode = | RootNode; class Tree { - roots: Map = new Map(); + roots: Map = new Map(); getOrCreateRoot(identifier: Identifier): PropertyLoadNode { /** * Reads from a statically scoped variable are always safe in JS, * with the exception of TDZ (not addressed by this pass). */ - let rootNode = this.roots.get(identifier); + let rootNode = this.roots.get(identifier.id); if (rootNode === undefined) { rootNode = { - root: identifier, + root: identifier.id, properties: new Map(), fullPath: { identifier, @@ -183,7 +127,7 @@ class Tree { }, parent: null, }; - this.roots.set(identifier, rootNode); + this.roots.set(identifier.id, rootNode); } return rootNode; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts index 4c7dac004d80a..1fe218c352818 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts @@ -20,7 +20,6 @@ import { import { BlockInfo, collectHoistablePropertyLoads, - getProperty, } from './CollectHoistablePropertyLoads'; import { ScopeBlockTraversal, @@ -220,6 +219,54 @@ function collectTemporariesSidemap( return temporaries; } +function getProperty( + object: Place, + propertyName: string, + temporaries: ReadonlyMap, +): ReactiveScopeDependency { + /* + * (1) Get the base object either from the temporary sidemap (e.g. a LoadLocal) + * or a deep copy of an existing property dependency. + * Example 1: + * $0 = LoadLocal x + * $1 = PropertyLoad $0.y + * getProperty($0, ...) -> resolvedObject = x, resolvedDependency = null + * + * Example 2: + * $0 = LoadLocal x + * $1 = PropertyLoad $0.y + * $2 = PropertyLoad $1.z + * getProperty($1, ...) -> resolvedObject = null, resolvedDependency = x.y + * + * Example 3: + * $0 = Call(...) + * $1 = PropertyLoad $0.y + * getProperty($0, ...) -> resolvedObject = null, resolvedDependency = null + */ + const resolvedDependency = temporaries.get(object.identifier.id); + + /** + * (2) Push the last PropertyLoad + * TODO(mofeiZ): understand optional chaining + */ + let property: ReactiveScopeDependency; + if (resolvedDependency == null) { + property = { + identifier: object.identifier, + path: [{property: propertyName, optional: false}], + }; + } else { + property = { + identifier: resolvedDependency.identifier, + path: [ + ...resolvedDependency.path, + {property: propertyName, optional: false}, + ], + }; + } + return property; +} + type Decl = { id: InstructionId; scope: Stack; From 5d12e9e10b9957bc131ec77e013e1a76e4f32eb6 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Mon, 30 Sep 2024 12:24:22 -0400 Subject: [PATCH 6/8] [compiler] repro for dep merging edge case (non-hir) Found when writing #31037, summary copied from comments: This is an extreme edge case and not code we'd expect any reasonable developer to write. In most cases e.g. `(a?.b != null ? a.b : DEFAULT)`, we do want to take a dependency on `a?.b`. I found this trying to come up with edge cases that break the current dependency + CFG merging logic. I think it makes sense to error on the side of correctness. After all, we still take `a` as a dependency if users write `a != null ? a.b : DEFAULT`, and the same fix (understanding the ` != null` test expression) works for both. Can be convinced otherwise though! ghstack-source-id: cc06afda59f7681e228495f5e35a596c20f875f5 Pull Request resolved: https://github.com/facebook/react/pull/31035 --- ...e-uncond-optional-chain-and-cond.expect.md | 88 +++++++++++++++++++ ...ug-merge-uncond-optional-chain-and-cond.ts | 31 +++++++ .../packages/snap/src/SproutTodoFilter.ts | 1 + 3 files changed, 120 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/bug-merge-uncond-optional-chain-and-cond.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/bug-merge-uncond-optional-chain-and-cond.ts diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/bug-merge-uncond-optional-chain-and-cond.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/bug-merge-uncond-optional-chain-and-cond.expect.md new file mode 100644 index 0000000000000..9a95e7dc875b4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/bug-merge-uncond-optional-chain-and-cond.expect.md @@ -0,0 +1,88 @@ + +## Input + +```javascript +import {identity} from 'shared-runtime'; + +/** + * Evaluator failure: + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) {} + * [[ (exception in render) TypeError: Cannot read properties of null (reading 'title_text') ]] + * Forget: + * (kind: ok) {} + * {} + */ +/** + * Very contrived text fixture showing that it's technically incorrect to merge + * a conditional dependency (e.g. dep.path in `cond ? dep.path : ...`) and an + * unconditionally evaluated optional chain (`dep?.path`). + * + * + * when screen is non-null, useFoo returns { title: null } or "(not null)" + * when screen is null, useFoo throws + */ +function useFoo({screen}: {screen: null | undefined | {title_text: null}}) { + return screen?.title_text != null + ? '(not null)' + : identity({title: screen.title_text}); +} +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{screen: null}], + sequentialRenders: [{screen: {title_bar: undefined}}, {screen: null}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { identity } from "shared-runtime"; + +/** + * Evaluator failure: + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) {} + * [[ (exception in render) TypeError: Cannot read properties of null (reading 'title_text') ]] + * Forget: + * (kind: ok) {} + * {} + */ +/** + * Very contrived text fixture showing that it's technically incorrect to merge + * a conditional dependency (e.g. dep.path in `cond ? dep.path : ...`) and an + * unconditionally evaluated optional chain (`dep?.path`). + * + * + * when screen is non-null, useFoo returns { title: null } or "(not null)" + * when screen is null, useFoo throws + */ +function useFoo(t0) { + const $ = _c(2); + const { screen } = t0; + let t1; + if ($[0] !== screen?.title_text) { + t1 = + screen?.title_text != null + ? "(not null)" + : identity({ title: screen.title_text }); + $[0] = screen?.title_text; + $[1] = t1; + } else { + t1 = $[1]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ screen: null }], + sequentialRenders: [{ screen: { title_bar: undefined } }, { screen: null }], +}; + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/bug-merge-uncond-optional-chain-and-cond.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/bug-merge-uncond-optional-chain-and-cond.ts new file mode 100644 index 0000000000000..bb361e3c9fefd --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/bug-merge-uncond-optional-chain-and-cond.ts @@ -0,0 +1,31 @@ +import {identity} from 'shared-runtime'; + +/** + * Evaluator failure: + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) {} + * [[ (exception in render) TypeError: Cannot read properties of null (reading 'title_text') ]] + * Forget: + * (kind: ok) {} + * {} + */ +/** + * Very contrived text fixture showing that it's technically incorrect to merge + * a conditional dependency (e.g. dep.path in `cond ? dep.path : ...`) and an + * unconditionally evaluated optional chain (`dep?.path`). + * + * + * when screen is non-null, useFoo returns { title: null } or "(not null)" + * when screen is null, useFoo throws + */ +function useFoo({screen}: {screen: null | undefined | {title_text: null}}) { + return screen?.title_text != null + ? '(not null)' + : identity({title: screen.title_text}); +} +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{screen: null}], + sequentialRenders: [{screen: {title_bar: undefined}}, {screen: null}], +}; diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index 7140cad2f74bc..0ae22a643b326 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -478,6 +478,7 @@ const skipFilter = new Set([ 'fbt/bug-fbt-plural-multiple-function-calls', 'fbt/bug-fbt-plural-multiple-mixed-call-tag', 'bug-invalid-hoisting-functionexpr', + 'reduce-reactive-deps/bug-merge-uncond-optional-chain-and-cond', 'original-reactive-scopes-fork/bug-nonmutating-capture-in-unsplittable-memo-block', 'original-reactive-scopes-fork/bug-hoisted-declaration-with-scope', 'bug-codegen-inline-iife', From 2cbea245cca4044f02c4c231a7f86c8062074579 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Mon, 30 Sep 2024 12:24:23 -0400 Subject: [PATCH 7/8] [compiler][fixtures] Patch error-handling edge case in snap evaluator Fix edge case in which we incorrectly returned a cached exception instead of trying to rerender with new props. ghstack-source-id: 843fb85df4a2ae7a88f296104fb16b5f9a34c76e Pull Request resolved: https://github.com/facebook/react/pull/31082 --- .../throw-before-scope-starts.expect.md | 2 +- .../packages/snap/src/sprout/evaluator.ts | 45 +++++++++++++------ 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/jump-unpoisoned/throw-before-scope-starts.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/jump-unpoisoned/throw-before-scope-starts.expect.md index 4f04ef7fdc1ba..dd7509c48c4c4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/jump-unpoisoned/throw-before-scope-starts.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/jump-unpoisoned/throw-before-scope-starts.expect.md @@ -75,7 +75,7 @@ export const FIXTURE_ENTRYPOINT = { ### Eval output (kind: ok) [[ (exception in render) Error: throw with error! ]] -[[ (exception in render) Error: throw with error! ]] +[2] [[ (exception in render) Error: throw with error! ]] [[ (exception in render) TypeError: Cannot read properties of undefined (reading 'b') ]] [null] diff --git a/compiler/packages/snap/src/sprout/evaluator.ts b/compiler/packages/snap/src/sprout/evaluator.ts index 27cbe55858f60..77e8044fb5ed5 100644 --- a/compiler/packages/snap/src/sprout/evaluator.ts +++ b/compiler/packages/snap/src/sprout/evaluator.ts @@ -60,6 +60,7 @@ const ExportSchema = z.object({ FIXTURE_ENTRYPOINT: EntrypointSchema, }); +const NO_ERROR_SENTINEL = Symbol(); /** * Wraps WrapperTestComponent in an error boundary to simplify re-rendering * when an exception is thrown. @@ -67,31 +68,47 @@ const ExportSchema = z.object({ */ class WrapperTestComponentWithErrorBoundary extends React.Component< {fn: any; params: Array}, - {hasError: boolean; error: any} + {errorFromLastRender: any} > { - propsErrorMap: MutableRefObject>; + /** + * Limit retries of the child component by caching seen errors. + */ + propsErrorMap: Map; + lastProps: any | null; + // lastProps: object | null; constructor(props: any) { super(props); - this.state = {hasError: false, error: null}; - this.propsErrorMap = React.createRef() as MutableRefObject>; - this.propsErrorMap.current = new Map(); + this.lastProps = null; + this.propsErrorMap = new Map(); + this.state = { + errorFromLastRender: NO_ERROR_SENTINEL, + }; } static getDerivedStateFromError(error: any) { - return {hasError: true, error: error}; + // Reschedule a second render that immediately returns the cached error + return {errorFromLastRender: error}; } override componentDidUpdate() { - if (this.state.hasError) { - this.setState({hasError: false, error: null}); + if (this.state.errorFromLastRender !== NO_ERROR_SENTINEL) { + // Reschedule a third render that immediately returns the cached error + this.setState({errorFromLastRender: NO_ERROR_SENTINEL}); } } override render() { - if (this.state.hasError) { - this.propsErrorMap.current!.set( - this.props, - `[[ (exception in render) ${this.state.error?.toString()} ]]`, - ); + if ( + this.state.errorFromLastRender !== NO_ERROR_SENTINEL && + this.props === this.lastProps + ) { + /** + * The last render errored, cache the error message to avoid running the + * test fixture more than once + */ + const errorMsg = `[[ (exception in render) ${this.state.errorFromLastRender?.toString()} ]]`; + this.propsErrorMap.set(this.lastProps, errorMsg); + return errorMsg; } - const cachedError = this.propsErrorMap.current!.get(this.props); + this.lastProps = this.props; + const cachedError = this.propsErrorMap.get(this.props); if (cachedError != null) { return cachedError; } From c67e241c1656dea4ece22a4ee5c25b6b36d0ca75 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Mon, 30 Sep 2024 12:24:24 -0400 Subject: [PATCH 8/8] [compiler] Renames and no-op refactor for next PR Rename for clarity: - `CollectHoistablePropertyLoads:Tree` -> `CollectHoistablePropertyLoads:PropertyPathRegistry` - `getPropertyLoadNode` -> `getOrCreateProperty` - `getOrCreateRoot` -> `getOrCreateIdentifier` - `PropertyLoadNode` -> `PropertyPathNode` Refactor to CFG joining logic for `CollectHoistablePropertyLoads`. We now write to the same set of inferredNonNullObjects when traversing from entry and exit blocks. This is more correct, as non-nulls inferred from a forward traversal should be included when computing the backward traversal (and vice versa). This fix is needed by an edge case in #31036 Added invariant into fixed-point iteration to terminate (instead of infinite looping). ghstack-source-id: 1e8eb2d566b649ede93de9a9c13dad09b96416a5 Pull Request resolved: https://github.com/facebook/react/pull/31036 --- .../src/HIR/CollectHoistablePropertyLoads.ts | 160 ++++++++---------- .../src/HIR/DeriveMinimalDependenciesHIR.ts | 25 ++- .../src/HIR/HIR.ts | 18 +- 3 files changed, 95 insertions(+), 108 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts index 4f642e57a65f3..cb778c329226b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -4,6 +4,7 @@ import {Set_intersect, Set_union, getOrInsertDefault} from '../Utils/utils'; import { BasicBlock, BlockId, + DependencyPathEntry, GeneratedSource, HIRFunction, Identifier, @@ -66,7 +67,9 @@ export function collectHoistablePropertyLoads( fn: HIRFunction, temporaries: ReadonlyMap, ): ReadonlyMap { - const nodes = collectNonNullsInBlocks(fn, temporaries); + const registry = new PropertyPathRegistry(); + + const nodes = collectNonNullsInBlocks(fn, temporaries, registry); propagateNonNull(fn, nodes); const nodesKeyedByScopeId = new Map(); @@ -84,33 +87,33 @@ export function collectHoistablePropertyLoads( export type BlockInfo = { block: BasicBlock; - assumedNonNullObjects: ReadonlySet; + assumedNonNullObjects: ReadonlySet; }; /** - * Tree data structure to dedupe property loads (e.g. a.b.c) + * PropertyLoadRegistry data structure to dedupe property loads (e.g. a.b.c) * and make computing sets intersections simpler. */ type RootNode = { - properties: Map; + properties: Map; parent: null; // Recorded to make later computations simpler fullPath: ReactiveScopeDependency; root: IdentifierId; }; -type PropertyLoadNode = +type PropertyPathNode = | { - properties: Map; - parent: PropertyLoadNode; + properties: Map; + parent: PropertyPathNode; fullPath: ReactiveScopeDependency; } | RootNode; -class Tree { +class PropertyPathRegistry { roots: Map = new Map(); - getOrCreateRoot(identifier: Identifier): PropertyLoadNode { + getOrCreateIdentifier(identifier: Identifier): PropertyPathNode { /** * Reads from a statically scoped variable are always safe in JS, * with the exception of TDZ (not addressed by this pass). @@ -132,49 +135,61 @@ class Tree { return rootNode; } - static #getOrCreateProperty( - node: PropertyLoadNode, - property: string, - ): PropertyLoadNode { - let child = node.properties.get(property); + static getOrCreatePropertyEntry( + parent: PropertyPathNode, + entry: DependencyPathEntry, + ): PropertyPathNode { + if (entry.optional) { + CompilerError.throwTodo({ + reason: 'handle optional nodes', + loc: GeneratedSource, + }); + } + let child = parent.properties.get(entry.property); if (child == null) { child = { properties: new Map(), - parent: node, + parent: parent, fullPath: { - identifier: node.fullPath.identifier, - path: node.fullPath.path.concat([{property, optional: false}]), + identifier: parent.fullPath.identifier, + path: parent.fullPath.path.concat(entry), }, }; - node.properties.set(property, child); + parent.properties.set(entry.property, child); } return child; } - getPropertyLoadNode(n: ReactiveScopeDependency): PropertyLoadNode { + getOrCreateProperty(n: ReactiveScopeDependency): PropertyPathNode { /** * We add ReactiveScopeDependencies according to instruction ordering, * so all subpaths of a PropertyLoad should already exist * (e.g. a.b is added before a.b.c), */ - let currNode = this.getOrCreateRoot(n.identifier); + let currNode = this.getOrCreateIdentifier(n.identifier); if (n.path.length === 0) { return currNode; } for (let i = 0; i < n.path.length - 1; i++) { - currNode = assertNonNull(currNode.properties.get(n.path[i].property)); + currNode = PropertyPathRegistry.getOrCreatePropertyEntry( + currNode, + n.path[i], + ); } - return Tree.#getOrCreateProperty(currNode, n.path.at(-1)!.property); + return PropertyPathRegistry.getOrCreatePropertyEntry( + currNode, + n.path.at(-1)!, + ); } } -function pushPropertyLoadNode( - loadSource: Identifier, - loadSourceNode: PropertyLoadNode, +function addNonNullPropertyPath( + source: Identifier, + sourceNode: PropertyPathNode, instrId: InstructionId, knownImmutableIdentifiers: Set, - result: Set, + result: Set, ): void { /** * Since this runs *after* buildReactiveScopeTerminals, identifier mutable ranges @@ -187,26 +202,22 @@ function pushPropertyLoadNode( * See comment at top of function for why we track known immutable identifiers. */ const isMutableAtInstr = - loadSource.mutableRange.end > loadSource.mutableRange.start + 1 && - loadSource.scope != null && - inRange({id: instrId}, loadSource.scope.range); + source.mutableRange.end > source.mutableRange.start + 1 && + source.scope != null && + inRange({id: instrId}, source.scope.range); if ( !isMutableAtInstr || - knownImmutableIdentifiers.has(loadSourceNode.fullPath.identifier.id) + knownImmutableIdentifiers.has(sourceNode.fullPath.identifier.id) ) { - let curr: PropertyLoadNode | null = loadSourceNode; - while (curr != null) { - result.add(curr); - curr = curr.parent; - } + result.add(sourceNode); } } function collectNonNullsInBlocks( fn: HIRFunction, temporaries: ReadonlyMap, + registry: PropertyPathRegistry, ): ReadonlyMap { - const tree = new Tree(); /** * Due to current limitations of mutable range inference, there are edge cases in * which we infer known-immutable values (e.g. props or hook params) to have a @@ -227,18 +238,18 @@ function collectNonNullsInBlocks( * Known non-null objects such as functional component props can be safely * read from any block. */ - const knownNonNullIdentifiers = new Set(); + const knownNonNullIdentifiers = new Set(); if ( fn.fnType === 'Component' && fn.params.length > 0 && fn.params[0].kind === 'Identifier' ) { const identifier = fn.params[0].identifier; - knownNonNullIdentifiers.add(tree.getOrCreateRoot(identifier)); + knownNonNullIdentifiers.add(registry.getOrCreateIdentifier(identifier)); } const nodes = new Map(); for (const [_, block] of fn.body.blocks) { - const assumedNonNullObjects = new Set( + const assumedNonNullObjects = new Set( knownNonNullIdentifiers, ); for (const instr of block.instructions) { @@ -247,9 +258,9 @@ function collectNonNullsInBlocks( identifier: instr.value.object.identifier, path: [], }; - pushPropertyLoadNode( + addNonNullPropertyPath( instr.value.object.identifier, - tree.getPropertyLoadNode(source), + registry.getOrCreateProperty(source), instr.id, knownImmutableIdentifiers, assumedNonNullObjects, @@ -258,9 +269,9 @@ function collectNonNullsInBlocks( const source = instr.value.value.identifier.id; const sourceNode = temporaries.get(source); if (sourceNode != null) { - pushPropertyLoadNode( + addNonNullPropertyPath( instr.value.value.identifier, - tree.getPropertyLoadNode(sourceNode), + registry.getOrCreateProperty(sourceNode), instr.id, knownImmutableIdentifiers, assumedNonNullObjects, @@ -270,9 +281,9 @@ function collectNonNullsInBlocks( const source = instr.value.object.identifier.id; const sourceNode = temporaries.get(source); if (sourceNode != null) { - pushPropertyLoadNode( + addNonNullPropertyPath( instr.value.object.identifier, - tree.getPropertyLoadNode(sourceNode), + registry.getOrCreateProperty(sourceNode), instr.id, knownImmutableIdentifiers, assumedNonNullObjects, @@ -314,7 +325,6 @@ function propagateNonNull( nodeId: BlockId, direction: 'forward' | 'backward', traversalState: Map, - nonNullObjectsByBlock: Map>, ): boolean { /** * Avoid re-visiting computed or currently active nodes, which can @@ -345,7 +355,6 @@ function propagateNonNull( pred, direction, traversalState, - nonNullObjectsByBlock, ); changed ||= neighborChanged; } @@ -374,38 +383,36 @@ function propagateNonNull( const neighborAccesses = Set_intersect( Array.from(neighbors) .filter(n => traversalState.get(n) === 'done') - .map(n => assertNonNull(nonNullObjectsByBlock.get(n))), + .map(n => assertNonNull(nodes.get(n)).assumedNonNullObjects), ); - const prevObjects = assertNonNull(nonNullObjectsByBlock.get(nodeId)); - const newObjects = Set_union(prevObjects, neighborAccesses); + const prevObjects = assertNonNull(nodes.get(nodeId)).assumedNonNullObjects; + const mergedObjects = Set_union(prevObjects, neighborAccesses); - nonNullObjectsByBlock.set(nodeId, newObjects); + assertNonNull(nodes.get(nodeId)).assumedNonNullObjects = mergedObjects; traversalState.set(nodeId, 'done'); - changed ||= prevObjects.size !== newObjects.size; + changed ||= prevObjects.size !== mergedObjects.size; return changed; } - const fromEntry = new Map>(); - const fromExit = new Map>(); - for (const [blockId, blockInfo] of nodes) { - fromEntry.set(blockId, blockInfo.assumedNonNullObjects); - fromExit.set(blockId, blockInfo.assumedNonNullObjects); - } const traversalState = new Map(); const reversedBlocks = [...fn.body.blocks]; reversedBlocks.reverse(); - let i = 0; let changed; + let i = 0; do { - i++; + CompilerError.invariant(i++ < 100, { + reason: + '[CollectHoistablePropertyLoads] fixed point iteration did not terminate after 100 loops', + loc: GeneratedSource, + }); + changed = false; for (const [blockId] of fn.body.blocks) { const forwardChanged = recursivelyPropagateNonNull( blockId, 'forward', traversalState, - fromEntry, ); changed ||= forwardChanged; } @@ -415,43 +422,14 @@ function propagateNonNull( blockId, 'backward', traversalState, - fromExit, ); changed ||= backwardChanged; } traversalState.clear(); } while (changed); - - /** - * TODO: validate against meta internal code, then remove in future PR. - * Currently cannot come up with a case that requires fixed-point iteration. - */ - CompilerError.invariant(i <= 2, { - reason: 'require fixed-point iteration', - description: `#iterations = ${i}`, - loc: GeneratedSource, - }); - - CompilerError.invariant( - fromEntry.size === fromExit.size && fromEntry.size === nodes.size, - { - reason: - 'bad sizes after calculating fromEntry + fromExit ' + - `${fromEntry.size} ${fromExit.size} ${nodes.size}`, - loc: GeneratedSource, - }, - ); - - for (const [id, node] of nodes) { - const assumedNonNullObjects = Set_union( - assertNonNull(fromEntry.get(id)), - assertNonNull(fromExit.get(id)), - ); - node.assumedNonNullObjects = assumedNonNullObjects; - } } -function assertNonNull, U>( +export function assertNonNull, U>( value: T | null | undefined, source?: string, ): T { diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts index ecc1844b006aa..f2bb0b31f05c9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts @@ -19,16 +19,17 @@ const ENABLE_DEBUG_INVARIANTS = true; export class ReactiveScopeDependencyTreeHIR { #roots: Map = new Map(); - #getOrCreateRoot(identifier: Identifier, isNonNull: boolean): DependencyNode { + #getOrCreateRoot( + identifier: Identifier, + accessType: PropertyAccessType, + ): DependencyNode { // roots can always be accessed unconditionally in JS let rootNode = this.#roots.get(identifier); if (rootNode === undefined) { rootNode = { properties: new Map(), - accessType: isNonNull - ? PropertyAccessType.NonNullAccess - : PropertyAccessType.Access, + accessType, }; this.#roots.set(identifier, rootNode); } @@ -37,7 +38,7 @@ export class ReactiveScopeDependencyTreeHIR { addDependency(dep: ReactiveScopePropertyDependency): void { const {path} = dep; - let currNode = this.#getOrCreateRoot(dep.identifier, false); + let currNode = this.#getOrCreateRoot(dep.identifier, MIN_ACCESS_TYPE); const accessType = PropertyAccessType.Access; @@ -45,8 +46,11 @@ export class ReactiveScopeDependencyTreeHIR { for (const property of path) { // all properties read 'on the way' to a dependency are marked as 'access' - let currChild = getOrMakeProperty(currNode, property.property); - currChild.accessType = merge(currChild.accessType, accessType); + let currChild = makeOrMergeProperty( + currNode, + property.property, + accessType, + ); currNode = currChild; } @@ -251,17 +255,20 @@ function printSubtree( return results; } -function getOrMakeProperty( +function makeOrMergeProperty( node: DependencyNode, property: string, + accessType: PropertyAccessType, ): DependencyNode { let child = node.properties.get(property); if (child == null) { child = { properties: new Map(), - accessType: MIN_ACCESS_TYPE, + accessType, }; node.properties.set(property, child); + } else { + child.accessType = merge(child.accessType, accessType); } return child; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts index 30408ab032b35..615ec18feb962 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -874,13 +874,7 @@ export type InstructionValue = }; loc: SourceLocation; } - | { - kind: 'StoreLocal'; - lvalue: LValue; - value: Place; - type: t.FlowType | t.TSType | null; - loc: SourceLocation; - } + | StoreLocal | { kind: 'StoreContext'; lvalue: { @@ -1123,6 +1117,13 @@ export type Primitive = { export type JSXText = {kind: 'JSXText'; value: string; loc: SourceLocation}; +export type StoreLocal = { + kind: 'StoreLocal'; + lvalue: LValue; + value: Place; + type: t.FlowType | t.TSType | null; + loc: SourceLocation; +}; export type PropertyLoad = { kind: 'PropertyLoad'; object: Place; @@ -1496,7 +1497,8 @@ export type ReactiveScopeDeclaration = { scope: ReactiveScope; // the scope in which the variable was originally declared }; -export type DependencyPath = Array<{property: string; optional: boolean}>; +export type DependencyPathEntry = {property: string; optional: boolean}; +export type DependencyPath = Array; export type ReactiveScopeDependency = { identifier: Identifier; path: DependencyPath;