Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compiler: Allow global mutation in jsx props #29591

Merged
merged 2 commits into from
May 29, 2024
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 @@ -1103,13 +1103,56 @@ function inferBlock(
break;
}
case "JsxExpression": {
valueKind = {
if (instrValue.tag.kind === "Identifier") {
state.referenceAndRecordEffects(
instrValue.tag,
Effect.Freeze,
ValueReason.JsxCaptured,
functionEffects
);
}
if (instrValue.children !== null) {
for (const child of instrValue.children) {
state.referenceAndRecordEffects(
child,
Effect.Freeze,
ValueReason.JsxCaptured,
functionEffects
);
}
}
for (const attr of instrValue.props) {
if (attr.kind === "JsxSpreadAttribute") {
state.referenceAndRecordEffects(
attr.argument,
Effect.Freeze,
ValueReason.JsxCaptured,
functionEffects
);
} else {
const propEffects: Array<FunctionEffect> = [];
state.referenceAndRecordEffects(
attr.place,
Effect.Freeze,
ValueReason.JsxCaptured,
propEffects
);
functionEffects.push(
...propEffects.filter(
(propEffect) => propEffect.kind !== "GlobalMutation"
)
);
}
}

state.initialize(instrValue, {
kind: ValueKind.Frozen,
reason: new Set([ValueReason.Other]),
context: new Set(),
};
effect = { kind: Effect.Freeze, reason: ValueReason.JsxCaptured };
break;
});
state.define(instr.lvalue, instrValue);
instr.lvalue.effect = Effect.ConditionallyMutate;
continue;
}
case "JsxFragment": {
valueKind = {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@

## Input

```javascript
import { useMemo } from "react";

const someGlobal = { value: 0 };

function Component({ value }) {
const onClick = () => {
someGlobal.value = value;
};
return useMemo(() => {
return <div onClick={onClick}>{someGlobal.value}</div>;
}, []);
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ value: 0 }],
sequentialRenders: [
{ value: 1 },
{ value: 1 },
{ value: 42 },
{ value: 42 },
{ value: 0 },
],
};

```

## Code

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

const someGlobal = { value: 0 };

function Component(t0) {
const $ = _c(4);
const { value } = t0;
let t1;
if ($[0] !== value) {
t1 = () => {
someGlobal.value = value;
};
$[0] = value;
$[1] = t1;
} else {
t1 = $[1];
}
const onClick = t1;
let t2;
let t3;
if ($[2] !== onClick) {
t3 = <div onClick={onClick}>{someGlobal.value}</div>;
$[2] = onClick;
$[3] = t3;
} else {
t3 = $[3];
}
t2 = t3;
return t2;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ value: 0 }],
sequentialRenders: [
{ value: 1 },
{ value: 1 },
{ value: 42 },
{ value: 42 },
{ value: 0 },
],
};

```

### Eval output
(kind: ok) <div>0</div>
<div>0</div>
<div>0</div>
<div>0</div>
<div>0</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { useMemo } from "react";

const someGlobal = { value: 0 };

function Component({ value }) {
const onClick = () => {
someGlobal.value = value;
};
return useMemo(() => {
return <div onClick={onClick}>{someGlobal.value}</div>;
}, []);
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{ value: 0 }],
sequentialRenders: [
{ value: 1 },
{ value: 1 },
{ value: 42 },
{ value: 42 },
{ value: 0 },
],
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@

## Input

```javascript
function Component() {
const onClick = () => {
// Cannot assign to globals
someUnknownGlobal = true;
moduleLocal = true;
};
// It's possible that this could be an event handler / effect function,
// but we don't know that and optimistically assume it will only be
// called by an event handler or effect, where it is allowed to modify globals
return <div onClick={onClick} />;
}

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

```

## Code

```javascript
import { c as _c } from "react/compiler-runtime";
function Component() {
const $ = _c(1);
let t0;
if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
const onClick = () => {
someUnknownGlobal = true;
moduleLocal = true;
};

t0 = <div onClick={onClick} />;
$[0] = t0;
} else {
t0 = $[0];
}
return t0;
}

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

```

### Eval output
(kind: ok) <div></div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
function Component() {
const onClick = () => {
// Cannot assign to globals
someUnknownGlobal = true;
moduleLocal = true;
};
// It's possible that this could be an event handler / effect function,
// but we don't know that and optimistically assume it will only be
// called by an event handler or effect, where it is allowed to modify globals
return <div onClick={onClick} />;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{}],
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@

## Input

```javascript
function Component() {
const Foo = () => {
someGlobal = true;
};
return <Foo />;
}

```


## Error

```
1 | function Component() {
2 | const Foo = () => {
> 3 | someGlobal = true;
| ^^^^^^^^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (3:3)
4 | };
5 | return <Foo />;
6 | }
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
function Component() {
const Foo = () => {
someGlobal = true;
};
return <Foo />;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@

## Input

```javascript
function Component() {
const foo = () => {
someGlobal = true;
};
// Children are generally access/called during render, so
// modifying a global in a children function is almost
// certainly a mistake.
return <Foo>{foo}</Foo>;
}

```


## Error

```
1 | function Component() {
2 | const foo = () => {
> 3 | someGlobal = true;
| ^^^^^^^^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (3:3)
4 | };
5 | // Children are generally access/called during render, so
6 | // modifying a global in a children function is almost
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
function Component() {
const foo = () => {
someGlobal = true;
};
// Children are generally access/called during render, so
// modifying a global in a children function is almost
// certainly a mistake.
return <Foo>{foo}</Foo>;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@

## Input

```javascript
function Component() {
const foo = () => {
someGlobal = true;
};
return <div {...foo} />;
}

```


## Error

```
1 | function Component() {
2 | const foo = () => {
> 3 | someGlobal = true;
| ^^^^^^^^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (3:3)
4 | };
5 | return <div {...foo} />;
6 | }
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
function Component() {
const foo = () => {
someGlobal = true;
};
return <div {...foo} />;
}

This file was deleted.

Loading