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 @@ -458,9 +458,9 @@ export function dropManualMemoization(
reason: 'useMemo() callbacks must return a value',
description: `This ${
manualMemo.loadInstr.value.kind === 'PropertyLoad'
? 'React.useMemo'
: 'useMemo'
} callback doesn't return a value. useMemo is for computing and caching values, not for arbitrary side effects`,
? 'React.useMemo()'
: 'useMemo()'
} callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects`,
suggestions: null,
}).withDetails({
kind: 'error',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,36 @@ import {
CompilerError,
ErrorCategory,
} from '../CompilerError';
import {FunctionExpression, HIRFunction, IdentifierId} from '../HIR';
import {
FunctionExpression,
HIRFunction,
IdentifierId,
SourceLocation,
} from '../HIR';
import {
eachInstructionValueOperand,
eachTerminalOperand,
} from '../HIR/visitors';
import {Result} from '../Utils/Result';

export function validateUseMemo(fn: HIRFunction): Result<void, CompilerError> {
const errors = new CompilerError();
const useMemos = new Set<IdentifierId>();
const react = new Set<IdentifierId>();
const functions = new Map<IdentifierId, FunctionExpression>();
const unusedUseMemos = new Map<IdentifierId, SourceLocation>();
for (const [, block] of fn.body.blocks) {
for (const {lvalue, value} of block.instructions) {
if (unusedUseMemos.size !== 0) {
/**
* Most of the time useMemo results are referenced immediately. Don't bother
* scanning instruction operands for useMemos unless there is an as-yet-unused
* useMemo.
*/
for (const operand of eachInstructionValueOperand(value)) {
unusedUseMemos.delete(operand.identifier.id);
}
}
switch (value.kind) {
case 'LoadGlobal': {
if (value.binding.name === 'useMemo') {
Expand All @@ -45,10 +65,8 @@ export function validateUseMemo(fn: HIRFunction): Result<void, CompilerError> {
case 'CallExpression': {
// Is the function being called useMemo, with at least 1 argument?
const callee =
value.kind === 'CallExpression'
? value.callee.identifier.id
: value.property.identifier.id;
const isUseMemo = useMemos.has(callee);
value.kind === 'CallExpression' ? value.callee : value.property;
const isUseMemo = useMemos.has(callee.identifier.id);
if (!isUseMemo || value.args.length === 0) {
continue;
}
Expand Down Expand Up @@ -104,10 +122,73 @@ export function validateUseMemo(fn: HIRFunction): Result<void, CompilerError> {
);
}

validateNoContextVariableAssignment(body.loweredFunc.func, errors);

if (fn.env.config.validateNoVoidUseMemo) {
unusedUseMemos.set(lvalue.identifier.id, callee.loc);
}
break;
}
}
}
if (unusedUseMemos.size !== 0) {
for (const operand of eachTerminalOperand(block.terminal)) {
unusedUseMemos.delete(operand.identifier.id);
}
}
}
if (unusedUseMemos.size !== 0) {
/**
* Basic check for unused memos, where the result of the call is never referenced. This runs
* before DCE so it's more of an AST-level check that something, _anything_, cares about the value.
*
* This is easy to defeat with e.g. `const _ = useMemo(...)` but it at least gives us something to teach.
* Even a DCE-based version could be bypassed with `noop(useMemo(...))`.
*/
for (const loc of unusedUseMemos.values()) {
errors.pushDiagnostic(
CompilerDiagnostic.create({
category: ErrorCategory.VoidUseMemo,
reason: 'Unused useMemo()',
description: `This useMemo() value is unused. useMemo() is for computing and caching values, not for arbitrary side effects`,
suggestions: null,
}).withDetails({
kind: 'error',
loc,
message: 'useMemo() result is unused',
}),
);
}
}
return errors.asResult();
}

function validateNoContextVariableAssignment(
fn: HIRFunction,
errors: CompilerError,
): void {
for (const block of fn.body.blocks.values()) {
for (const instr of block.instructions) {
const value = instr.value;
switch (value.kind) {
case 'StoreContext': {
errors.pushDiagnostic(
CompilerDiagnostic.create({
category: ErrorCategory.UseMemo,
reason:
'useMemo() callbacks may not reassign variables declared outside of the callback',
description:
'useMemo() callbacks must be pure functions and cannot reassign variables defined outside of the callback function',
suggestions: null,
}).withDetails({
kind: 'error',
loc: value.lvalue.place.loc,
message: 'Cannot reassign variable',
}),
);
break;
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@

## Input

```javascript
function Component() {
let x;
const y = useMemo(() => {
let z;
x = [];
z = true;
return z;
}, []);
return [x, y];
}

```


## Error

```
Found 1 error:

Error: useMemo() callbacks may not reassign variables declared outside of the callback

useMemo() callbacks must be pure functions and cannot reassign variables defined outside of the callback function.

error.invalid-reassign-variable-in-usememo.ts:5:4
3 | const y = useMemo(() => {
4 | let z;
> 5 | x = [];
| ^ Cannot reassign variable
6 | z = true;
7 | return z;
8 | }, []);
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
function Component() {
let x;
const y = useMemo(() => {
let z;
x = [];
z = true;
return z;
}, []);
return [x, y];
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@

## Input

```javascript
// @validateNoVoidUseMemo
function Component() {
useMemo(() => {
return [];
}, []);
return <div />;
}

```


## Error

```
Found 1 error:

Error: Unused useMemo()

This useMemo() value is unused. useMemo() is for computing and caching values, not for arbitrary side effects.

error.invalid-unused-usememo.ts:3:2
1 | // @validateNoVoidUseMemo
2 | function Component() {
> 3 | useMemo(() => {
| ^^^^^^^ useMemo() result is unused
4 | return [];
5 | }, []);
6 | return <div />;
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// @validateNoVoidUseMemo
function Component() {
useMemo(() => {
return [];
}, []);
return <div />;
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ Found 2 errors:

Error: useMemo() callbacks must return a value

This useMemo callback doesn't return a value. useMemo is for computing and caching values, not for arbitrary side effects.
This useMemo() callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects.

error.useMemo-no-return-value.ts:3:16
1 | // @validateNoVoidUseMemo
Expand All @@ -45,7 +45,7 @@ error.useMemo-no-return-value.ts:3:16

Error: useMemo() callbacks must return a value

This React.useMemo callback doesn't return a value. useMemo is for computing and caching values, not for arbitrary side effects.
This React.useMemo() callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects.

error.useMemo-no-return-value.ts:6:17
4 | console.log('computing');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ testRule('plugin-recommended', TestRecommendedRules, {

return <Child x={state} />;
}`,
errors: [makeTestCaseError('useMemo() callbacks must return a value')],
errors: [makeTestCaseError('Unused useMemo()')],
},
{
name: 'Pipeline errors are reported',
Expand Down
Loading