Skip to content

[pull] main from facebook:main #198

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

Merged
merged 10 commits into from
Jul 28, 2025
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 19.1.1 (July 28, 2025)

### React
* Fixed Owner Stacks to work with ES2015 function.name semantics ([#33680](https://github.com/facebook/react/pull/33680) by @hoxyq)

## 19.1.0 (March 28, 2025)

### Owner Stack
Expand Down
1 change: 1 addition & 0 deletions compiler/apps/playground/components/Editor/EditorImpl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ function compile(
validateNoImpureFunctionsInRender: true,
validateStaticComponents: true,
validateNoFreezingKnownMutableFunctions: true,
validateNoVoidUseMemo: true,
}
: {
/* use defaults for compiler mode */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ function runWithEnvironment(
!env.config.disableMemoizationForDebugging &&
!env.config.enableChangeDetectionForDebugging
) {
dropManualMemoization(hir);
dropManualMemoization(hir).unwrap();
log({kind: 'hir', name: 'DropManualMemoization', value: hir});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ export function lower(
const fallthrough = builder.reserve('block');
const terminal: ReturnTerminal = {
kind: 'return',
returnVariant: 'Implicit',
loc: GeneratedSource,
value: lowerExpressionToTemporary(builder, body),
id: makeInstructionId(0),
Expand Down Expand Up @@ -219,6 +220,7 @@ export function lower(
builder.terminate(
{
kind: 'return',
returnVariant: 'Void',
loc: GeneratedSource,
value: lowerValueToTemporary(builder, {
kind: 'Primitive',
Expand Down Expand Up @@ -302,6 +304,7 @@ function lowerStatement(
}
const terminal: ReturnTerminal = {
kind: 'return',
returnVariant: 'Explicit',
loc: stmt.node.loc ?? GeneratedSource,
value,
id: makeInstructionId(0),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,17 @@ export const EnvironmentConfigSchema = z.object({
* ```
*/
lowerContextAccess: ExternalFunctionSchema.nullable().default(null),

/**
* If enabled, will validate useMemos that don't return any values:
*
* Valid:
* useMemo(() => foo, [foo]);
* useMemo(() => { return foo }, [foo]);
* Invalid:
* useMemo(() => { ... }, [...]);
*/
validateNoVoidUseMemo: z.boolean().default(false),
});

export type EnvironmentConfig = z.infer<typeof EnvironmentConfigSchema>;
Expand Down
12 changes: 12 additions & 0 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -446,8 +446,20 @@ export type ThrowTerminal = {
};
export type Case = {test: Place | null; block: BlockId};

export type ReturnVariant = 'Void' | 'Implicit' | 'Explicit';
export type ReturnTerminal = {
kind: 'return';
/**
* Void:
* () => { ... }
* function() { ... }
* Implicit (ArrowFunctionExpression only):
* () => foo
* Explicit:
* () => { return ... }
* function () { return ... }
*/
returnVariant: ReturnVariant;
loc: SourceLocation;
value: Place;
id: InstructionId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ export function printTerminal(terminal: Terminal): Array<string> | string {
break;
}
case 'return': {
value = `[${terminal.id}] Return${
value = `[${terminal.id}] Return ${terminal.returnVariant}${
terminal.value != null ? ' ' + printPlace(terminal.value) : ''
}`;
if (terminal.effects != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,7 @@ export function mapTerminalSuccessors(
case 'return': {
return {
kind: 'return',
returnVariant: terminal.returnVariant,
loc: terminal.loc,
value: terminal.value,
id: makeInstructionId(0),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@
* LICENSE file in the root directory of this source tree.
*/

import {CompilerError, SourceLocation} from '..';
import {
CompilerDiagnostic,
CompilerError,
ErrorSeverity,
SourceLocation,
} from '..';
import {
CallExpression,
Effect,
Expand All @@ -30,6 +35,7 @@ import {
makeInstructionId,
} from '../HIR';
import {createTemporaryPlace, markInstructionIds} from '../HIR/HIRBuilder';
import {Result} from '../Utils/Result';

type ManualMemoCallee = {
kind: 'useMemo' | 'useCallback';
Expand Down Expand Up @@ -283,50 +289,84 @@ function extractManualMemoizationArgs(
instr: TInstruction<CallExpression> | TInstruction<MethodCall>,
kind: 'useCallback' | 'useMemo',
sidemap: IdentifierSidemap,
errors: CompilerError,
): {
fnPlace: Place;
fnPlace: Place | null;
depsList: Array<ManualMemoDependency> | null;
} {
const [fnPlace, depsListPlace] = instr.value.args as Array<
Place | SpreadPattern | undefined
>;
if (fnPlace == null) {
CompilerError.throwInvalidReact({
reason: `Expected a callback function to be passed to ${kind}`,
loc: instr.value.loc,
suggestions: null,
});
errors.pushDiagnostic(
CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidReact,
category: `Expected a callback function to be passed to ${kind}`,
description: `Expected a callback function to be passed to ${kind}`,
suggestions: null,
}).withDetail({
kind: 'error',
loc: instr.value.loc,
message: `Expected a callback function to be passed to ${kind}`,
}),
);
return {fnPlace: null, depsList: null};
}
if (fnPlace.kind === 'Spread' || depsListPlace?.kind === 'Spread') {
CompilerError.throwInvalidReact({
reason: `Unexpected spread argument to ${kind}`,
loc: instr.value.loc,
suggestions: null,
});
errors.pushDiagnostic(
CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidReact,
category: `Unexpected spread argument to ${kind}`,
description: `Unexpected spread argument to ${kind}`,
suggestions: null,
}).withDetail({
kind: 'error',
loc: instr.value.loc,
message: `Unexpected spread argument to ${kind}`,
}),
);
return {fnPlace: null, depsList: null};
}
let depsList: Array<ManualMemoDependency> | null = null;
if (depsListPlace != null) {
const maybeDepsList = sidemap.maybeDepsLists.get(
depsListPlace.identifier.id,
);
if (maybeDepsList == null) {
CompilerError.throwInvalidReact({
reason: `Expected the dependency list for ${kind} to be an array literal`,
suggestions: null,
loc: depsListPlace.loc,
});
errors.pushDiagnostic(
CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidReact,
category: `Expected the dependency list for ${kind} to be an array literal`,
description: `Expected the dependency list for ${kind} to be an array literal`,
suggestions: null,
}).withDetail({
kind: 'error',
loc: depsListPlace.loc,
message: `Expected the dependency list for ${kind} to be an array literal`,
}),
);
return {fnPlace, depsList: null};
}
depsList = maybeDepsList.map(dep => {
depsList = [];
for (const dep of maybeDepsList) {
const maybeDep = sidemap.maybeDeps.get(dep.identifier.id);
if (maybeDep == null) {
CompilerError.throwInvalidReact({
reason: `Expected the dependency list to be an array of simple expressions (e.g. \`x\`, \`x.y.z\`, \`x?.y?.z\`)`,
suggestions: null,
loc: dep.loc,
});
errors.pushDiagnostic(
CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidReact,
category: `Expected the dependency list to be an array of simple expressions (e.g. \`x\`, \`x.y.z\`, \`x?.y?.z\`)`,
description: `Expected the dependency list to be an array of simple expressions (e.g. \`x\`, \`x.y.z\`, \`x?.y?.z\`)`,
suggestions: null,
}).withDetail({
kind: 'error',
loc: dep.loc,
message: `Expected the dependency list to be an array of simple expressions (e.g. \`x\`, \`x.y.z\`, \`x?.y?.z\`)`,
}),
);
} else {
depsList.push(maybeDep);
}
return maybeDep;
});
}
}
return {
fnPlace,
Expand All @@ -341,8 +381,14 @@ function extractManualMemoizationArgs(
* rely on type inference to find useMemo/useCallback invocations, and instead does basic tracking
* of globals and property loads to find both direct calls as well as usage via the React namespace,
* eg `React.useMemo()`.
*
* This pass also validates that useMemo callbacks return a value (not void), ensuring that useMemo
* is only used for memoizing values and not for running arbitrary side effects.
*/
export function dropManualMemoization(func: HIRFunction): void {
export function dropManualMemoization(
func: HIRFunction,
): Result<void, CompilerError> {
const errors = new CompilerError();
const isValidationEnabled =
func.env.config.validatePreserveExistingMemoizationGuarantees ||
func.env.config.validateNoSetStateInRender ||
Expand Down Expand Up @@ -389,7 +435,47 @@ export function dropManualMemoization(func: HIRFunction): void {
instr as TInstruction<CallExpression> | TInstruction<MethodCall>,
manualMemo.kind,
sidemap,
errors,
);

if (fnPlace == null) {
continue;
}

/**
* Bailout on void return useMemos. This is an anti-pattern where code might be using
* useMemo like useEffect: running arbirtary side-effects synced to changes in specific
* values.
*/
if (
func.env.config.validateNoVoidUseMemo &&
manualMemo.kind === 'useMemo'
) {
const funcToCheck = sidemap.functions.get(
fnPlace.identifier.id,
)?.value;
if (funcToCheck !== undefined && funcToCheck.loweredFunc.func) {
if (!hasNonVoidReturn(funcToCheck.loweredFunc.func)) {
errors.pushDiagnostic(
CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidReact,
category: '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.`,
suggestions: null,
}).withDetail({
kind: 'error',
loc: instr.value.loc,
message: 'useMemo() callbacks must return a value',
}),
);
}
}
}

instr.value = getManualMemoizationReplacement(
fnPlace,
instr.value.loc,
Expand All @@ -410,11 +496,19 @@ export function dropManualMemoization(func: HIRFunction): void {
* is rare and likely sketchy.
*/
if (!sidemap.functions.has(fnPlace.identifier.id)) {
CompilerError.throwInvalidReact({
reason: `Expected the first argument to be an inline function expression`,
suggestions: [],
loc: fnPlace.loc,
});
errors.pushDiagnostic(
CompilerDiagnostic.create({
severity: ErrorSeverity.InvalidReact,
category: `Expected the first argument to be an inline function expression`,
description: `Expected the first argument to be an inline function expression`,
suggestions: [],
}).withDetail({
kind: 'error',
loc: fnPlace.loc,
message: `Expected the first argument to be an inline function expression`,
}),
);
continue;
}
const memoDecl: Place =
manualMemo.kind === 'useMemo'
Expand Down Expand Up @@ -486,6 +580,8 @@ export function dropManualMemoization(func: HIRFunction): void {
markInstructionIds(func.body);
}
}

return errors.asResult();
}

function findOptionalPlaces(fn: HIRFunction): Set<IdentifierId> {
Expand Down Expand Up @@ -530,3 +626,17 @@ function findOptionalPlaces(fn: HIRFunction): Set<IdentifierId> {
}
return optionals;
}

function hasNonVoidReturn(func: HIRFunction): boolean {
for (const [, block] of func.body.blocks) {
if (block.terminal.kind === 'return') {
if (
block.terminal.returnVariant === 'Explicit' ||
block.terminal.returnVariant === 'Implicit'
) {
return true;
}
}
}
return false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ function emitSelectorFn(env: Environment, keys: Array<string>): Instruction {
terminal: {
id: makeInstructionId(0),
kind: 'return',
returnVariant: 'Explicit',
loc: GeneratedSource,
value: arrayInstr.lvalue,
effects: null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,7 @@ function emitOutlinedFn(
terminal: {
id: makeInstructionId(0),
kind: 'return',
returnVariant: 'Explicit',
loc: GeneratedSource,
value: instructions.at(-1)!.lvalue,
effects: null,
Expand Down
Loading
Loading