Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -389,14 +389,6 @@ export function findDisjointMutableValues(
*/
operand.identifier.mutableRange.start > 0
) {
if (
instr.value.kind === 'FunctionExpression' ||
instr.value.kind === 'ObjectMethod'
) {
if (operand.identifier.type.kind === 'Primitive') {
continue;
}
}
Comment on lines -392 to -399
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intuitively this makes sense: we don't usually create scopes for primitives, but if we assigned a scope then it should follow all the regular merging logic of this pass

operands.push(operand.identifier);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@

## Input

```javascript
import {useState} from 'react';

/**
* Repro for https://github.com/facebook/react/issues/35122
*
* InferReactiveScopeVariables was excluding primitive operands
* when considering operands for merging. We previously did not
* infer types for context variables (StoreContext etc), but later
* started inferring types in cases of `const` context variables,
* since the type cannot change.
*
* In this example, this meant that we skipped the `isExpired`
* operand of the onClick function expression when considering
* scopes to merge.
*/
function Test1() {
const [expire, setExpire] = useState(5);

const onClick = () => {
// Reference to isExpired prior to declaration
console.log('isExpired', isExpired);
};

const isExpired = expire === 0;

return <div onClick={onClick}>{expire}</div>;
}

export const FIXTURE_ENTRYPOINT = {
fn: Test1,
params: [{}],
};

```

## Code

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

/**
* Repro for https://github.com/facebook/react/issues/35122
*
* InferReactiveScopeVariables was excluding primitive operands
* when considering operands for merging. We previously did not
* infer types for context variables (StoreContext etc), but later
* started inferring types in cases of `const` context variables,
* since the type cannot change.
*
* In this example, this meant that we skipped the `isExpired`
* operand of the onClick function expression when considering
* scopes to merge.
*/
function Test1() {
const $ = _c(5);
const [expire] = useState(5);
let onClick;
if ($[0] !== expire) {
onClick = () => {
console.log("isExpired", isExpired);
};

const isExpired = expire === 0;
$[0] = expire;
$[1] = onClick;
} else {
onClick = $[1];
}
let t0;
if ($[2] !== expire || $[3] !== onClick) {
t0 = <div onClick={onClick}>{expire}</div>;
$[2] = expire;
$[3] = onClick;
$[4] = t0;
} else {
t0 = $[4];
}
return t0;
}

export const FIXTURE_ENTRYPOINT = {
fn: Test1,
params: [{}],
};

```
### Eval output
(kind: ok) <div>5</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import {useState} from 'react';

/**
* Repro for https://github.com/facebook/react/issues/35122
*
* InferReactiveScopeVariables was excluding primitive operands
* when considering operands for merging. We previously did not
* infer types for context variables (StoreContext etc), but later
* started inferring types in cases of `const` context variables,
* since the type cannot change.
*
* In this example, this meant that we skipped the `isExpired`
* operand of the onClick function expression when considering
* scopes to merge.
*/
function Test1() {
const [expire, setExpire] = useState(5);

const onClick = () => {
// Reference to isExpired prior to declaration
console.log('isExpired', isExpired);
};

const isExpired = expire === 0;

return <div onClick={onClick}>{expire}</div>;
}

export const FIXTURE_ENTRYPOINT = {
fn: Test1,
params: [{}],
};
Loading