@samitouri / QOS-React / commits / 3770c11011

[compiler] repro for reactive ref.current accesses (#31519)

See test fixture --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31519). * #31521 * __->__ #31519

mofeiZ committed Nov 12, 2024 at 14:04 UTC 3770c11011c78d227c7013468787e0c34ae91dc8
3 files changed +123
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-nonreactive-ref.expect.md new
+89
@@ -0,0 +1,89 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +import {useRef} from 'react';
6 +import {Stringify} from 'shared-runtime';
7 +
8 +/**
9 + * Bug: we're currently filtering out `ref.current` dependencies in
10 + * `propagateScopeDependencies:checkValidDependency`. This is incorrect.
11 + * Instead, we should always take a dependency on ref values (the outer box) as
12 + * they may be reactive. Pruning should be done in
13 + * `pruneNonReactiveDependencies`
14 + *
15 + * Found differences in evaluator results
16 + * Non-forget (expected):
17 + * (kind: ok)
18 + * <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
19 + * <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
20 + * Forget:
21 + * (kind: ok)
22 + * <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
23 + * <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
24 + */
25 +function Component({cond}) {
26 + const ref1 = useRef(1);
27 + const ref2 = useRef(2);
28 + const ref = cond ? ref1 : ref2;
29 + const cb = () => ref.current;
30 + return <Stringify cb={cb} shouldInvokeFns={true} />;
31 +}
32 +
33 +export const FIXTURE_ENTRYPOINT = {
34 + fn: Component,
35 + params: [{cond: true}],
36 + sequentialRenders: [{cond: true}, {cond: false}],
37 +};
38 +
39 +```
40 +
41 +## Code
42 +
43 +```javascript
44 +import { c as _c } from "react/compiler-runtime";
45 +import { useRef } from "react";
46 +import { Stringify } from "shared-runtime";
47 +
48 +/**
49 + * Bug: we're currently filtering out `ref.current` dependencies in
50 + * `propagateScopeDependencies:checkValidDependency`. This is incorrect.
51 + * Instead, we should always take a dependency on ref values (the outer box) as
52 + * they may be reactive. Pruning should be done in
53 + * `pruneNonReactiveDependencies`
54 + *
55 + * Found differences in evaluator results
56 + * Non-forget (expected):
57 + * (kind: ok)
58 + * <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
59 + * <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
60 + * Forget:
61 + * (kind: ok)
62 + * <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
63 + * <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
64 + */
65 +function Component(t0) {
66 + const $ = _c(1);
67 + const { cond } = t0;
68 + const ref1 = useRef(1);
69 + const ref2 = useRef(2);
70 + const ref = cond ? ref1 : ref2;
71 + let t1;
72 + if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
73 + const cb = () => ref.current;
74 + t1 = <Stringify cb={cb} shouldInvokeFns={true} />;
75 + $[0] = t1;
76 + } else {
77 + t1 = $[0];
78 + }
79 + return t1;
80 +}
81 +
82 +export const FIXTURE_ENTRYPOINT = {
83 + fn: Component,
84 + params: [{ cond: true }],
85 + sequentialRenders: [{ cond: true }, { cond: false }],
86 +};
87 +
88 +```
89 +
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-nonreactive-ref.tsx new
+33
@@ -0,0 +1,33 @@
1 +import {useRef} from 'react';
2 +import {Stringify} from 'shared-runtime';
3 +
4 +/**
5 + * Bug: we're currently filtering out `ref.current` dependencies in
6 + * `propagateScopeDependencies:checkValidDependency`. This is incorrect.
7 + * Instead, we should always take a dependency on ref values (the outer box) as
8 + * they may be reactive. Pruning should be done in
9 + * `pruneNonReactiveDependencies`
10 + *
11 + * Found differences in evaluator results
12 + * Non-forget (expected):
13 + * (kind: ok)
14 + * <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
15 + * <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
16 + * Forget:
17 + * (kind: ok)
18 + * <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
19 + * <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
20 + */
21 +function Component({cond}) {
22 + const ref1 = useRef(1);
23 + const ref2 = useRef(2);
24 + const ref = cond ? ref1 : ref2;
25 + const cb = () => ref.current;
26 + return <Stringify cb={cb} shouldInvokeFns={true} />;
27 +}
28 +
29 +export const FIXTURE_ENTRYPOINT = {
30 + fn: Component,
31 + params: [{cond: true}],
32 + sequentialRenders: [{cond: true}, {cond: false}],
33 +};
compiler/packages/snap/src/SproutTodoFilter.ts
+1
@@ -484,6 +484,7 @@ const skipFilter = new Set([
484 'bug-aliased-capture-mutate',
485 'bug-functiondecl-hoisting',
486 'bug-try-catch-maybe-null-dependency',
487 + 'bug-nonreactive-ref',
488 'reduce-reactive-deps/bug-infer-function-cond-access-not-hoisted',
489 'bug-invalid-phi-as-dependency',
490 'reduce-reactive-deps/bug-merge-uncond-optional-chain-and-cond',