From d923cf4684efdb1187b78049b0b09b69eeab1898 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Tue, 5 Nov 2024 15:16:19 -0500 Subject: [PATCH] [compiler] Add fixture for objectexpr computed key bug We were bailing out on complex computed-key syntax (prior to #31344) as we assumed that this caused bugs (due to inferring computed key rvalues to have `freeze` effects). This fixture shows that this bailout is unrelated to the underlying bug --- ...nstruction-hoisted-sequence-expr.expect.md | 109 ++++++++++++++++++ ...fter-construction-hoisted-sequence-expr.js | 31 +++++ .../packages/snap/src/SproutTodoFilter.ts | 1 + 3 files changed, 141 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.expect.md new file mode 100644 index 0000000000000..dcca6cddd8fb2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.expect.md @@ -0,0 +1,109 @@ + +## Input + +```javascript +import {identity, mutate} from 'shared-runtime'; + +/** + * Bug: copy of error.todo-object-expression-computed-key-modified-during-after-construction-sequence-expr + * with the mutation hoisted to a named variable instead of being directly + * inlined into the Object key. + * + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] + * [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] + * Forget: + * (kind: ok) [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] + * [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe","wat2":"joe"}] + */ +function Component(props) { + const key = {}; + const tmp = (mutate(key), key); + const context = { + // Here, `tmp` is frozen (as it's inferred to be a primitive/string) + [tmp]: identity([props.value]), + }; + mutate(key); + return [context, key]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 42}], + sequentialRenders: [{value: 42}, {value: 42}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { identity, mutate } from "shared-runtime"; + +/** + * Bug: copy of error.todo-object-expression-computed-key-modified-during-after-construction-sequence-expr + * with the mutation hoisted to a named variable instead of being directly + * inlined into the Object key. + * + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] + * [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] + * Forget: + * (kind: ok) [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] + * [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe","wat2":"joe"}] + */ +function Component(props) { + const $ = _c(8); + let t0; + let key; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + key = {}; + t0 = (mutate(key), key); + $[0] = t0; + $[1] = key; + } else { + t0 = $[0]; + key = $[1]; + } + const tmp = t0; + let t1; + if ($[2] !== props.value) { + t1 = identity([props.value]); + $[2] = props.value; + $[3] = t1; + } else { + t1 = $[3]; + } + let t2; + if ($[4] !== t1) { + t2 = { [tmp]: t1 }; + $[4] = t1; + $[5] = t2; + } else { + t2 = $[5]; + } + const context = t2; + + mutate(key); + let t3; + if ($[6] !== context) { + t3 = [context, key]; + $[6] = context; + $[7] = t3; + } else { + t3 = $[7]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: 42 }], + sequentialRenders: [{ value: 42 }, { value: 42 }], +}; + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.js new file mode 100644 index 0000000000000..94befbdd17b77 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.js @@ -0,0 +1,31 @@ +import {identity, mutate} from 'shared-runtime'; + +/** + * Bug: copy of error.todo-object-expression-computed-key-modified-during-after-construction-sequence-expr + * with the mutation hoisted to a named variable instead of being directly + * inlined into the Object key. + * + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] + * [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] + * Forget: + * (kind: ok) [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] + * [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe","wat2":"joe"}] + */ +function Component(props) { + const key = {}; + const tmp = (mutate(key), key); + const context = { + // Here, `tmp` is frozen (as it's inferred to be a primitive/string) + [tmp]: identity([props.value]), + }; + mutate(key); + return [context, key]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 42}], + sequentialRenders: [{value: 42}, {value: 42}], +}; diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index 351f242e40820..76914f1dd28a5 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -478,6 +478,7 @@ const skipFilter = new Set([ // bugs 'fbt/bug-fbt-plural-multiple-function-calls', 'fbt/bug-fbt-plural-multiple-mixed-call-tag', + 'bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr', 'bug-invalid-hoisting-functionexpr', 'bug-try-catch-maybe-null-dependency', 'reduce-reactive-deps/bug-infer-function-cond-access-not-hoisted',