Skip to content

[pull] main from facebook:main #134

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 12 commits into from
May 3, 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
10 changes: 8 additions & 2 deletions .github/workflows/runtime_commit_artifacts.yml
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,9 @@ jobs:
mkdir ./compiled/facebook-www/__test_utils__
mv build/__test_utils__/ReactAllWarnings.js ./compiled/facebook-www/__test_utils__/ReactAllWarnings.js

# Move eslint-plugin-react-hooks into eslint-plugin-react-hooks
# Copy eslint-plugin-react-hooks
mkdir ./compiled/eslint-plugin-react-hooks
mv build/oss-experimental/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js \
cp build/oss-experimental/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js \
./compiled/eslint-plugin-react-hooks/index.js

# Move unstable_server-external-runtime.js into facebook-www
Expand Down Expand Up @@ -167,6 +167,12 @@ jobs:
rm $RENDERER_FOLDER/ReactFabric-{dev,prod,profiling}.js
rm $RENDERER_FOLDER/ReactNativeRenderer-{dev,prod,profiling}.js

# Copy eslint-plugin-react-hooks
# NOTE: This is different from www, here we include the full package
# including package.json to include dependencies in fbsource.
mkdir "$BASE_FOLDER/tools"
cp -r build/oss-experimental/eslint-plugin-react-hooks "$BASE_FOLDER/tools"

# Move React Native version file
mv build/facebook-react-native/VERSION_NATIVE_FB ./compiled-rn/VERSION_NATIVE_FB

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ import {transformFire} from '../Transform';
import {validateNoImpureFunctionsInRender} from '../Validation/ValidateNoImpureFunctionsInRender';
import {CompilerError} from '..';
import {validateStaticComponents} from '../Validation/ValidateStaticComponents';
import {validateNoFreezingKnownMutableFunctions} from '../Validation/ValidateNoFreezingKnownMutableFunctions';

export type CompilerPipelineValue =
| {kind: 'ast'; name: string; value: CodegenFunction}
Expand Down Expand Up @@ -274,6 +275,10 @@ function runWithEnvironment(
if (env.config.validateNoImpureFunctionsInRender) {
validateNoImpureFunctionsInRender(hir).unwrap();
}

if (env.config.validateNoFreezingKnownMutableFunctions) {
validateNoFreezingKnownMutableFunctions(hir).unwrap();
}
}

inferReactivePlaces(hir);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,11 @@ const EnvironmentConfigSchema = z.object({
*/
validateNoImpureFunctionsInRender: z.boolean().default(false),

/**
* Validate against passing mutable functions to hooks
*/
validateNoFreezingKnownMutableFunctions: z.boolean().default(false),

/*
* When enabled, the compiler assumes that hooks follow the Rules of React:
* - Hooks may memoize computation based on any of their parameters, thus
Expand Down
37 changes: 36 additions & 1 deletion compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ import {
BuiltInUseRefId,
BuiltInUseStateId,
BuiltInUseTransitionId,
BuiltInWeakMapId,
BuiltInWeakSetId,
ReanimatedSharedValueId,
ShapeRegistry,
addFunction,
addHook,
Expand Down Expand Up @@ -491,6 +494,38 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [
true,
),
],
[
'WeakMap',
addFunction(
DEFAULT_SHAPES,
[],
{
positionalParams: [Effect.ConditionallyMutateIterator],
restParam: null,
returnType: {kind: 'Object', shapeId: BuiltInWeakMapId},
calleeEffect: Effect.Read,
returnValueKind: ValueKind.Mutable,
},
null,
true,
),
],
[
'WeakSet',
addFunction(
DEFAULT_SHAPES,
[],
{
positionalParams: [Effect.ConditionallyMutateIterator],
restParam: null,
returnType: {kind: 'Object', shapeId: BuiltInWeakSetId},
calleeEffect: Effect.Read,
returnValueKind: ValueKind.Mutable,
},
null,
true,
),
],
// TODO: rest of Global objects
];

Expand Down Expand Up @@ -908,7 +943,7 @@ export function getReanimatedModuleType(registry: ShapeRegistry): ObjectType {
addHook(registry, {
positionalParams: [],
restParam: Effect.Freeze,
returnType: {kind: 'Poly'},
returnType: {kind: 'Object', shapeId: ReanimatedSharedValueId},
returnValueKind: ValueKind.Mutable,
noAlias: true,
calleeEffect: Effect.Read,
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 @@ -1725,6 +1725,18 @@ export function isRefOrRefValue(id: Identifier): boolean {
return isUseRefType(id) || isRefValueType(id);
}

/*
* Returns true if the type is a Ref or a custom user type that acts like a ref when it
* shouldn't. For now the only other case of this is Reanimated's shared values.
*/
export function isRefOrRefLikeMutableType(type: Type): boolean {
return (
type.kind === 'Object' &&
(type.shapeId === 'BuiltInUseRefId' ||
type.shapeId == 'ReanimatedSharedValueId')
);
}

export function isSetStateType(id: Identifier): boolean {
return id.type.kind === 'Function' && id.type.shapeId === 'BuiltInSetState';
}
Expand Down
100 changes: 100 additions & 0 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,8 @@ export const BuiltInPropsId = 'BuiltInProps';
export const BuiltInArrayId = 'BuiltInArray';
export const BuiltInSetId = 'BuiltInSet';
export const BuiltInMapId = 'BuiltInMap';
export const BuiltInWeakSetId = 'BuiltInWeakSet';
export const BuiltInWeakMapId = 'BuiltInWeakMap';
export const BuiltInFunctionId = 'BuiltInFunction';
export const BuiltInJsxId = 'BuiltInJsx';
export const BuiltInObjectId = 'BuiltInObject';
Expand All @@ -225,6 +227,9 @@ export const BuiltInStartTransitionId = 'BuiltInStartTransition';
export const BuiltInFireId = 'BuiltInFire';
export const BuiltInFireFunctionId = 'BuiltInFireFunction';

// See getReanimatedModuleType() in Globals.ts — this is part of supporting Reanimated's ref-like types
export const ReanimatedSharedValueId = 'ReanimatedSharedValueId';

// ShapeRegistry with default definitions for built-ins.
export const BUILTIN_SHAPES: ShapeRegistry = new Map();

Expand Down Expand Up @@ -764,6 +769,101 @@ addObject(BUILTIN_SHAPES, BuiltInMapId, [
],
]);

addObject(BUILTIN_SHAPES, BuiltInWeakSetId, [
[
/**
* add(value)
* Parameters
* value: the value of the element to add to the Set object.
* Returns the Set object with added value.
*/
'add',
addFunction(BUILTIN_SHAPES, [], {
positionalParams: [Effect.Capture],
restParam: null,
returnType: {kind: 'Object', shapeId: BuiltInWeakSetId},
calleeEffect: Effect.Store,
// returnValueKind is technically dependent on the ValueKind of the set itself
returnValueKind: ValueKind.Mutable,
}),
],
[
/**
* setInstance.delete(value)
* Returns true if value was already in Set; otherwise false.
*/
'delete',
addFunction(BUILTIN_SHAPES, [], {
positionalParams: [Effect.Read],
restParam: null,
returnType: PRIMITIVE_TYPE,
calleeEffect: Effect.Store,
returnValueKind: ValueKind.Primitive,
}),
],
[
'has',
addFunction(BUILTIN_SHAPES, [], {
positionalParams: [Effect.Read],
restParam: null,
returnType: PRIMITIVE_TYPE,
calleeEffect: Effect.Read,
returnValueKind: ValueKind.Primitive,
}),
],
]);

addObject(BUILTIN_SHAPES, BuiltInWeakMapId, [
[
'delete',
addFunction(BUILTIN_SHAPES, [], {
positionalParams: [Effect.Read],
restParam: null,
returnType: PRIMITIVE_TYPE,
calleeEffect: Effect.Store,
returnValueKind: ValueKind.Primitive,
}),
],
[
'get',
addFunction(BUILTIN_SHAPES, [], {
positionalParams: [Effect.Read],
restParam: null,
returnType: {kind: 'Poly'},
calleeEffect: Effect.Capture,
returnValueKind: ValueKind.Mutable,
}),
],
[
'has',
addFunction(BUILTIN_SHAPES, [], {
positionalParams: [Effect.Read],
restParam: null,
returnType: PRIMITIVE_TYPE,
calleeEffect: Effect.Read,
returnValueKind: ValueKind.Primitive,
}),
],
[
/**
* Params
* key: the key of the element to add to the Map object. The key may be
* any JavaScript type (any primitive value or any type of JavaScript
* object).
* value: the value of the element to add to the Map object.
* Returns the Map object.
*/
'set',
addFunction(BUILTIN_SHAPES, [], {
positionalParams: [Effect.Capture, Effect.Capture],
restParam: null,
returnType: {kind: 'Object', shapeId: BuiltInWeakMapId},
calleeEffect: Effect.Store,
returnValueKind: ValueKind.Mutable,
}),
],
]);

addObject(BUILTIN_SHAPES, BuiltInUseStateId, [
['0', {kind: 'Poly'}],
[
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import {
Effect,
HIRFunction,
Identifier,
isMutableEffect,
isRefOrRefLikeMutableType,
makeInstructionId,
} from '../HIR/HIR';
import {eachInstructionValueOperand} from '../HIR/visitors';
import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables';
import DisjointSet from '../Utils/DisjointSet';

/**
* If a function captures a mutable value but never gets called, we don't infer a
* mutable range for that function. This means that we also don't alias the function
* with its mutable captures.
*
* This case is tricky, because we don't generally know for sure what is a mutation
* and what may just be a normal function call. For example:
*
* ```
* hook useFoo() {
* const x = makeObject();
* return () => {
* return readObject(x); // could be a mutation!
* }
* }
* ```
*
* If we pessimistically assume that all such cases are mutations, we'd have to group
* lots of memo scopes together unnecessarily. However, if there is definitely a mutation:
*
* ```
* hook useFoo(createEntryForKey) {
* const cache = new WeakMap();
* return (key) => {
* let entry = cache.get(key);
* if (entry == null) {
* entry = createEntryForKey(key);
* cache.set(key, entry); // known mutation!
* }
* return entry;
* }
* }
* ```
*
* Then we have to ensure that the function and its mutable captures alias together and
* end up in the same scope. However, aliasing together isn't enough if the function
* and operands all have empty mutable ranges (end = start + 1).
*
* This pass finds function expressions and object methods that have an empty mutable range
* and known-mutable operands which also don't have a mutable range, and ensures that the
* function and those operands are aliased together *and* that their ranges are updated to
* end after the function expression. This is sufficient to ensure that a reactive scope is
* created for the alias set.
*/
export function inferAliasForUncalledFunctions(
fn: HIRFunction,
aliases: DisjointSet<Identifier>,
): void {
for (const block of fn.body.blocks.values()) {
instrs: for (const instr of block.instructions) {
const {lvalue, value} = instr;
if (
value.kind !== 'ObjectMethod' &&
value.kind !== 'FunctionExpression'
) {
continue;
}
/*
* If the function is known to be mutated, we will have
* already aliased any mutable operands with it
*/
const range = lvalue.identifier.mutableRange;
if (range.end > range.start + 1) {
continue;
}
/*
* If the function already has operands with an active mutable range,
* then we don't need to do anything — the function will have already
* been visited and included in some mutable alias set. This case can
* also occur due to visiting the same function in an earlier iteration
* of the outer fixpoint loop.
*/
for (const operand of eachInstructionValueOperand(value)) {
if (isMutable(instr, operand)) {
continue instrs;
}
}
const operands: Set<Identifier> = new Set();
for (const effect of value.loweredFunc.func.effects ?? []) {
if (effect.kind !== 'ContextMutation') {
continue;
}
/*
* We're looking for known-mutations only, so we look at the effects
* rather than function context
*/
if (effect.effect === Effect.Store || effect.effect === Effect.Mutate) {
for (const operand of effect.places) {
/*
* It's possible that function effect analysis thinks there was a context mutation,
* but then InferReferenceEffects figures out some operands are globals and therefore
* creates a non-mutable effect for those operands.
* We should change InferReferenceEffects to swap the ContextMutation for a global
* mutation in that case, but for now we just filter them out here
*/
if (
isMutableEffect(operand.effect, operand.loc) &&
!isRefOrRefLikeMutableType(operand.identifier.type)
) {
operands.add(operand.identifier);
}
}
}
}
if (operands.size !== 0) {
operands.add(lvalue.identifier);
aliases.union([...operands]);
// Update mutable ranges, if the ranges are empty then a reactive scope isn't created
for (const operand of operands) {
operand.mutableRange.end = makeInstructionId(instr.id + 1);
}
}
}
}
}
Loading
Loading