Skip to content
Closed
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
@@ -0,0 +1,49 @@

## Input

```javascript
import {useEffect, useState} from 'react';
import {Stringify} from 'shared-runtime';

function Foo() {
/**
* Previously, this lowered to
* $1 = LoadContext capture setState
* $2 = FunctionExpression deps=$1 context=setState
* [[ at this point, we freeze the `LoadContext setState` instruction, but it will never be referenced again ]]
*
* Now, this function expression directly references `setState`, which freezes
* the source `DeclareContext HoistedConst setState`. Freezing source identifiers
* (instead of the one level removed `LoadContext`) is more semantically correct
* for everything *other* than hoisted context declarations.
*
* $2 = Function context=setState
*/
useEffect(() => setState(2), []);

const [state, setState] = useState(0);
return <Stringify state={state} />;
}

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

```


## Error

```
18 | useEffect(() => setState(2), []);
19 |
> 20 | const [state, setState] = useState(0);
| ^^^^^^^^ InvalidReact: Updating a value used previously in an effect function or as an effect dependency is not allowed. Consider moving the mutation before calling useEffect(). Found mutation of `setState` (20:20)
21 | return <Stringify state={state} />;
22 | }
23 |
```


Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ function useFoo() {
> 6 | cache.set('key', 'value');
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 7 | });
| ^^^^ InvalidReact: This argument is a function which modifies local variables when called, which can bypass memoization and cause the UI not to update. Functions that are returned from hooks, passed as arguments to hooks, or passed as props to components may not mutate local variables (5:7)
| ^^^^ InvalidReact: This argument is a function which may reassign or mutate local variables after render, which can cause inconsistent behavior on subsequent renders. Consider using state instead (5:7)

InvalidReact: The function modifies a local variable here (6:6)
8 | }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@

## Input

```javascript
import {Stringify, useIdentity} from 'shared-runtime';

function Component({prop1, prop2}) {
'use memo';

const data = useIdentity(
new Map([
[0, 'value0'],
[1, 'value1'],
])
);
let i = 0;
const items = [];
items.push(
<Stringify
key={i}
onClick={() => data.get(i) + prop1}
shouldInvokeFns={true}
/>
);
i = i + 1;
Comment on lines +21 to +25
Copy link
Member Author

Choose a reason for hiding this comment

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

the reassignment is mutating a value captured by jsx, this should have been rejected

items.push(
<Stringify
key={i}
onClick={() => data.get(i) + prop2}
shouldInvokeFns={true}
/>
);
return <>{items}</>;
}

export const FIXTURE_ENTRYPOINT = {
fn: Component,
params: [{prop1: 'prop1', prop2: 'prop2'}],
sequentialRenders: [
{prop1: 'prop1', prop2: 'prop2'},
{prop1: 'prop1', prop2: 'prop2'},
{prop1: 'changed', prop2: 'prop2'},
],
};

```


## Error

```
19 | />
20 | );
> 21 | i = i + 1;
| ^ InvalidReact: Updating a value used previously in JSX is not allowed. Consider moving the mutation before the JSX. Found mutation of `i` (21:21)
22 | items.push(
23 | <Stringify
24 | key={i}
```


Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function Component() {
5 | cache.set('key', 'value');
6 | };
> 7 | return <Foo fn={fn} />;
| ^^ InvalidReact: This argument is a function which modifies local variables when called, which can bypass memoization and cause the UI not to update. Functions that are returned from hooks, passed as arguments to hooks, or passed as props to components may not mutate local variables (7:7)
| ^^ InvalidReact: This argument is a function which may reassign or mutate local variables after render, which can cause inconsistent behavior on subsequent renders. Consider using state instead (7:7)

InvalidReact: The function modifies a local variable here (5:5)
8 | }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,16 @@ function useFoo() {
## Error

```
1 | function useFoo() {
2 | let x = 0;
3 | return value => {
> 3 | return value => {
| ^^^^^^^^^^
> 4 | x = value;
| ^ InvalidReact: Reassigning a variable after render has completed can cause inconsistent behavior on subsequent renders. Consider using state instead. Variable `x` cannot be reassigned after render (4:4)
5 | };
| ^^^^^^^^^^^^^^
> 5 | };
| ^^^^ InvalidReact: This argument is a function which may reassign or mutate local variables after render, which can cause inconsistent behavior on subsequent renders. Consider using state instead (3:5)

InvalidReact: The function modifies a local variable here (4:4)
6 | }
7 |
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,19 @@ function Component() {
## Error

```
5 |
6 | const reassignLocal = newValue => {
> 7 | local = newValue;
| ^^^^^ InvalidReact: Reassigning a variable after render has completed can cause inconsistent behavior on subsequent renders. Consider using state instead. Variable `local` cannot be reassigned after render (7:7)
8 | };
9 |
10 | const onMount = newValue => {
31 | };
32 |
> 33 | useEffect(() => {
| ^^^^^^^
> 34 | onMount();
| ^^^^^^^^^^^^^^
> 35 | }, [onMount]);
| ^^^^ InvalidReact: This argument is a function which may reassign or mutate local variables after render, which can cause inconsistent behavior on subsequent renders. Consider using state instead (33:35)

InvalidReact: The function modifies a local variable here (7:7)
36 |
37 | return 'ok';
38 | }
```


Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,19 @@ function Component() {
## Error

```
6 |
7 | const reassignLocal = newValue => {
> 8 | local = newValue;
| ^^^^^ InvalidReact: Reassigning a variable after render has completed can cause inconsistent behavior on subsequent renders. Consider using state instead. Variable `local` cannot be reassigned after render (8:8)
9 | };
10 |
11 | const callback = newValue => {
32 | };
33 |
> 34 | useIdentity(() => {
| ^^^^^^^
> 35 | callback();
| ^^^^^^^^^^^^^^^
> 36 | });
| ^^^^ InvalidReact: This argument is a function which may reassign or mutate local variables after render, which can cause inconsistent behavior on subsequent renders. Consider using state instead (34:36)

InvalidReact: The function modifies a local variable here (8:8)
37 |
38 | return 'ok';
39 | }
```


Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,14 @@ function Component() {
## Error

```
3 |
4 | const reassignLocal = newValue => {
> 5 | local = newValue;
| ^^^^^ InvalidReact: Reassigning a variable after render has completed can cause inconsistent behavior on subsequent renders. Consider using state instead. Variable `local` cannot be reassigned after render (5:5)
6 | };
7 |
8 | const onClick = newValue => {
29 | };
30 |
> 31 | return <button onClick={onClick}>Submit</button>;
| ^^^^^^^ InvalidReact: This argument is a function which may reassign or mutate local variables after render, which can cause inconsistent behavior on subsequent renders. Consider using state instead (31:31)

InvalidReact: The function modifies a local variable here (5:5)
32 | }
33 |
```


Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ function useFoo() {
> 8 | cache.set('key', 'value');
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 9 | };
| ^^^^ InvalidReact: This argument is a function which modifies local variables when called, which can bypass memoization and cause the UI not to update. Functions that are returned from hooks, passed as arguments to hooks, or passed as props to components may not mutate local variables (7:9)
| ^^^^ InvalidReact: This argument is a function which may reassign or mutate local variables after render, which can cause inconsistent behavior on subsequent renders. Consider using state instead (7:9)

InvalidReact: The function modifies a local variable here (8:8)
10 | }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@

## Input

```javascript
// @flow
/**
* This hook returns a function that when called with an input object,
* will return the result of mapping that input with the supplied map
* function. Results are cached, so if the same input is passed again,
* the same output object will be returned.
*
* Note that this technically violates the rules of React and is unsafe:
* hooks must return immutable objects and be pure, and a function which
* captures and mutates a value when called is inherently not pure.
*
* However, in this case it is technically safe _if_ the mapping function
* is pure *and* the resulting objects are never modified. This is because
* the function only caches: the result of `returnedFunction(someInput)`
* strictly depends on `returnedFunction` and `someInput`, and cannot
* otherwise change over time.
*/
hook useMemoMap<TInput: interface {}, TOutput>(
map: TInput => TOutput
): TInput => TOutput {
return useMemo(() => {
// The original issue is that `cache` was not memoized together with the returned
// function. This was because neither appears to ever be mutated — the function
// is known to mutate `cache` but the function isn't called.
//
// The fix is to detect cases like this — functions that are mutable but not called -
// and ensure that their mutable captures are aliased together into the same scope.
const cache = new WeakMap<TInput, TOutput>();
return input => {
let output = cache.get(input);
if (output == null) {
output = map(input);
cache.set(input, output);
}
return output;
};
}, [map]);
}

```


## Error

```
19 | map: TInput => TOutput
20 | ): TInput => TOutput {
> 21 | return useMemo(() => {
| ^^^^^^^^^^^^^^^
> 22 | // The original issue is that `cache` was not memoized together with the returned
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 23 | // function. This was because neither appears to ever be mutated — the function
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 24 | // is known to mutate `cache` but the function isn't called.
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 25 | //
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 26 | // The fix is to detect cases like this — functions that are mutable but not called -
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 27 | // and ensure that their mutable captures are aliased together into the same scope.
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 28 | const cache = new WeakMap<TInput, TOutput>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 29 | return input => {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 30 | let output = cache.get(input);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 31 | if (output == null) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 32 | output = map(input);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 33 | cache.set(input, output);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 34 | }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 35 | return output;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 36 | };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 37 | }, [map]);
| ^^^^^^^^^^^^ InvalidReact: This argument is a function which may reassign or mutate local variables after render, which can cause inconsistent behavior on subsequent renders. Consider using state instead (21:37)

InvalidReact: The function modifies a local variable here (33:33)
38 | }
39 |
```


Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ function Component(props) {
a.property = true;
b.push(false);
};
return <div onClick={f()} />;
return <div onClick={f} />;
}

export const FIXTURE_ENTRYPOINT = {
Expand All @@ -32,13 +32,15 @@ export const FIXTURE_ENTRYPOINT = {
## Error

```
6 | const f = () => {
7 | if (cond) {
> 8 | a = {};
| ^ InvalidReact: Reassigning a variable after render has completed can cause inconsistent behavior on subsequent renders. Consider using state instead. Variable `a` cannot be reassigned after render (8:8)
9 | b = [];
10 | } else {
11 | a = {};
15 | b.push(false);
16 | };
> 17 | return <div onClick={f} />;
| ^ InvalidReact: This argument is a function which may reassign or mutate local variables after render, which can cause inconsistent behavior on subsequent renders. Consider using state instead (17:17)

InvalidReact: The function modifies a local variable here (8:8)
18 | }
19 |
20 | export const FIXTURE_ENTRYPOINT = {
```


Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ function useHook(a, b) {
1 | function useHook(a, b) {
> 2 | b.test = 1;
| ^ InvalidReact: Mutating component props or hook arguments is not allowed. Consider using a local variable instead (2:2)

InvalidReact: Mutating component props or hook arguments is not allowed. Consider using a local variable instead (3:3)
3 | a.test = 2;
4 | }
5 |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ function Component(props) {
4 | foo(() => {
> 5 | x.a = 10;
| ^ InvalidReact: Writing to a variable defined outside a component or hook is not allowed. Consider using an effect (5:5)

InvalidReact: Writing to a variable defined outside a component or hook is not allowed. Consider using an effect (6:6)
6 | x.a = 20;
7 | });
8 | }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ function Component() {
3 | // Cannot assign to globals
> 4 | someUnknownGlobal = 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) (4:4)

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) (5:5)
5 | moduleLocal = true;
6 | };
7 | foo();
Expand Down
Loading
Loading