diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index b61243f0424ad..7dc9e044bfb7d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -1126,9 +1126,32 @@ export class Environment { ); } + getFallthroughPropertyType( + receiver: Type, + _property: Type, + ): BuiltInType | PolyType | null { + let shapeId = null; + if (receiver.kind === 'Object' || receiver.kind === 'Function') { + shapeId = receiver.shapeId; + } + + if (shapeId !== null) { + const shape = this.#shapes.get(shapeId); + + CompilerError.invariant(shape !== undefined, { + reason: `[HIR] Forget internal error: cannot resolve shape ${shapeId}`, + description: null, + loc: null, + suggestions: null, + }); + return shape.properties.get('*') ?? null; + } + return null; + } + getPropertyType( receiver: Type, - property: string, + property: string | number, ): BuiltInType | PolyType | null { let shapeId = null; if (receiver.kind === 'Object' || receiver.kind === 'Function') { @@ -1146,17 +1169,19 @@ export class Environment { loc: null, suggestions: null, }); - let value = - shape.properties.get(property) ?? shape.properties.get('*') ?? null; - if (value === null && isHookName(property)) { - value = this.#getCustomHookType(); + if (typeof property === 'string') { + return ( + shape.properties.get(property) ?? + shape.properties.get('*') ?? + (isHookName(property) ? this.#getCustomHookType() : null) + ); + } else { + return shape.properties.get('*') ?? null; } - return value; - } else if (isHookName(property)) { + } else if (typeof property === 'string' && isHookName(property)) { return this.#getCustomHookType(); - } else { - return null; } + return null; } getFunctionSignature(type: FunctionType): FunctionSignature | null { diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts index 3fe2e938ce57b..df8196c1d7a0d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts @@ -119,8 +119,8 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [ ], /* * https://tc39.es/ecma262/multipage/indexed-collections.html#sec-array.from - * Array.from(arrayLike, optionalFn, optionalThis) not added because - * the Effect of `arrayLike` is polymorphic i.e. + * Array.from(arrayLike, optionalFn, optionalThis) + * Note that the Effect of `arrayLike` is polymorphic i.e. * - Effect.read if * - it does not have an @iterator property and is array-like * (i.e. has a length property) @@ -128,6 +128,20 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [ * - Effect.mutate if it is a self-mutative iterator (e.g. a generator * function) */ + [ + 'from', + addFunction(DEFAULT_SHAPES, [], { + positionalParams: [ + Effect.ConditionallyMutate, + Effect.ConditionallyMutate, + Effect.ConditionallyMutate, + ], + restParam: Effect.Read, + returnType: {kind: 'Object', shapeId: BuiltInArrayId}, + calleeEffect: Effect.Read, + returnValueKind: ValueKind.Mutable, + }), + ], [ 'of', // Array.of(element0, ..., elementN) diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts index f51768c586b72..22ae261867b3a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts @@ -535,6 +535,30 @@ addObject(BUILTIN_SHAPES, BuiltInRefValueId, [ ['*', {kind: 'Object', shapeId: BuiltInRefValueId}], ]); +/** + * MixedReadOnly = + * | primitive + * | simple objects (Record) + * | Array + * + * APIs such as Relay — but also Flux and other data stores — often return a + * union of types with some interesting properties in terms of analysis. + * + * Given this constraint, if data came from Relay, then we should be able to + * infer things like `data.items.map(): Array`. That may seem like a leap at + * first but remember, we assume you're not patching builtins. Thus the only way + * data.items.map can exist and be a function, given the above set of data types + * and builtin JS methods, is if `data.items` was an Array, and `data.items.map` + * is therefore calling Array.prototype.map. Then we know that function returns + * an Array as well. This relies on the fact that map() is being called, so if + * data.items was some other type it would error at runtime - so it's sound. + * + * Note that this shape is currently only used for hook return values, which + * means that it's safe to type aliasing method-call return kinds as `Frozen`. + * + * Also note that all newly created arrays from method-calls (e.g. `.map`) + * have the appropriate mutable `BuiltInArray` shape + */ addObject(BUILTIN_SHAPES, BuiltInMixedReadonlyId, [ [ 'toString', @@ -546,6 +570,36 @@ addObject(BUILTIN_SHAPES, BuiltInMixedReadonlyId, [ returnValueKind: ValueKind.Primitive, }), ], + [ + 'indexOf', + addFunction(BUILTIN_SHAPES, [], { + positionalParams: [], + restParam: Effect.Read, + returnType: {kind: 'Primitive'}, + calleeEffect: Effect.Read, + returnValueKind: ValueKind.Primitive, + }), + ], + [ + 'includes', + addFunction(BUILTIN_SHAPES, [], { + positionalParams: [], + restParam: Effect.Read, + returnType: {kind: 'Primitive'}, + calleeEffect: Effect.Read, + returnValueKind: ValueKind.Primitive, + }), + ], + [ + 'at', + addFunction(BUILTIN_SHAPES, [], { + positionalParams: [Effect.Read], + restParam: null, + returnType: {kind: 'Object', shapeId: BuiltInMixedReadonlyId}, + calleeEffect: Effect.Capture, + returnValueKind: ValueKind.Frozen, + }), + ], [ 'map', addFunction(BUILTIN_SHAPES, [], { @@ -642,9 +696,9 @@ addObject(BUILTIN_SHAPES, BuiltInMixedReadonlyId, [ addFunction(BUILTIN_SHAPES, [], { positionalParams: [], restParam: Effect.ConditionallyMutate, - returnType: {kind: 'Poly'}, + returnType: {kind: 'Object', shapeId: BuiltInMixedReadonlyId}, calleeEffect: Effect.ConditionallyMutate, - returnValueKind: ValueKind.Mutable, + returnValueKind: ValueKind.Frozen, noAlias: true, mutableOnlyIfOperandsAreMutable: true, }), diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Types.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Types.ts index a67b580a6728b..1de81919c391d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Types.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Types.ts @@ -60,7 +60,15 @@ export type PropType = { kind: 'Property'; objectType: Type; objectName: string; - propertyName: PropertyLiteral; + propertyName: + | { + kind: 'literal'; + value: PropertyLiteral; + } + | { + kind: 'computed'; + value: Type; + }; }; export type ObjectMethod = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts index e6883250705d4..bfa0825408355 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts @@ -872,11 +872,33 @@ function inferBlock( reason: new Set([ValueReason.Other]), context: new Set(), }; + + for (const element of instrValue.elements) { + if (element.kind === 'Spread') { + state.referenceAndRecordEffects( + freezeActions, + element.place, + isArrayType(element.place.identifier) + ? Effect.Capture + : Effect.ConditionallyMutate, + ValueReason.Other, + ); + } else if (element.kind === 'Identifier') { + state.referenceAndRecordEffects( + freezeActions, + element, + Effect.Capture, + ValueReason.Other, + ); + } else { + let _: 'Hole' = element.kind; + } + } + state.initialize(instrValue, valueKind); + state.define(instr.lvalue, instrValue); + instr.lvalue.effect = Effect.Store; continuation = { - kind: 'initialize', - valueKind, - effect: {kind: Effect.Capture, reason: ValueReason.Other}, - lvalueEffect: Effect.Store, + kind: 'funeffects', }; break; } @@ -1241,21 +1263,12 @@ function inferBlock( for (let i = 0; i < instrValue.args.length; i++) { const arg = instrValue.args[i]; const place = arg.kind === 'Identifier' ? arg : arg.place; - if (effects !== null) { - state.referenceAndRecordEffects( - freezeActions, - place, - effects[i], - ValueReason.Other, - ); - } else { - state.referenceAndRecordEffects( - freezeActions, - place, - Effect.ConditionallyMutate, - ValueReason.Other, - ); - } + state.referenceAndRecordEffects( + freezeActions, + place, + getArgumentEffect(effects != null ? effects[i] : null, arg), + ValueReason.Other, + ); hasCaptureArgument ||= place.effect === Effect.Capture; } if (signature !== null) { @@ -1307,7 +1320,10 @@ function inferBlock( signature !== null ? { kind: signature.returnValueKind, - reason: new Set([ValueReason.Other]), + reason: new Set([ + signature.returnValueReason ?? + ValueReason.KnownReturnSignature, + ]), context: new Set(), } : { @@ -1356,25 +1372,16 @@ function inferBlock( for (let i = 0; i < instrValue.args.length; i++) { const arg = instrValue.args[i]; const place = arg.kind === 'Identifier' ? arg : arg.place; - if (effects !== null) { - /* - * If effects are inferred for an argument, we should fail invalid - * mutating effects - */ - state.referenceAndRecordEffects( - freezeActions, - place, - effects[i], - ValueReason.Other, - ); - } else { - state.referenceAndRecordEffects( - freezeActions, - place, - Effect.ConditionallyMutate, - ValueReason.Other, - ); - } + /* + * If effects are inferred for an argument, we should fail invalid + * mutating effects + */ + state.referenceAndRecordEffects( + freezeActions, + place, + getArgumentEffect(effects != null ? effects[i] : null, arg), + ValueReason.Other, + ); hasCaptureArgument ||= place.effect === Effect.Capture; } if (signature !== null) { @@ -2049,3 +2056,31 @@ function areArgumentsImmutableAndNonMutating( } return true; } + +function getArgumentEffect( + signatureEffect: Effect | null, + arg: Place | SpreadPattern, +): Effect { + if (signatureEffect != null) { + if (arg.kind === 'Identifier') { + return signatureEffect; + } else if ( + signatureEffect === Effect.Mutate || + signatureEffect === Effect.ConditionallyMutate + ) { + return signatureEffect; + } else { + // see call-spread-argument-mutable-iterator test fixture + if (signatureEffect === Effect.Freeze) { + CompilerError.throwTodo({ + reason: 'Support spread syntax for hook arguments', + loc: arg.place.loc, + }); + } + // effects[i] is Effect.Capture | Effect.Read | Effect.Store + return Effect.ConditionallyMutate; + } + } else { + return Effect.ConditionallyMutate; + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts b/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts index 3054a83c76138..02e4e60e4b840 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts @@ -307,11 +307,26 @@ function* generateInstructionTypes( kind: 'Property', objectType: value.object.identifier.type, objectName: getName(names, value.object.identifier.id), - propertyName: value.property, + propertyName: { + kind: 'literal', + value: value.property, + }, }); break; } + case 'ComputedLoad': { + yield equation(left, { + kind: 'Property', + objectType: value.object.identifier.type, + objectName: getName(names, value.object.identifier.id), + propertyName: { + kind: 'computed', + value: value.property.identifier.type, + }, + }); + break; + } case 'MethodCall': { const returnType = makeType(); yield equation(value.property.identifier.type, { @@ -336,7 +351,10 @@ function* generateInstructionTypes( kind: 'Property', objectType: value.value.identifier.type, objectName: getName(names, value.value.identifier.id), - propertyName: makePropertyLiteral(propertyName), + propertyName: { + kind: 'literal', + value: makePropertyLiteral(propertyName), + }, }); } else { break; @@ -353,7 +371,10 @@ function* generateInstructionTypes( kind: 'Property', objectType: value.value.identifier.type, objectName: getName(names, value.value.identifier.id), - propertyName: makePropertyLiteral(property.key.name), + propertyName: { + kind: 'literal', + value: makePropertyLiteral(property.key.name), + }, }); } } @@ -410,7 +431,6 @@ function* generateInstructionTypes( case 'RegExpLiteral': case 'MetaProperty': case 'ComputedStore': - case 'ComputedLoad': case 'Await': case 'GetIterator': case 'IteratorNext': @@ -454,12 +474,13 @@ class Unifier { return; } const objectType = this.get(tB.objectType); - let propertyType; - if (typeof tB.propertyName === 'number') { - propertyType = null; - } else { - propertyType = this.env.getPropertyType(objectType, tB.propertyName); - } + const propertyType = + tB.propertyName.kind === 'literal' + ? this.env.getPropertyType(objectType, tB.propertyName.value) + : this.env.getFallthroughPropertyType( + objectType, + tB.propertyName.value, + ); if (propertyType !== null) { this.unify(tA, propertyType); } @@ -677,7 +698,11 @@ class Unifier { const RefLikeNameRE = /^(?:[a-zA-Z$_][a-zA-Z$_0-9]*)Ref$|^ref$/; function isRefLikeName(t: PropType): boolean { - return RefLikeNameRE.test(t.objectName) && t.propertyName === 'current'; + return ( + t.propertyName.kind === 'literal' && + RefLikeNameRE.test(t.objectName) && + t.propertyName.value === 'current' + ); } function tryUnionTypes(ty1: Type, ty2: Type): Type | null { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-from-arg1-captures-arg0.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-from-arg1-captures-arg0.expect.md new file mode 100644 index 0000000000000..8892c8d484bfb --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-from-arg1-captures-arg0.expect.md @@ -0,0 +1,91 @@ + +## Input + +```javascript +import {useIdentity, Stringify} from 'shared-runtime'; + +/** + * TODO: Note that this `Array.from` is inferred to be mutating its first + * argument. This is because React Compiler's typing system does not yet support + * annotating a function with a set of argument match cases + distinct + * definitions (polymorphism). + * + * In this case, we should be able to infer that the `Array.from` call is + * not mutating its 0th argument. + * The 0th argument should be typed as having `effect:Mutate` only when + * (1) it might be a mutable iterable or + * (2) the 1st argument might mutate its callee + */ +function Component({value}) { + const arr = [{value: 'foo'}, {value: 'bar'}, {value}]; + useIdentity(); + const derived = Array.from(arr, (x, idx) => ({...x, id: idx})); + return {derived.at(-1)}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 5}], + sequentialRenders: [{value: 5}, {value: 6}, {value: 6}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { useIdentity, Stringify } from "shared-runtime"; + +/** + * TODO: Note that this `Array.from` is inferred to be mutating its first + * argument. This is because React Compiler's typing system does not yet support + * annotating a function with a set of argument match cases + distinct + * definitions (polymorphism). + * + * In this case, we should be able to infer that the `Array.from` call is + * not mutating its 0th argument. + * The 0th argument should be typed as having `effect:Mutate` only when + * (1) it might be a mutable iterable or + * (2) the 1st argument might mutate its callee + */ +function Component(t0) { + const $ = _c(4); + const { value } = t0; + const arr = [{ value: "foo" }, { value: "bar" }, { value }]; + useIdentity(); + const derived = Array.from(arr, _temp); + let t1; + if ($[0] !== derived) { + t1 = derived.at(-1); + $[0] = derived; + $[1] = t1; + } else { + t1 = $[1]; + } + let t2; + if ($[2] !== t1) { + t2 = {t1}; + $[2] = t1; + $[3] = t2; + } else { + t2 = $[3]; + } + return t2; +} +function _temp(x, idx) { + return { ...x, id: idx }; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: 5 }], + sequentialRenders: [{ value: 5 }, { value: 6 }, { value: 6 }], +}; + +``` + +### Eval output +(kind: ok)
{"children":{"value":5,"id":2}}
+
{"children":{"value":6,"id":2}}
+
{"children":{"value":6,"id":2}}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-from-arg1-captures-arg0.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-from-arg1-captures-arg0.js new file mode 100644 index 0000000000000..f2b364bc60eef --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-from-arg1-captures-arg0.js @@ -0,0 +1,26 @@ +import {useIdentity, Stringify} from 'shared-runtime'; + +/** + * TODO: Note that this `Array.from` is inferred to be mutating its first + * argument. This is because React Compiler's typing system does not yet support + * annotating a function with a set of argument match cases + distinct + * definitions (polymorphism). + * + * In this case, we should be able to infer that the `Array.from` call is + * not mutating its 0th argument. + * The 0th argument should be typed as having `effect:Mutate` only when + * (1) it might be a mutable iterable or + * (2) the 1st argument might mutate its callee + */ +function Component({value}) { + const arr = [{value: 'foo'}, {value: 'bar'}, {value}]; + useIdentity(); + const derived = Array.from(arr, (x, idx) => ({...x, id: idx})); + return {derived.at(-1)}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 5}], + sequentialRenders: [{value: 5}, {value: 6}, {value: 6}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-from-captures-arg0.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-from-captures-arg0.expect.md new file mode 100644 index 0000000000000..66d0b4258462c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-from-captures-arg0.expect.md @@ -0,0 +1,88 @@ + +## Input + +```javascript +import {useIdentity, Stringify} from 'shared-runtime'; + +/** + * TODO: Note that this `Array.from` is inferred to be mutating its first + * argument. This is because React Compiler's typing system does not yet support + * annotating a function with a set of argument match cases + distinct + * definitions (polymorphism) + * + * In this case, we should be able to infer that the `Array.from` call is + * not mutating its 0th argument. + * The 0th argument should be typed as having `effect:Mutate` only when + * (1) it might be a mutable iterable or + * (2) the 1st argument might mutate its callee + */ +function Component({value}) { + const arr = [{value: 'foo'}, {value: 'bar'}, {value}]; + useIdentity(); + const derived = Array.from(arr); + return {derived.at(-1)}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 5}], + sequentialRenders: [{value: 5}, {value: 6}, {value: 6}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { useIdentity, Stringify } from "shared-runtime"; + +/** + * TODO: Note that this `Array.from` is inferred to be mutating its first + * argument. This is because React Compiler's typing system does not yet support + * annotating a function with a set of argument match cases + distinct + * definitions (polymorphism) + * + * In this case, we should be able to infer that the `Array.from` call is + * not mutating its 0th argument. + * The 0th argument should be typed as having `effect:Mutate` only when + * (1) it might be a mutable iterable or + * (2) the 1st argument might mutate its callee + */ +function Component(t0) { + const $ = _c(4); + const { value } = t0; + const arr = [{ value: "foo" }, { value: "bar" }, { value }]; + useIdentity(); + const derived = Array.from(arr); + let t1; + if ($[0] !== derived) { + t1 = derived.at(-1); + $[0] = derived; + $[1] = t1; + } else { + t1 = $[1]; + } + let t2; + if ($[2] !== t1) { + t2 = {t1}; + $[2] = t1; + $[3] = t2; + } else { + t2 = $[3]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: 5 }], + sequentialRenders: [{ value: 5 }, { value: 6 }, { value: 6 }], +}; + +``` + +### Eval output +(kind: ok)
{"children":{"value":5}}
+
{"children":{"value":6}}
+
{"children":{"value":6}}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-from-captures-arg0.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-from-captures-arg0.js new file mode 100644 index 0000000000000..c9b09c384de20 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-from-captures-arg0.js @@ -0,0 +1,26 @@ +import {useIdentity, Stringify} from 'shared-runtime'; + +/** + * TODO: Note that this `Array.from` is inferred to be mutating its first + * argument. This is because React Compiler's typing system does not yet support + * annotating a function with a set of argument match cases + distinct + * definitions (polymorphism) + * + * In this case, we should be able to infer that the `Array.from` call is + * not mutating its 0th argument. + * The 0th argument should be typed as having `effect:Mutate` only when + * (1) it might be a mutable iterable or + * (2) the 1st argument might mutate its callee + */ +function Component({value}) { + const arr = [{value: 'foo'}, {value: 'bar'}, {value}]; + useIdentity(); + const derived = Array.from(arr); + return {derived.at(-1)}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 5}], + sequentialRenders: [{value: 5}, {value: 6}, {value: 6}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-from-maybemutates-arg0.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-from-maybemutates-arg0.expect.md new file mode 100644 index 0000000000000..586124280a3dd --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-from-maybemutates-arg0.expect.md @@ -0,0 +1,64 @@ + +## Input + +```javascript +import {mutateAndReturn, Stringify, useIdentity} from 'shared-runtime'; + +function Component({value}) { + const arr = [{value: 'foo'}, {value: 'bar'}, {value}]; + useIdentity(); + const derived = Array.from(arr, mutateAndReturn); + return {derived.at(-1)}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 5}], + sequentialRenders: [{value: 5}, {value: 6}, {value: 6}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { mutateAndReturn, Stringify, useIdentity } from "shared-runtime"; + +function Component(t0) { + const $ = _c(4); + const { value } = t0; + const arr = [{ value: "foo" }, { value: "bar" }, { value }]; + useIdentity(); + const derived = Array.from(arr, mutateAndReturn); + let t1; + if ($[0] !== derived) { + t1 = derived.at(-1); + $[0] = derived; + $[1] = t1; + } else { + t1 = $[1]; + } + let t2; + if ($[2] !== t1) { + t2 = {t1}; + $[2] = t1; + $[3] = t2; + } else { + t2 = $[3]; + } + return t2; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: 5 }], + sequentialRenders: [{ value: 5 }, { value: 6 }, { value: 6 }], +}; + +``` + +### Eval output +(kind: ok)
{"children":{"value":5,"wat0":"joe"}}
+
{"children":{"value":6,"wat0":"joe"}}
+
{"children":{"value":6,"wat0":"joe"}}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-from-maybemutates-arg0.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-from-maybemutates-arg0.js new file mode 100644 index 0000000000000..edb4e37125843 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-from-maybemutates-arg0.js @@ -0,0 +1,14 @@ +import {mutateAndReturn, Stringify, useIdentity} from 'shared-runtime'; + +function Component({value}) { + const arr = [{value: 'foo'}, {value: 'bar'}, {value}]; + useIdentity(); + const derived = Array.from(arr, mutateAndReturn); + return {derived.at(-1)}; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 5}], + sequentialRenders: [{value: 5}, {value: 6}, {value: 6}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-spread-later-mutated.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-spread-later-mutated.expect.md new file mode 100644 index 0000000000000..a317e22faf92e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-spread-later-mutated.expect.md @@ -0,0 +1,62 @@ + +## Input + +```javascript +function useBar({arg}) { + /** + * Note that mutableIterator is mutated by the later object spread. Therefore, + * `s.values()` should be memoized within the same block as the object spread. + * In terms of compiler internals, they should have the same reactive scope. + */ + const obj = {}; + const s = new Set([obj, 5, 4]); + const mutableIterator = s.values(); + const arr = [...mutableIterator]; + + obj.x = arg; + return arr; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useBar, + params: [{arg: 3}], + sequentialRenders: [{arg: 3}, {arg: 3}, {arg: 4}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function useBar(t0) { + const $ = _c(2); + const { arg } = t0; + let arr; + if ($[0] !== arg) { + const obj = {}; + const s = new Set([obj, 5, 4]); + const mutableIterator = s.values(); + arr = [...mutableIterator]; + + obj.x = arg; + $[0] = arg; + $[1] = arr; + } else { + arr = $[1]; + } + return arr; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useBar, + params: [{ arg: 3 }], + sequentialRenders: [{ arg: 3 }, { arg: 3 }, { arg: 4 }], +}; + +``` + +### Eval output +(kind: ok) [{"x":3},5,4] +[{"x":3},5,4] +[{"x":4},5,4] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-spread-later-mutated.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-spread-later-mutated.js new file mode 100644 index 0000000000000..036ce2ddfc681 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-spread-later-mutated.js @@ -0,0 +1,20 @@ +function useBar({arg}) { + /** + * Note that mutableIterator is mutated by the later object spread. Therefore, + * `s.values()` should be memoized within the same block as the object spread. + * In terms of compiler internals, they should have the same reactive scope. + */ + const obj = {}; + const s = new Set([obj, 5, 4]); + const mutableIterator = s.values(); + const arr = [...mutableIterator]; + + obj.x = arg; + return arr; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useBar, + params: [{arg: 3}], + sequentialRenders: [{arg: 3}, {arg: 3}, {arg: 4}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-array-spread-mutable-iterator.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-spread-mutable-iterator.expect.md similarity index 82% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-array-spread-mutable-iterator.expect.md rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-spread-mutable-iterator.expect.md index 70f41f40477ae..25499af1b0202 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-array-spread-mutable-iterator.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-spread-mutable-iterator.expect.md @@ -55,26 +55,20 @@ import { c as _c } from "react/compiler-runtime"; /** function useBar(t0) { "use memo"; - const $ = _c(3); + const $ = _c(2); const { arg } = t0; let t1; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + if ($[0] !== arg) { const s = new Set([1, 5, 4]); - t1 = s.values(); - $[0] = t1; - } else { - t1 = $[0]; - } - const mutableIterator = t1; - let t2; - if ($[1] !== arg) { - t2 = [arg, ...mutableIterator]; - $[1] = arg; - $[2] = t2; + const mutableIterator = s.values(); + + t1 = [arg, ...mutableIterator]; + $[0] = arg; + $[1] = t1; } else { - t2 = $[2]; + t1 = $[1]; } - return t2; + return t1; } export const FIXTURE_ENTRYPOINT = { @@ -84,4 +78,8 @@ export const FIXTURE_ENTRYPOINT = { }; ``` - \ No newline at end of file + +### Eval output +(kind: ok) [3,1,5,4] +[3,1,5,4] +[4,1,5,4] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-array-spread-mutable-iterator.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-spread-mutable-iterator.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-array-spread-mutable-iterator.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-spread-mutable-iterator.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/call-spread-argument-mutable-iterator.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/call-spread-argument-mutable-iterator.expect.md new file mode 100644 index 0000000000000..74fb57b6bc2fe --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/call-spread-argument-mutable-iterator.expect.md @@ -0,0 +1,42 @@ + +## Input + +```javascript +import {useIdentity} from 'shared-runtime'; + +function useFoo() { + const it = new Set([1, 2]).values(); + useIdentity(); + return Math.max(...it); +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{}], + sequentialRenders: [{}, {}], +}; + +``` + +## Code + +```javascript +import { useIdentity } from "shared-runtime"; + +function useFoo() { + const it = new Set([1, 2]).values(); + useIdentity(); + return Math.max(...it); +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{}], + sequentialRenders: [{}, {}], +}; + +``` + +### Eval output +(kind: ok) 2 +2 \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/call-spread-argument-mutable-iterator.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/call-spread-argument-mutable-iterator.js new file mode 100644 index 0000000000000..1b30f0a46f7c7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/call-spread-argument-mutable-iterator.js @@ -0,0 +1,13 @@ +import {useIdentity} from 'shared-runtime'; + +function useFoo() { + const it = new Set([1, 2]).values(); + useIdentity(); + return Math.max(...it); +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{}], + sequentialRenders: [{}, {}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-hook-call-spreads-mutable-iterator.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-hook-call-spreads-mutable-iterator.expect.md new file mode 100644 index 0000000000000..5d0af122f2146 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-hook-call-spreads-mutable-iterator.expect.md @@ -0,0 +1,33 @@ + +## Input + +```javascript +import {useIdentity} from 'shared-runtime'; + +function Component() { + const items = makeArray(0, 1, 2, null, 4, false, 6); + return useIdentity(...items.values()); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], + sequentialRenders: [{}, {}], +}; + +``` + + +## Error + +``` + 3 | function Component() { + 4 | const items = makeArray(0, 1, 2, null, 4, false, 6); +> 5 | return useIdentity(...items.values()); + | ^^^^^^^^^^^^^^ Todo: Support spread syntax for hook arguments (5:5) + 6 | } + 7 | + 8 | export const FIXTURE_ENTRYPOINT = { +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-hook-call-spreads-mutable-iterator.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-hook-call-spreads-mutable-iterator.js new file mode 100644 index 0000000000000..982fd83dc8a20 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-hook-call-spreads-mutable-iterator.js @@ -0,0 +1,12 @@ +import {useIdentity} from 'shared-runtime'; + +function Component() { + const items = makeArray(0, 1, 2, null, 4, false, 6); + return useIdentity(...items.values()); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [], + sequentialRenders: [{}, {}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-nested-method-calls-lower-property-load-into-temporary.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-nested-method-calls-lower-property-load-into-temporary.expect.md index 877c15e883ffa..56bf9e3d62148 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-nested-method-calls-lower-property-load-into-temporary.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-nested-method-calls-lower-property-load-into-temporary.expect.md @@ -4,9 +4,10 @@ ```javascript import {makeArray} from 'shared-runtime'; -function Component(props) { +const other = [0, 1]; +function Component({}) { const items = makeArray(0, 1, 2, null, 4, false, 6); - const max = Math.max(...items.filter(Boolean)); + const max = Math.max(2, items.push(5), ...other); return max; } @@ -21,13 +22,13 @@ export const FIXTURE_ENTRYPOINT = { ## Error ``` - 3 | function Component(props) { - 4 | const items = makeArray(0, 1, 2, null, 4, false, 6); -> 5 | const max = Math.max(...items.filter(Boolean)); - | ^^^^^^^^ Invariant: [Codegen] Internal error: MethodCall::property must be an unpromoted + unmemoized MemberExpression. Got a `Identifier` (5:5) - 6 | return max; - 7 | } - 8 | + 4 | function Component({}) { + 5 | const items = makeArray(0, 1, 2, null, 4, false, 6); +> 6 | const max = Math.max(2, items.push(5), ...other); + | ^^^^^^^^ Invariant: [Codegen] Internal error: MethodCall::property must be an unpromoted + unmemoized MemberExpression. Got a `Identifier` (6:6) + 7 | return max; + 8 | } + 9 | ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-nested-method-calls-lower-property-load-into-temporary.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-nested-method-calls-lower-property-load-into-temporary.js index 475cf383cf1e4..b2883c3303831 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-nested-method-calls-lower-property-load-into-temporary.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-nested-method-calls-lower-property-load-into-temporary.js @@ -1,8 +1,9 @@ import {makeArray} from 'shared-runtime'; -function Component(props) { +const other = [0, 1]; +function Component({}) { const items = makeArray(0, 1, 2, null, 4, false, 6); - const max = Math.max(...items.filter(Boolean)); + const max = Math.max(2, items.push(5), ...other); return max; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/relay-transitive-mixeddata.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/relay-transitive-mixeddata.expect.md new file mode 100644 index 0000000000000..5caf2e8e0da08 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/relay-transitive-mixeddata.expect.md @@ -0,0 +1,61 @@ + +## Input + +```javascript +import {useFragment} from 'shared-runtime'; + +/** + * React compiler should infer that the returned value is a primitive and avoid + * memoizing it. + */ +function useRelayData({query, idx}) { + 'use memo'; + const data = useFragment('', query); + return data.a[idx].toString(); +} + +export const FIXTURE_ENTRYPOINT = { + fn: useRelayData, + params: [{query: '', idx: 0}], + sequentialRenders: [ + {query: '', idx: 0}, + {query: '', idx: 0}, + {query: '', idx: 1}, + ], +}; + +``` + +## Code + +```javascript +import { useFragment } from "shared-runtime"; + +/** + * React compiler should infer that the returned value is a primitive and avoid + * memoizing it. + */ +function useRelayData(t0) { + "use memo"; + const { query, idx } = t0; + + const data = useFragment("", query); + return data.a[idx].toString(); +} + +export const FIXTURE_ENTRYPOINT = { + fn: useRelayData, + params: [{ query: "", idx: 0 }], + sequentialRenders: [ + { query: "", idx: 0 }, + { query: "", idx: 0 }, + { query: "", idx: 1 }, + ], +}; + +``` + +### Eval output +(kind: ok) "1" +"1" +"2" \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/relay-transitive-mixeddata.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/relay-transitive-mixeddata.js new file mode 100644 index 0000000000000..78708f30c71c6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/relay-transitive-mixeddata.js @@ -0,0 +1,21 @@ +import {useFragment} from 'shared-runtime'; + +/** + * React compiler should infer that the returned value is a primitive and avoid + * memoizing it. + */ +function useRelayData({query, idx}) { + 'use memo'; + const data = useFragment('', query); + return data.a[idx].toString(); +} + +export const FIXTURE_ENTRYPOINT = { + fn: useRelayData, + params: [{query: '', idx: 0}], + sequentialRenders: [ + {query: '', idx: 0}, + {query: '', idx: 0}, + {query: '', idx: 1}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-granular-iterator-semantics.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-granular-iterator-semantics.expect.md index 1ba01dc5bf4df..ea3f1d4f38715 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-granular-iterator-semantics.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-granular-iterator-semantics.expect.md @@ -68,21 +68,29 @@ function Validate({ x, input }) { } function useFoo(input) { "use memo"; - const $ = _c(3); + const $ = _c(5); const x = Array.from([{}]); useIdentity(); - x.push([input]); let t0; - if ($[0] !== input || $[1] !== x) { - t0 = ; + if ($[0] !== input) { + t0 = [input]; $[0] = input; - $[1] = x; - $[2] = t0; + $[1] = t0; + } else { + t0 = $[1]; + } + x.push(t0); + let t1; + if ($[2] !== input || $[3] !== x) { + t1 = ; + $[2] = input; + $[3] = x; + $[4] = t1; } else { - t0 = $[2]; + t1 = $[4]; } - return t0; + return t1; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-type-inference-array-from.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-inference-array-from.expect.md similarity index 71% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-type-inference-array-from.expect.md rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-inference-array-from.expect.md index 6061464afce78..5209fd953ec88 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-type-inference-array-from.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-inference-array-from.expect.md @@ -37,6 +37,12 @@ function useFoo({val1, val2}) { export const FIXTURE_ENTRYPOINT = { fn: useFoo, params: [{val1: 1, val2: 2}], + params: [ + {val1: 1, val2: 2}, + {val1: 1, val2: 2}, + {val1: 1, val2: 3}, + {val1: 4, val2: 2}, + ], }; ``` @@ -71,29 +77,51 @@ function Validate({ x, val1, val2 }) { } function useFoo(t0) { "use memo"; - const $ = _c(4); + const $ = _c(8); const { val1, val2 } = t0; const x = Array.from([]); useIdentity(); - x.push([val1]); - x.push([val2]); let t1; - if ($[0] !== val1 || $[1] !== val2 || $[2] !== x) { - t1 = ; + if ($[0] !== val1) { + t1 = [val1]; $[0] = val1; - $[1] = val2; - $[2] = x; - $[3] = t1; + $[1] = t1; + } else { + t1 = $[1]; + } + x.push(t1); + let t2; + if ($[2] !== val2) { + t2 = [val2]; + $[2] = val2; + $[3] = t2; + } else { + t2 = $[3]; + } + x.push(t2); + let t3; + if ($[4] !== val1 || $[5] !== val2 || $[6] !== x) { + t3 = ; + $[4] = val1; + $[5] = val2; + $[6] = x; + $[7] = t3; } else { - t1 = $[3]; + t3 = $[7]; } - return t1; + return t3; } export const FIXTURE_ENTRYPOINT = { fn: useFoo, params: [{ val1: 1, val2: 2 }], + params: [ + { val1: 1, val2: 2 }, + { val1: 1, val2: 2 }, + { val1: 1, val2: 3 }, + { val1: 4, val2: 2 }, + ], }; ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-type-inference-array-from.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-inference-array-from.js similarity index 87% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-type-inference-array-from.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-inference-array-from.js index d1a80a4ea7090..dfd4e0e0f19af 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo-type-inference-array-from.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/type-inference-array-from.js @@ -33,4 +33,10 @@ function useFoo({val1, val2}) { export const FIXTURE_ENTRYPOINT = { fn: useFoo, params: [{val1: 1, val2: 2}], + params: [ + {val1: 1, val2: 2}, + {val1: 1, val2: 2}, + {val1: 1, val2: 3}, + {val1: 4, val2: 2}, + ], }; diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index bbed77c35a90d..0817bbf89c582 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -462,7 +462,6 @@ const skipFilter = new Set([ // bugs 'bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr', - 'bug-array-spread-mutable-iterator', `bug-capturing-func-maybealias-captured-mutate`, 'bug-aliased-capture-aliased-mutate', 'bug-aliased-capture-mutate',