Skip to content

Commit

Permalink
[compiler] repro for reactive ref.current accesses (#31519)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mofeiZ authored Nov 12, 2024
1 parent d9b3841 commit 3770c11
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@

## Input

```javascript
import {useRef} from 'react';
import {Stringify} from 'shared-runtime';

/**
* Bug: we're currently filtering out `ref.current` dependencies in
* `propagateScopeDependencies:checkValidDependency`. This is incorrect.
* Instead, we should always take a dependency on ref values (the outer box) as
* they may be reactive. Pruning should be done in
* `pruneNonReactiveDependencies`
*
* Found differences in evaluator results
* Non-forget (expected):
* (kind: ok)
* <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
* <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
* Forget:
* (kind: ok)
* <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
* <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
*/
function Component({cond}) {
const ref1 = useRef(1);
const ref2 = useRef(2);
const ref = cond ? ref1 : ref2;
const cb = () => ref.current;
return <Stringify cb={cb} shouldInvokeFns={true} />;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{cond: true}],
sequentialRenders: [{cond: true}, {cond: false}],
};

```

## Code

```javascript
import { c as _c } from "react/compiler-runtime";
import { useRef } from "react";
import { Stringify } from "shared-runtime";

/**
* Bug: we're currently filtering out `ref.current` dependencies in
* `propagateScopeDependencies:checkValidDependency`. This is incorrect.
* Instead, we should always take a dependency on ref values (the outer box) as
* they may be reactive. Pruning should be done in
* `pruneNonReactiveDependencies`
*
* Found differences in evaluator results
* Non-forget (expected):
* (kind: ok)
* <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
* <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
* Forget:
* (kind: ok)
* <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
* <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
*/
function Component(t0) {
const $ = _c(1);
const { cond } = t0;
const ref1 = useRef(1);
const ref2 = useRef(2);
const ref = cond ? ref1 : ref2;
let t1;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
const cb = () => ref.current;
t1 = <Stringify cb={cb} shouldInvokeFns={true} />;
$[0] = t1;
} else {
t1 = $[0];
}
return t1;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ cond: true }],
sequentialRenders: [{ cond: true }, { cond: false }],
};

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import {useRef} from 'react';
import {Stringify} from 'shared-runtime';

/**
* Bug: we're currently filtering out `ref.current` dependencies in
* `propagateScopeDependencies:checkValidDependency`. This is incorrect.
* Instead, we should always take a dependency on ref values (the outer box) as
* they may be reactive. Pruning should be done in
* `pruneNonReactiveDependencies`
*
* Found differences in evaluator results
* Non-forget (expected):
* (kind: ok)
* <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
* <div>{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}</div>
* Forget:
* (kind: ok)
* <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
* <div>{"cb":{"kind":"Function","result":1},"shouldInvokeFns":true}</div>
*/
function Component({cond}) {
const ref1 = useRef(1);
const ref2 = useRef(2);
const ref = cond ? ref1 : ref2;
const cb = () => ref.current;
return <Stringify cb={cb} shouldInvokeFns={true} />;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{cond: true}],
sequentialRenders: [{cond: true}, {cond: false}],
};
1 change: 1 addition & 0 deletions compiler/packages/snap/src/SproutTodoFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,7 @@ const skipFilter = new Set([
'bug-aliased-capture-mutate',
'bug-functiondecl-hoisting',
'bug-try-catch-maybe-null-dependency',
'bug-nonreactive-ref',
'reduce-reactive-deps/bug-infer-function-cond-access-not-hoisted',
'bug-invalid-phi-as-dependency',
'reduce-reactive-deps/bug-merge-uncond-optional-chain-and-cond',
Expand Down

0 comments on commit 3770c11

Please sign in to comment.