diff --git a/.eslintrc.js b/.eslintrc.js index 4e43b19e7b61d..ed134392a9727 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -496,6 +496,7 @@ module.exports = { __IS_CHROME__: 'readonly', __IS_FIREFOX__: 'readonly', __IS_EDGE__: 'readonly', + __IS_NATIVE__: 'readonly', __IS_INTERNAL_VERSION__: 'readonly', }, }, diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index 65ee683e45e4f..95804df358d3e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -128,11 +128,11 @@ export function* run( code, useMemoCacheIdentifier, ); - yield { + yield log({ kind: 'debug', name: 'EnvironmentConfig', value: prettyFormat(env.config), - }; + }); const ast = yield* runWithEnvironment(func, env); return ast; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts index aae7b82594c58..2d9e21af1d61b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts @@ -335,6 +335,7 @@ function extractManualMemoizationArgs( export function dropManualMemoization(func: HIRFunction): void { const isValidationEnabled = func.env.config.validatePreserveExistingMemoizationGuarantees || + func.env.config.validateNoSetStateInRender || func.env.config.enablePreserveExistingMemoizationGuarantees; const sidemap: IdentifierSidemap = { functions: new Map(), diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts index a3152034fe1d8..0f23ef81d547a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Optimization/LowerContextAccess.ts @@ -21,17 +21,16 @@ import { PropertyLoad, isUseContextHookType, makeBlockId, - makeIdentifierId, - makeIdentifierName, makeInstructionId, makeTemporary, - makeType, markInstructionIds, mergeConsecutiveBlocks, + promoteTemporary, removeUnnecessaryTryCatch, reversePostorderBlocks, } from '../HIR'; import { + createTemporaryPlace, removeDeadDoWhileStatements, removeUnreachableForUpdates, } from '../HIR/HIRBuilder'; @@ -66,7 +65,7 @@ export function lowerContextAccess(fn: HIRFunction): void { const keys = getContextKeys(value); if (keys === null) { - continue; + return; } if (contextKeys.has(destructureId)) { @@ -83,8 +82,10 @@ export function lowerContextAccess(fn: HIRFunction): void { if (contextAccess.size > 0) { for (const [, block] of fn.body.blocks) { - const nextInstructions: Array = []; - for (const instr of block.instructions) { + let nextInstructions: Array | null = null; + + for (let i = 0; i < block.instructions.length; i++) { + const instr = block.instructions[i]; const {lvalue, value} = instr; if ( value.kind === 'CallExpression' && @@ -93,15 +94,22 @@ export function lowerContextAccess(fn: HIRFunction): void { ) { const keys = contextKeys.get(lvalue.identifier.id)!; const selectorFnInstr = emitSelectorFn(fn.env, keys); + if (nextInstructions === null) { + nextInstructions = block.instructions.slice(0, i); + } nextInstructions.push(selectorFnInstr); const selectorFn = selectorFnInstr.lvalue; value.args.push(selectorFn); } - nextInstructions.push(instr); + if (nextInstructions) { + nextInstructions.push(instr); + } + } + if (nextInstructions) { + block.instructions = nextInstructions; } - block.instructions = nextInstructions; } markInstructionIds(fn.body); } @@ -113,26 +121,18 @@ function getContextKeys(value: Destructure): Array | null { switch (pattern.kind) { case 'ArrayPattern': { - for (const place of pattern.items) { - if (place.kind !== 'Identifier') { - return null; - } - - if (place.identifier.name === null) { - return null; - } - - keys.push(place.identifier.name.value); - } - return keys; + return null; } case 'ObjectPattern': { for (const place of pattern.properties) { + debugger; if ( place.kind !== 'ObjectProperty' || place.type !== 'property' || - place.key.kind !== 'identifier' + place.key.kind !== 'identifier' || + place.place.identifier.name === null || + place.place.identifier.name.kind !== 'named' ) { return null; } @@ -153,13 +153,7 @@ function emitPropertyLoad( place: obj, loc: GeneratedSource, }; - const object: Place = { - kind: 'Identifier', - identifier: makeTemporary(env.nextIdentifierId, GeneratedSource), - effect: Effect.Unknown, - reactive: false, - loc: GeneratedSource, - }; + const object: Place = createTemporaryPlace(env, GeneratedSource); const loadLocalInstr: Instruction = { lvalue: object, value: loadObj, @@ -173,13 +167,7 @@ function emitPropertyLoad( property, loc: GeneratedSource, }; - const element: Place = { - kind: 'Identifier', - identifier: makeTemporary(env.nextIdentifierId, GeneratedSource), - effect: Effect.Unknown, - reactive: false, - loc: GeneratedSource, - }; + const element: Place = createTemporaryPlace(env, GeneratedSource); const loadPropInstr: Instruction = { lvalue: element, value: loadProp, @@ -193,20 +181,8 @@ function emitPropertyLoad( } function emitSelectorFn(env: Environment, keys: Array): Instruction { - const obj: Place = { - kind: 'Identifier', - identifier: { - id: makeIdentifierId(env.nextIdentifierId), - name: makeIdentifierName('c'), - mutableRange: {start: makeInstructionId(0), end: makeInstructionId(0)}, - scope: null, - type: makeType(), - loc: GeneratedSource, - }, - effect: Effect.Unknown, - reactive: false, - loc: GeneratedSource, - }; + const obj: Place = createTemporaryPlace(env, GeneratedSource); + promoteTemporary(obj.identifier); const instr: Array = []; const elements = []; for (const key of keys) { @@ -251,11 +227,7 @@ function emitSelectorFn(env: Environment, keys: Array): Instruction { }; reversePostorderBlocks(fn.body); - removeUnreachableForUpdates(fn.body); - removeDeadDoWhileStatements(fn.body); - removeUnnecessaryTryCatch(fn.body); markInstructionIds(fn.body); - mergeConsecutiveBlocks(fn); enterSSA(fn); inferTypes(fn); diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateLocalsNotReassignedAfterRender.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateLocalsNotReassignedAfterRender.ts index cde56f748404a..2dab01f86e838 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateLocalsNotReassignedAfterRender.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateLocalsNotReassignedAfterRender.ts @@ -8,9 +8,11 @@ import {CompilerError, Effect} from '..'; import {HIRFunction, IdentifierId, Place} from '../HIR'; import { + eachInstructionLValue, eachInstructionValueOperand, eachTerminalOperand, } from '../HIR/visitors'; +import {getFunctionCallSignature} from '../Inference/InferReferenceEffects'; /** * Validates that local variables cannot be reassigned after render. @@ -131,7 +133,26 @@ function getContextReassignment( break; } default: { - for (const operand of eachInstructionValueOperand(value)) { + let operands = eachInstructionValueOperand(value); + // If we're calling a function that doesn't let its arguments escape, only test the callee + if (value.kind === 'CallExpression') { + const signature = getFunctionCallSignature( + fn.env, + value.callee.identifier.type, + ); + if (signature?.noAlias) { + operands = [value.callee]; + } + } else if (value.kind === 'MethodCall') { + const signature = getFunctionCallSignature( + fn.env, + value.property.identifier.type, + ); + if (signature?.noAlias) { + operands = [value.receiver, value.property]; + } + } + for (const operand of operands) { CompilerError.invariant(operand.effect !== Effect.Unknown, { reason: `Expected effects to be inferred prior to ValidateLocalsNotReassignedAfterRender`, loc: operand.loc, @@ -139,15 +160,22 @@ function getContextReassignment( const reassignment = reassigningFunctions.get( operand.identifier.id, ); - if ( - reassignment !== undefined && - operand.effect === Effect.Freeze - ) { + if (reassignment !== undefined) { /* * Functions that reassign local variables are inherently mutable and are unsafe to pass * to a place that expects a frozen value. Propagate the reassignment upward. */ - return reassignment; + if (operand.effect === Effect.Freeze) { + return reassignment; + } else { + /* + * If the operand is not frozen but it does reassign, then the lvalues + * of the instruction could also be reassigning + */ + for (const lval of eachInstructionLValue(instr)) { + reassigningFunctions.set(lval.identifier.id, reassignment); + } + } } } break; diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoSetStateInRender.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoSetStateInRender.ts index 5c6339a2be539..3f378b1289e6c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoSetStateInRender.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoSetStateInRender.ts @@ -6,7 +6,7 @@ */ import {CompilerError, ErrorSeverity} from '../CompilerError'; -import {HIRFunction, IdentifierId, Place, isSetStateType} from '../HIR'; +import {HIRFunction, IdentifierId, isSetStateType} from '../HIR'; import {computeUnconditionalBlocks} from '../HIR/ComputeUnconditionalBlocks'; import {eachInstructionValueOperand} from '../HIR/visitors'; import {Err, Ok, Result} from '../Utils/Result'; @@ -49,63 +49,97 @@ function validateNoSetStateInRenderImpl( unconditionalSetStateFunctions: Set, ): Result { const unconditionalBlocks = computeUnconditionalBlocks(fn); - + let activeManualMemoId: number | null = null; const errors = new CompilerError(); for (const [, block] of fn.body.blocks) { - if (unconditionalBlocks.has(block.id)) { - for (const instr of block.instructions) { - switch (instr.value.kind) { - case 'LoadLocal': { - if ( - unconditionalSetStateFunctions.has( - instr.value.place.identifier.id, - ) - ) { - unconditionalSetStateFunctions.add(instr.lvalue.identifier.id); - } - break; + for (const instr of block.instructions) { + switch (instr.value.kind) { + case 'LoadLocal': { + if ( + unconditionalSetStateFunctions.has(instr.value.place.identifier.id) + ) { + unconditionalSetStateFunctions.add(instr.lvalue.identifier.id); } - case 'StoreLocal': { - if ( - unconditionalSetStateFunctions.has( - instr.value.value.identifier.id, - ) - ) { - unconditionalSetStateFunctions.add( - instr.value.lvalue.place.identifier.id, - ); - unconditionalSetStateFunctions.add(instr.lvalue.identifier.id); - } - break; - } - case 'ObjectMethod': - case 'FunctionExpression': { - if ( - // faster-path to check if the function expression references a setState - [...eachInstructionValueOperand(instr.value)].some( - operand => - isSetStateType(operand.identifier) || - unconditionalSetStateFunctions.has(operand.identifier.id), - ) && - // if yes, does it unconditonally call it? - validateNoSetStateInRenderImpl( - instr.value.loweredFunc.func, - unconditionalSetStateFunctions, - ).isErr() - ) { - // This function expression unconditionally calls a setState - unconditionalSetStateFunctions.add(instr.lvalue.identifier.id); - } - break; + break; + } + case 'StoreLocal': { + if ( + unconditionalSetStateFunctions.has(instr.value.value.identifier.id) + ) { + unconditionalSetStateFunctions.add( + instr.value.lvalue.place.identifier.id, + ); + unconditionalSetStateFunctions.add(instr.lvalue.identifier.id); } - case 'CallExpression': { - validateNonSetState( - errors, + break; + } + case 'ObjectMethod': + case 'FunctionExpression': { + if ( + // faster-path to check if the function expression references a setState + [...eachInstructionValueOperand(instr.value)].some( + operand => + isSetStateType(operand.identifier) || + unconditionalSetStateFunctions.has(operand.identifier.id), + ) && + // if yes, does it unconditonally call it? + validateNoSetStateInRenderImpl( + instr.value.loweredFunc.func, unconditionalSetStateFunctions, - instr.value.callee, - ); - break; + ).isErr() + ) { + // This function expression unconditionally calls a setState + unconditionalSetStateFunctions.add(instr.lvalue.identifier.id); } + break; + } + case 'StartMemoize': { + CompilerError.invariant(activeManualMemoId === null, { + reason: 'Unexpected nested StartMemoize instructions', + loc: instr.value.loc, + }); + activeManualMemoId = instr.value.manualMemoId; + break; + } + case 'FinishMemoize': { + CompilerError.invariant( + activeManualMemoId === instr.value.manualMemoId, + { + reason: + 'Expected FinishMemoize to align with previous StartMemoize instruction', + loc: instr.value.loc, + }, + ); + activeManualMemoId = null; + break; + } + case 'CallExpression': { + const callee = instr.value.callee; + if ( + isSetStateType(callee.identifier) || + unconditionalSetStateFunctions.has(callee.identifier.id) + ) { + if (activeManualMemoId !== null) { + errors.push({ + reason: + 'Calling setState from useMemo may trigger an infinite loop. (https://react.dev/reference/react/useState)', + description: null, + severity: ErrorSeverity.InvalidReact, + loc: callee.loc, + suggestions: null, + }); + } else if (unconditionalBlocks.has(block.id)) { + errors.push({ + reason: + 'This is an unconditional set state during render, which will trigger an infinite loop. (https://react.dev/reference/react/useState)', + description: null, + severity: ErrorSeverity.InvalidReact, + loc: callee.loc, + suggestions: null, + }); + } + } + break; } } } @@ -117,23 +151,3 @@ function validateNoSetStateInRenderImpl( return Ok(undefined); } } - -function validateNonSetState( - errors: CompilerError, - unconditionalSetStateFunctions: Set, - operand: Place, -): void { - if ( - isSetStateType(operand.identifier) || - unconditionalSetStateFunctions.has(operand.identifier.id) - ) { - errors.push({ - reason: - 'This is an unconditional set state during render, which will trigger an infinite loop. (https://react.dev/reference/react/useState)', - description: null, - severity: ErrorSeverity.InvalidReact, - loc: typeof operand.loc !== 'symbol' ? operand.loc : null, - suggestions: null, - }); - } -} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-global-mutation-unused-usecallback.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-global-mutation-unused-usecallback.expect.md index 68bc0d5913010..f5c393c174656 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-global-mutation-unused-usecallback.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-global-mutation-unused-usecallback.expect.md @@ -36,6 +36,9 @@ function Component() { } return t0; } +function _temp() { + window.foo = true; +} export const FIXTURE_ENTRYPOINT = { fn: Component, diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-effect-indirect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-effect-indirect.expect.md index 207c884aeb389..e8ed54ef262e6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-effect-indirect.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-effect-indirect.expect.md @@ -2,7 +2,7 @@ ## Input ```javascript -// @validateRefAccessDuringRender +// @validateRefAccessDuringRender @validateNoSetStateInRender:false import {useCallback, useEffect, useRef, useState} from 'react'; function Component() { @@ -42,7 +42,7 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRender +import { c as _c } from "react/compiler-runtime"; // @validateRefAccessDuringRender @validateNoSetStateInRender:false import { useCallback, useEffect, useRef, useState } from "react"; function Component() { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-effect-indirect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-effect-indirect.js index cf311d4df6c21..69429049022ab 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-effect-indirect.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-ref-access-in-effect-indirect.js @@ -1,4 +1,4 @@ -// @validateRefAccessDuringRender +// @validateRefAccessDuringRender @validateNoSetStateInRender:false import {useCallback, useEffect, useRef, useState} from 'react'; function Component() { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/context-variable-only-chained-assign.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/context-variable-only-chained-assign.expect.md deleted file mode 100644 index 4b5f4690af392..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/context-variable-only-chained-assign.expect.md +++ /dev/null @@ -1,65 +0,0 @@ - -## Input - -```javascript -import {identity, invoke} from 'shared-runtime'; - -function foo() { - let x = 2; - const fn1 = () => { - const copy1 = (x = 3); - return identity(copy1); - }; - const fn2 = () => { - const copy2 = (x = 4); - return [invoke(fn1), copy2, identity(copy2)]; - }; - return invoke(fn2); -} - -export const FIXTURE_ENTRYPOINT = { - fn: foo, - params: [], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; -import { identity, invoke } from "shared-runtime"; - -function foo() { - const $ = _c(1); - let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - let x; - x = 2; - const fn1 = () => { - const copy1 = (x = 3); - return identity(copy1); - }; - - const fn2 = () => { - const copy2 = (x = 4); - return [invoke(fn1), copy2, identity(copy2)]; - }; - - t0 = invoke(fn2); - $[0] = t0; - } else { - t0 = $[0]; - } - return t0; -} - -export const FIXTURE_ENTRYPOINT = { - fn: foo, - params: [], -}; - -``` - -### Eval output -(kind: ok) [3,4,4] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/declare-reassign-variable-in-function-declaration.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/declare-reassign-variable-in-function-declaration.expect.md deleted file mode 100644 index b9fc15ea0a7be..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/declare-reassign-variable-in-function-declaration.expect.md +++ /dev/null @@ -1,40 +0,0 @@ - -## Input - -```javascript -function Component() { - let x = null; - function foo() { - x = 9; - } - const y = bar(foo); - return ; -} - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; -function Component() { - const $ = _c(1); - let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - let x; - x = null; - const foo = function foo() { - x = 9; - }; - - const y = bar(foo); - t0 = ; - $[0] = t0; - } else { - t0 = $[0]; - } - return t0; -} - -``` - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.context-variable-only-chained-assign.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.context-variable-only-chained-assign.expect.md new file mode 100644 index 0000000000000..0318fa9525fda --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.context-variable-only-chained-assign.expect.md @@ -0,0 +1,40 @@ + +## Input + +```javascript +import {identity, invoke} from 'shared-runtime'; + +function foo() { + let x = 2; + const fn1 = () => { + const copy1 = (x = 3); + return identity(copy1); + }; + const fn2 = () => { + const copy2 = (x = 4); + return [invoke(fn1), copy2, identity(copy2)]; + }; + return invoke(fn2); +} + +export const FIXTURE_ENTRYPOINT = { + fn: foo, + params: [], +}; + +``` + + +## Error + +``` + 8 | }; + 9 | const fn2 = () => { +> 10 | const copy2 = (x = 4); + | ^ 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 (10:10) + 11 | return [invoke(fn1), copy2, identity(copy2)]; + 12 | }; + 13 | return invoke(fn2); +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/context-variable-only-chained-assign.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.context-variable-only-chained-assign.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/context-variable-only-chained-assign.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.context-variable-only-chained-assign.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.declare-reassign-variable-in-function-declaration.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.declare-reassign-variable-in-function-declaration.expect.md new file mode 100644 index 0000000000000..2a6dce11f242e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.declare-reassign-variable-in-function-declaration.expect.md @@ -0,0 +1,29 @@ + +## Input + +```javascript +function Component() { + let x = null; + function foo() { + x = 9; + } + const y = bar(foo); + return ; +} + +``` + + +## Error + +``` + 2 | let x = null; + 3 | function foo() { +> 4 | x = 9; + | ^ 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 | } + 6 | const y = bar(foo); + 7 | return ; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/declare-reassign-variable-in-function-declaration.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.declare-reassign-variable-in-function-declaration.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/declare-reassign-variable-in-function-declaration.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.declare-reassign-variable-in-function-declaration.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-conditional-setState-in-useMemo.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-conditional-setState-in-useMemo.expect.md new file mode 100644 index 0000000000000..f1666cc4013f7 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-conditional-setState-in-useMemo.expect.md @@ -0,0 +1,36 @@ + +## Input + +```javascript +function Component({item, cond}) { + const [prevItem, setPrevItem] = useState(item); + const [state, setState] = useState(0); + + useMemo(() => { + if (cond) { + setPrevItem(item); + setState(0); + } + }, [cond, key, init]); + + return state; +} + +``` + + +## Error + +``` + 5 | useMemo(() => { + 6 | if (cond) { +> 7 | setPrevItem(item); + | ^^^^^^^^^^^ InvalidReact: Calling setState from useMemo may trigger an infinite loop. (https://react.dev/reference/react/useState) (7:7) + +InvalidReact: Calling setState from useMemo may trigger an infinite loop. (https://react.dev/reference/react/useState) (8:8) + 8 | setState(0); + 9 | } + 10 | }, [cond, key, init]); +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-conditional-setState-in-useMemo.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-conditional-setState-in-useMemo.js new file mode 100644 index 0000000000000..4385ef6a6cc3c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-conditional-setState-in-useMemo.js @@ -0,0 +1,13 @@ +function Component({item, cond}) { + const [prevItem, setPrevItem] = useState(item); + const [state, setState] = useState(0); + + useMemo(() => { + if (cond) { + setPrevItem(item); + setState(0); + } + }, [cond, key, init]); + + return state; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-nested-function-reassign-local-variable-in-effect.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-nested-function-reassign-local-variable-in-effect.expect.md similarity index 55% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-nested-function-reassign-local-variable-in-effect.expect.md rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-nested-function-reassign-local-variable-in-effect.expect.md index 8e415a2fdd4cc..c1aec8a432544 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-nested-function-reassign-local-variable-in-effect.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-nested-function-reassign-local-variable-in-effect.expect.md @@ -42,60 +42,17 @@ function Component() { ``` -## Code -```javascript -import { c as _c } from "react/compiler-runtime"; -import { useEffect } from "react"; -function Component() { - const $ = _c(4); - let local; - let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - const mk_reassignlocal = () => { - const reassignLocal = (newValue) => { - local = newValue; - }; - return reassignLocal; - }; - - t0 = mk_reassignlocal(); - $[0] = t0; - } else { - t0 = $[0]; - } - const reassignLocal_0 = t0; - let t1; - if ($[1] === Symbol.for("react.memo_cache_sentinel")) { - t1 = (newValue_0) => { - reassignLocal_0("hello"); - if (local === newValue_0) { - console.log("`local` was updated!"); - } else { - throw new Error("`local` not updated!"); - } - }; - $[1] = t1; - } else { - t1 = $[1]; - } - const onMount = t1; - let t2; - let t3; - if ($[2] === Symbol.for("react.memo_cache_sentinel")) { - t2 = () => { - onMount(); - }; - t3 = [onMount]; - $[2] = t2; - $[3] = t3; - } else { - t2 = $[2]; - t3 = $[3]; - } - useEffect(t2, t3); - return "ok"; -} +## Error ``` + 5 | // Create the reassignment function inside another function, then return it + 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 | return reassignLocal; + 10 | }; +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-nested-function-reassign-local-variable-in-effect.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-nested-function-reassign-local-variable-in-effect.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.invalid-nested-function-reassign-local-variable-in-effect.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-nested-function-reassign-local-variable-in-effect.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-setState-in-useMemo-indirect-useCallback.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-setState-in-useMemo-indirect-useCallback.expect.md new file mode 100644 index 0000000000000..acf84652b2d79 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-setState-in-useMemo-indirect-useCallback.expect.md @@ -0,0 +1,38 @@ + +## Input + +```javascript +import {useCallback} from 'react'; + +function useKeyedState({key, init}) { + const [prevKey, setPrevKey] = useState(key); + const [state, setState] = useState(init); + + const fn = useCallback(() => { + setPrevKey(key); + setState(init); + }); + + useMemo(() => { + fn(); + }, [key, init]); + + return state; +} + +``` + + +## Error + +``` + 11 | + 12 | useMemo(() => { +> 13 | fn(); + | ^^ InvalidReact: Calling setState from useMemo may trigger an infinite loop. (https://react.dev/reference/react/useState) (13:13) + 14 | }, [key, init]); + 15 | + 16 | return state; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-setState-in-useMemo-indirect-useCallback.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-setState-in-useMemo-indirect-useCallback.js new file mode 100644 index 0000000000000..dbdf5f5a4da3c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-setState-in-useMemo-indirect-useCallback.js @@ -0,0 +1,17 @@ +import {useCallback} from 'react'; + +function useKeyedState({key, init}) { + const [prevKey, setPrevKey] = useState(key); + const [state, setState] = useState(init); + + const fn = useCallback(() => { + setPrevKey(key); + setState(init); + }); + + useMemo(() => { + fn(); + }, [key, init]); + + return state; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-setState-in-useMemo.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-setState-in-useMemo.expect.md new file mode 100644 index 0000000000000..aee09b179083f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-setState-in-useMemo.expect.md @@ -0,0 +1,34 @@ + +## Input + +```javascript +function useKeyedState({key, init}) { + const [prevKey, setPrevKey] = useState(key); + const [state, setState] = useState(init); + + useMemo(() => { + setPrevKey(key); + setState(init); + }, [key, init]); + + return state; +} + +``` + + +## Error + +``` + 4 | + 5 | useMemo(() => { +> 6 | setPrevKey(key); + | ^^^^^^^^^^ InvalidReact: Calling setState from useMemo may trigger an infinite loop. (https://react.dev/reference/react/useState) (6:6) + +InvalidReact: Calling setState from useMemo may trigger an infinite loop. (https://react.dev/reference/react/useState) (7:7) + 7 | setState(init); + 8 | }, [key, init]); + 9 | +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-setState-in-useMemo.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-setState-in-useMemo.js new file mode 100644 index 0000000000000..ebbb6b2d14bfc --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-setState-in-useMemo.js @@ -0,0 +1,11 @@ +function useKeyedState({key, init}) { + const [prevKey, setPrevKey] = useState(key); + const [state, setState] = useState(init); + + useMemo(() => { + setPrevKey(key); + setState(init); + }, [key, init]); + + return state; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.mutable-range-shared-inner-outer-function.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.mutable-range-shared-inner-outer-function.expect.md new file mode 100644 index 0000000000000..cdcd6b3ffad99 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.mutable-range-shared-inner-outer-function.expect.md @@ -0,0 +1,44 @@ + +## Input + +```javascript +// @enableAssumeHooksFollowRulesOfReact @enableTransitivelyFreezeFunctionExpressions +let cond = true; +function Component(props) { + let a; + let b; + const f = () => { + if (cond) { + a = {}; + b = []; + } else { + a = {}; + b = []; + } + a.property = true; + b.push(false); + }; + return
; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + +``` + + +## 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 = {}; +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-mutable-range-shared-inner-outer-function.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.mutable-range-shared-inner-outer-function.js similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-mutable-range-shared-inner-outer-function.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.mutable-range-shared-inner-outer-function.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/lower-context-acess-multiple.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/lower-context-acess-multiple.expect.md index b7ee5e1902a39..0f9124536c55f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/lower-context-acess-multiple.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/lower-context-acess-multiple.expect.md @@ -30,11 +30,11 @@ function App() { } return t0; } -function _temp2(c) { - return [c.bar]; +function _temp2(t0) { + return [t0.bar]; } -function _temp(c) { - return [c.foo]; +function _temp(t0) { + return [t0.foo]; } ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/lower-context-selector-simple.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/lower-context-selector-simple.expect.md index 17405b518d365..3385b58b4aca8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/lower-context-selector-simple.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/lower-context-selector-simple.expect.md @@ -28,8 +28,8 @@ function App() { } return t0; } -function _temp(c) { - return [c.foo, c.bar]; +function _temp(t0) { + return [t0.foo, t0.bar]; } ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-mutable-range-shared-inner-outer-function.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-mutable-range-shared-inner-outer-function.expect.md deleted file mode 100644 index 945f68d4be107..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-mutable-range-shared-inner-outer-function.expect.md +++ /dev/null @@ -1,71 +0,0 @@ - -## Input - -```javascript -// @enableAssumeHooksFollowRulesOfReact @enableTransitivelyFreezeFunctionExpressions -let cond = true; -function Component(props) { - let a; - let b; - const f = () => { - if (cond) { - a = {}; - b = []; - } else { - a = {}; - b = []; - } - a.property = true; - b.push(false); - }; - return
; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{}], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; // @enableAssumeHooksFollowRulesOfReact @enableTransitivelyFreezeFunctionExpressions -let cond = true; -function Component(props) { - const $ = _c(1); - let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - let a; - let b; - const f = () => { - if (cond) { - a = {}; - b = []; - } else { - a = {}; - b = []; - } - - a.property = true; - b.push(false); - }; - - t0 =
; - $[0] = t0; - } else { - t0 = $[0]; - } - return t0; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Component, - params: [{}], -}; - -``` - -### Eval output -(kind: ok)
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro.expect.md index dcd252ab580fa..2e9daceed798a 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro.expect.md @@ -24,12 +24,14 @@ function Component(props) { ```javascript import { c as _c } from "react/compiler-runtime"; function Component(props) { - const $ = _c(2); + const $ = _c(7); const item = props.item; let t0; + let baseVideos; + let thumbnails; if ($[0] !== item) { - const thumbnails = []; - const baseVideos = getBaseVideos(item); + thumbnails = []; + baseVideos = getBaseVideos(item); baseVideos.forEach((video) => { const baseVideo = video.hasBaseVideo; @@ -37,14 +39,26 @@ function Component(props) { thumbnails.push({ extraVideo: true }); } }); - - t0 = ; $[0] = item; $[1] = t0; + $[2] = baseVideos; + $[3] = thumbnails; } else { t0 = $[1]; + baseVideos = $[2]; + thumbnails = $[3]; + } + t0 = undefined; + let t1; + if ($[4] !== baseVideos || $[5] !== thumbnails) { + t1 = ; + $[4] = baseVideos; + $[5] = thumbnails; + $[6] = t1; + } else { + t1 = $[6]; } - return t0; + return t1; } ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.lower-context-access-array-destructuring.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.lower-context-access-array-destructuring.expect.md new file mode 100644 index 0000000000000..c9d0215767dc5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.lower-context-access-array-destructuring.expect.md @@ -0,0 +1,35 @@ + +## Input + +```javascript +// @enableLowerContextAccess +function App() { + const [foo, bar] = useContext(MyContext); + return ; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableLowerContextAccess +function App() { + const $ = _c(3); + const [foo, bar] = useContext(MyContext); + let t0; + if ($[0] !== foo || $[1] !== bar) { + t0 = ; + $[0] = foo; + $[1] = bar; + $[2] = t0; + } else { + t0 = $[2]; + } + return t0; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.lower-context-access-array-destructuring.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.lower-context-access-array-destructuring.js new file mode 100644 index 0000000000000..b1f60bf8e7d97 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.lower-context-access-array-destructuring.js @@ -0,0 +1,5 @@ +// @enableLowerContextAccess +function App() { + const [foo, bar] = useContext(MyContext); + return ; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.lower-context-access-mixed-array-obj.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.lower-context-access-mixed-array-obj.expect.md new file mode 100644 index 0000000000000..647f246dc4f3f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.lower-context-access-mixed-array-obj.expect.md @@ -0,0 +1,39 @@ + +## Input + +```javascript +// @enableLowerContextAccess +function App() { + const context = useContext(MyContext); + const [foo] = context; + const {bar} = context; + return ; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableLowerContextAccess +function App() { + const $ = _c(3); + const context = useContext(MyContext); + const [foo] = context; + const { bar } = context; + let t0; + if ($[0] !== foo || $[1] !== bar) { + t0 = ; + $[0] = foo; + $[1] = bar; + $[2] = t0; + } else { + t0 = $[2]; + } + return t0; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.lower-context-access-mixed-array-obj.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.lower-context-access-mixed-array-obj.js new file mode 100644 index 0000000000000..2eefdcd76f9fd --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.lower-context-access-mixed-array-obj.js @@ -0,0 +1,7 @@ +// @enableLowerContextAccess +function App() { + const context = useContext(MyContext); + const [foo] = context; + const {bar} = context; + return ; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.lower-context-access-nested-destructuring.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.lower-context-access-nested-destructuring.expect.md new file mode 100644 index 0000000000000..07a505a2a2998 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.lower-context-access-nested-destructuring.expect.md @@ -0,0 +1,39 @@ + +## Input + +```javascript +// @enableLowerContextAccess +function App() { + const { + joe: {foo}, + bar, + } = useContext(MyContext); + return ; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableLowerContextAccess +function App() { + const $ = _c(3); + const { joe: t0, bar } = useContext(MyContext); + const { foo } = t0; + let t1; + if ($[0] !== foo || $[1] !== bar) { + t1 = ; + $[0] = foo; + $[1] = bar; + $[2] = t1; + } else { + t1 = $[2]; + } + return t1; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.lower-context-access-nested-destructuring.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.lower-context-access-nested-destructuring.js new file mode 100644 index 0000000000000..ceecaaaaffb43 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/todo.lower-context-access-nested-destructuring.js @@ -0,0 +1,8 @@ +// @enableLowerContextAccess +function App() { + const { + joe: {foo}, + bar, + } = useContext(MyContext); + return ; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-named-function.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-named-function.expect.md index 9bcc56f32fc90..9c87512a0f7c8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-named-function.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-named-function.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +// @validateNoSetStateInRender:false import {useMemo} from 'react'; import {makeArray} from 'shared-runtime'; @@ -20,7 +21,7 @@ export const FIXTURE_ENTRYPOINT = { ## Code ```javascript -import { c as _c } from "react/compiler-runtime"; +import { c as _c } from "react/compiler-runtime"; // @validateNoSetStateInRender:false import { useMemo } from "react"; import { makeArray } from "shared-runtime"; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-named-function.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-named-function.ts index cd84e42862f77..317491efbfe72 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-named-function.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-named-function.ts @@ -1,3 +1,4 @@ +// @validateNoSetStateInRender:false import {useMemo} from 'react'; import {makeArray} from 'shared-runtime'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-nested-ifs.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-nested-ifs.expect.md index f7b3605f4d5a0..12f51643dd4d4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-nested-ifs.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-nested-ifs.expect.md @@ -24,10 +24,12 @@ export const FIXTURE_ENTRYPOINT = { ```javascript function Component(props) { + let t0; if (props.cond) { if (props.cond) { } } + t0 = undefined; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-return-empty.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-return-empty.expect.md index be20ee39bd4ec..b348ae34b6f56 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-return-empty.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-return-empty.expect.md @@ -15,7 +15,10 @@ function component(a) { ```javascript function component(a) { + let t0; + mutate(a); + t0 = undefined; } ``` diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index 943163a420c4b..8e7a50a629977 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -187,7 +187,6 @@ const skipFilter = new Set([ 'alias-nested-member-path-mutate', 'concise-arrow-expr', 'const-propagation-into-function-expression-global', - 'declare-reassign-variable-in-function-declaration', 'lambda-mutate-shadowed-object', 'fbt/lambda-with-fbt', 'recursive-function-expr', @@ -503,8 +502,6 @@ const skipFilter = new Set([ // needs to be executed as a module 'meta-property', - - 'todo.invalid-nested-function-reassign-local-variable-in-effect', ]); export default skipFilter; diff --git a/packages/react-art/src/ReactFiberConfigART.js b/packages/react-art/src/ReactFiberConfigART.js index e80e6100ec534..816353e0ced2f 100644 --- a/packages/react-art/src/ReactFiberConfigART.js +++ b/packages/react-art/src/ReactFiberConfigART.js @@ -16,6 +16,8 @@ import { DefaultEventPriority, NoEventPriority, } from 'react-reconciler/src/ReactEventPriorities'; +import type {ReactContext} from 'shared/ReactTypes'; +import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols'; export {default as rendererVersion} from 'shared/ReactVersion'; export const rendererPackageName = 'react-art'; @@ -28,6 +30,8 @@ if (__DEV__) { Object.freeze(NO_CONTEXT); } +export type TransitionStatus = mixed; + /** Helper Methods */ function addEventListeners(instance, type, listener) { @@ -488,4 +492,12 @@ export function waitForCommitToBeReady() { } export const NotPendingTransition = null; +export const HostTransitionContext: ReactContext = { + $$typeof: REACT_CONTEXT_TYPE, + Provider: (null: any), + Consumer: (null: any), + _currentValue: NotPendingTransition, + _currentValue2: NotPendingTransition, + _threadCount: 0, +}; export function resetFormInstance() {} diff --git a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegrationDOM-test.js b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegrationDOM-test.js index 752d93fa30ab8..3ef3c2f3a7b7b 100644 --- a/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegrationDOM-test.js +++ b/packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegrationDOM-test.js @@ -119,7 +119,12 @@ describe('ReactHooksInspectionIntegration', () => { isStateEditable: false, name: 'FormStatus', subHooks: [], - value: null, + value: { + action: null, + data: null, + method: null, + pending: false, + }, }, { debugInfo: null, diff --git a/packages/react-devtools-core/webpack.backend.js b/packages/react-devtools-core/webpack.backend.js index 7efd5b0b5be15..32d4fadcb5884 100644 --- a/packages/react-devtools-core/webpack.backend.js +++ b/packages/react-devtools-core/webpack.backend.js @@ -71,6 +71,7 @@ module.exports = { __IS_FIREFOX__: false, __IS_CHROME__: false, __IS_EDGE__: false, + __IS_NATIVE__: true, 'process.env.DEVTOOLS_PACKAGE': `"react-devtools-core"`, 'process.env.DEVTOOLS_VERSION': `"${DEVTOOLS_VERSION}"`, 'process.env.GITHUB_URL': `"${GITHUB_URL}"`, diff --git a/packages/react-devtools-extensions/webpack.backend.js b/packages/react-devtools-extensions/webpack.backend.js index a8069184a4aaf..effa6cc330bb0 100644 --- a/packages/react-devtools-extensions/webpack.backend.js +++ b/packages/react-devtools-extensions/webpack.backend.js @@ -77,6 +77,7 @@ module.exports = { __IS_CHROME__: IS_CHROME, __IS_FIREFOX__: IS_FIREFOX, __IS_EDGE__: IS_EDGE, + __IS_NATIVE__: false, }), new Webpack.SourceMapDevToolPlugin({ filename: '[file].map', diff --git a/packages/react-devtools-extensions/webpack.config.js b/packages/react-devtools-extensions/webpack.config.js index aef6e93742b03..81bf4a1c520b3 100644 --- a/packages/react-devtools-extensions/webpack.config.js +++ b/packages/react-devtools-extensions/webpack.config.js @@ -112,6 +112,7 @@ module.exports = { __IS_CHROME__: IS_CHROME, __IS_FIREFOX__: IS_FIREFOX, __IS_EDGE__: IS_EDGE, + __IS_NATIVE__: false, __IS_INTERNAL_VERSION__: IS_INTERNAL_VERSION, 'process.env.DEVTOOLS_PACKAGE': `"react-devtools-extensions"`, 'process.env.DEVTOOLS_VERSION': `"${DEVTOOLS_VERSION}"`, diff --git a/packages/react-devtools-inline/webpack.config.js b/packages/react-devtools-inline/webpack.config.js index 7b153bbc13b13..3a92dff1f2195 100644 --- a/packages/react-devtools-inline/webpack.config.js +++ b/packages/react-devtools-inline/webpack.config.js @@ -77,6 +77,7 @@ module.exports = { __IS_CHROME__: false, __IS_FIREFOX__: false, __IS_EDGE__: false, + __IS_NATIVE__: false, 'process.env.DEVTOOLS_PACKAGE': `"react-devtools-inline"`, 'process.env.DEVTOOLS_VERSION': `"${DEVTOOLS_VERSION}"`, 'process.env.EDITOR_URL': EDITOR_URL != null ? `"${EDITOR_URL}"` : null, diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js index ba6bd0ad092af..ee6c4431b9526 100644 --- a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js +++ b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js @@ -117,12 +117,11 @@ describe('InspectedElement', () => { - - - {children} - - + defaultSelectedElementIndex={defaultSelectedElementIndex} + defaultInspectedElementID={defaultSelectedElementID}> + + {children} + diff --git a/packages/react-devtools-shared/src/__tests__/setupTests.js b/packages/react-devtools-shared/src/__tests__/setupTests.js index b78a3d162629c..79cda67f99b18 100644 --- a/packages/react-devtools-shared/src/__tests__/setupTests.js +++ b/packages/react-devtools-shared/src/__tests__/setupTests.js @@ -128,6 +128,11 @@ beforeEach(() => { // Fake timers let us flush Bridge operations between setup and assertions. jest.useFakeTimers(); + // We use fake timers heavily in tests but the bridge batching now uses microtasks. + global.devtoolsJestTestScheduler = callback => { + setTimeout(callback, 0); + }; + // Use utils.js#withErrorsOrWarningsIgnored instead of directly mutating this array. global._ignoredErrorOrWarningMessages = [ 'react-test-renderer is deprecated.', diff --git a/packages/react-devtools-shared/src/backend/console.js b/packages/react-devtools-shared/src/backend/console.js index 42b08a7205bf2..93725c4428269 100644 --- a/packages/react-devtools-shared/src/backend/console.js +++ b/packages/react-devtools-shared/src/backend/console.js @@ -294,7 +294,7 @@ export function patch({ // formatting. Otherwise it is left alone. So we prefix it. Otherwise we just override it // to our own stack. fakeError.stack = - __IS_CHROME__ || __IS_EDGE__ + __IS_CHROME__ || __IS_EDGE__ || __IS_NATIVE__ ? (enableOwnerStacks ? 'Error Stack:' : 'Error Component Stack:') + componentStack diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 8ba5e6503651d..75cf2c7c03755 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -959,7 +959,7 @@ export function attach( const debug = ( name: string, fiber: Fiber, - parentFiber: ?Fiber, + parentInstance: null | DevToolsInstance, extraString: string = '', ): void => { if (__DEBUG__) { @@ -967,18 +967,25 @@ export function attach( fiber.tag + ':' + (getDisplayNameForFiber(fiber) || 'null'); const maybeID = getFiberIDUnsafe(fiber) || ''; - const parentDisplayName = parentFiber - ? parentFiber.tag + + + let parentDisplayName; + let maybeParentID; + if (parentInstance !== null && parentInstance.kind === FIBER_INSTANCE) { + const parentFiber = parentInstance.data; + parentDisplayName = + parentFiber.tag + ':' + - (getDisplayNameForFiber(parentFiber) || 'null') - : ''; - const maybeParentID = parentFiber - ? getFiberIDUnsafe(parentFiber) || '' - : ''; + (getDisplayNameForFiber(parentFiber) || 'null'); + maybeParentID = String(parentInstance.id); + } else { + // TODO: Handle VirtualInstance + parentDisplayName = ''; + maybeParentID = ''; + } console.groupCollapsed( `[renderer] %c${name} %c${displayName} (${maybeID}) %c${ - parentFiber ? `${parentDisplayName} (${maybeParentID})` : '' + parentInstance ? `${parentDisplayName} (${maybeParentID})` : '' } %c${extraString}`, 'color: red; font-weight: bold;', 'color: blue;', @@ -1069,7 +1076,7 @@ export function attach( // Recursively unmount all roots. hook.getFiberRoots(rendererID).forEach(root => { - currentRootID = getOrGenerateFiberID(root.current); + currentRootID = getOrGenerateFiberInstance(root.current).id; // The TREE_OPERATION_REMOVE_ROOT operation serves two purposes: // 1. It avoids sending unnecessary bridge traffic to clear a root. // 2. It preserves Fiber IDs when remounting (below) which in turn ID to error/warning mapping. @@ -1085,7 +1092,7 @@ export function attach( // Recursively re-mount all roots with new filter criteria applied. hook.getFiberRoots(rendererID).forEach(root => { - currentRootID = getOrGenerateFiberID(root.current); + currentRootID = getOrGenerateFiberInstance(root.current).id; setRootPseudoKey(currentRootID, root.current); mountFiberRecursively(root.current, null, false, false); flushPendingEvents(root); @@ -1097,6 +1104,11 @@ export function attach( flushPendingEvents(); } + function shouldFilterVirtual(data: ReactComponentInfo): boolean { + // TODO: Apply filters to VirtualInstances. + return false; + } + // NOTICE Keep in sync with get*ForFiber methods function shouldFilterFiber(fiber: Fiber): boolean { const {tag, type, key} = fiber; @@ -1245,7 +1257,7 @@ export function attach( // Returns the unique ID for a Fiber or generates and caches a new one if the Fiber hasn't been seen before. // Once this method has been called for a Fiber, untrackFiberID() should always be called later to avoid leaking. - function getOrGenerateFiberID(fiber: Fiber): number { + function getOrGenerateFiberInstance(fiber: Fiber): FiberInstance { let fiberInstance = fiberToFiberInstanceMap.get(fiber); if (fiberInstance === undefined) { const {alternate} = fiber; @@ -1269,51 +1281,66 @@ export function attach( if (__DEBUG__) { if (didGenerateID) { debug( - 'getOrGenerateFiberID()', + 'getOrGenerateFiberInstance()', fiber, - fiber.return, + fiberInstance.parent, 'Generated a new UID', ); } } - return fiberInstance.id; + return fiberInstance; + } + + // Returns a FiberInstance if one has already been generated for the Fiber or throws. + function getFiberInstanceThrows(fiber: Fiber): FiberInstance { + const fiberInstance = getFiberInstanceUnsafe(fiber); + if (fiberInstance !== null) { + return fiberInstance; + } + throw Error( + `Could not find ID for Fiber "${getDisplayNameForFiber(fiber) || ''}"`, + ); } - // Returns an ID if one has already been generated for the Fiber or throws. function getFiberIDThrows(fiber: Fiber): number { - const maybeID = getFiberIDUnsafe(fiber); - if (maybeID !== null) { - return maybeID; + const fiberInstance = getFiberInstanceUnsafe(fiber); + if (fiberInstance !== null) { + return fiberInstance.id; } throw Error( `Could not find ID for Fiber "${getDisplayNameForFiber(fiber) || ''}"`, ); } - // Returns an ID if one has already been generated for the Fiber or null if one has not been generated. + // Returns a FiberInstance if one has already been generated for the Fiber or null if one has not been generated. // Use this method while e.g. logging to avoid over-retaining Fibers. - function getFiberIDUnsafe(fiber: Fiber): number | null { + function getFiberInstanceUnsafe(fiber: Fiber): FiberInstance | null { const fiberInstance = fiberToFiberInstanceMap.get(fiber); if (fiberInstance !== undefined) { - return fiberInstance.id; + return fiberInstance; } else { const {alternate} = fiber; if (alternate !== null) { const alternateInstance = fiberToFiberInstanceMap.get(alternate); if (alternateInstance !== undefined) { - return alternateInstance.id; + return alternateInstance; } } } return null; } + function getFiberIDUnsafe(fiber: Fiber): number | null { + const fiberInstance = getFiberInstanceUnsafe(fiber); + return fiberInstance === null ? null : fiberInstance.id; + } + // Removes a Fiber (and its alternate) from the Maps used to track their id. // This method should always be called when a Fiber is unmounting. function untrackFiberID(fiber: Fiber) { if (__DEBUG__) { - debug('untrackFiberID()', fiber, fiber.return, 'schedule after delay'); + debug('untrackFiberID()', fiber, null, 'schedule after delay'); } // Untrack Fibers after a slight delay in order to support a Fast Refresh edge case: @@ -2023,14 +2050,21 @@ export function attach( return id; } - function recordMount(fiber: Fiber, parentFiber: Fiber | null) { + function recordMount( + fiber: Fiber, + parentInstance: DevToolsInstance | null, + ): FiberInstance { const isRoot = fiber.tag === HostRoot; - const id = getOrGenerateFiberID(fiber); + const fiberInstance = getOrGenerateFiberInstance(fiber); + const id = fiberInstance.id; if (__DEBUG__) { - debug('recordMount()', fiber, parentFiber); + debug('recordMount()', fiber, parentInstance); } + // We're placing it in its parent below. + fiberInstance.parent = parentInstance; + const hasOwnerMetadata = fiber.hasOwnProperty('_debugOwner'); const isProfilingSupported = fiber.hasOwnProperty('treeBaseDuration'); @@ -2077,7 +2111,7 @@ export function attach( let ownerID: number; if (debugOwner != null) { if (typeof debugOwner.tag === 'number') { - ownerID = getOrGenerateFiberID((debugOwner: any)); + ownerID = getOrGenerateFiberInstance((debugOwner: any)).id; } else { // TODO: Track Server Component Owners. ownerID = 0; @@ -2085,7 +2119,7 @@ export function attach( } else { ownerID = 0; } - const parentID = parentFiber ? getFiberIDThrows(parentFiber) : 0; + const parentID = parentInstance ? parentInstance.id : 0; const displayNameStringID = getStringID(displayName); @@ -2103,13 +2137,21 @@ export function attach( pushOperation(keyStringID); // If this subtree has a new mode, let the frontend know. - if ( - (fiber.mode & StrictModeBits) !== 0 && - (((parentFiber: any): Fiber).mode & StrictModeBits) === 0 - ) { - pushOperation(TREE_OPERATION_SET_SUBTREE_MODE); - pushOperation(id); - pushOperation(StrictMode); + if ((fiber.mode & StrictModeBits) !== 0) { + let parentFiber = null; + let parentFiberInstance = parentInstance; + while (parentFiberInstance !== null) { + if (parentFiberInstance.kind === FIBER_INSTANCE) { + parentFiber = parentFiberInstance.data; + break; + } + parentFiberInstance = parentFiberInstance.parent; + } + if (parentFiber === null || (parentFiber.mode & StrictModeBits) === 0) { + pushOperation(TREE_OPERATION_SET_SUBTREE_MODE); + pushOperation(id); + pushOperation(StrictMode); + } } } @@ -2118,6 +2160,7 @@ export function attach( recordProfilingDurations(fiber); } + return fiberInstance; } function recordUnmount(fiber: Fiber, isSimulated: boolean) { @@ -2142,8 +2185,8 @@ export function attach( } } - const unsafeID = getFiberIDUnsafe(fiber); - if (unsafeID === null) { + const fiberInstance = getFiberInstanceUnsafe(fiber); + if (fiberInstance === null) { // If we've never seen this Fiber, it might be inside of a legacy render Suspense fragment (so the store is not even aware of it). // In that case we can just ignore it or it will cause errors later on. // One example of this is a Lazy component that never resolves before being unmounted. @@ -2154,8 +2197,10 @@ export function attach( return; } - // Flow refinement. - const id = ((unsafeID: any): number); + // We're about to remove this from its parent. + fiberInstance.parent = null; + + const id = fiberInstance.id; const isRoot = fiber.tag === HostRoot; if (isRoot) { // Roots must be removed only after all children (pending and simulated) have been removed. @@ -2185,7 +2230,7 @@ export function attach( function mountFiberRecursively( firstChild: Fiber, - parentFiber: Fiber | null, + parentInstance: DevToolsInstance | null, traverseSiblings: boolean, traceNearestHostComponentUpdate: boolean, ) { @@ -2194,10 +2239,11 @@ export function attach( let fiber: Fiber | null = firstChild; while (fiber !== null) { // Generate an ID even for filtered Fibers, in case it's needed later (e.g. for Profiling). - getOrGenerateFiberID(fiber); + // TODO: Do we really need to do this eagerly? + getOrGenerateFiberInstance(fiber); if (__DEBUG__) { - debug('mountFiberRecursively()', fiber, parentFiber); + debug('mountFiberRecursively()', fiber, parentInstance); } // If we have the tree selection from previous reload, try to match this Fiber. @@ -2206,9 +2252,9 @@ export function attach( updateTrackedPathStateBeforeMount(fiber); const shouldIncludeInTree = !shouldFilterFiber(fiber); - if (shouldIncludeInTree) { - recordMount(fiber, parentFiber); - } + const newParentInstance = shouldIncludeInTree + ? recordMount(fiber, parentInstance) + : parentInstance; if (traceUpdatesEnabled) { if (traceNearestHostComponentUpdate) { @@ -2241,7 +2287,7 @@ export function attach( if (fallbackChild !== null) { mountFiberRecursively( fallbackChild, - shouldIncludeInTree ? fiber : parentFiber, + newParentInstance, true, traceNearestHostComponentUpdate, ); @@ -2258,7 +2304,7 @@ export function attach( if (primaryChild !== null) { mountFiberRecursively( primaryChild, - shouldIncludeInTree ? fiber : parentFiber, + newParentInstance, true, traceNearestHostComponentUpdate, ); @@ -2268,7 +2314,7 @@ export function attach( if (fiber.child !== null) { mountFiberRecursively( fiber.child, - shouldIncludeInTree ? fiber : parentFiber, + newParentInstance, true, traceNearestHostComponentUpdate, ); @@ -2287,7 +2333,7 @@ export function attach( // when we switch from primary to fallback. function unmountFiberChildrenRecursively(fiber: Fiber) { if (__DEBUG__) { - debug('unmountFiberChildrenRecursively()', fiber); + debug('unmountFiberChildrenRecursively()', fiber, null); } // We might meet a nested Suspense on our way. @@ -2384,9 +2430,12 @@ export function attach( } } - function recordResetChildren(fiber: Fiber, childSet: Fiber) { + function recordResetChildren( + parentInstance: DevToolsInstance, + childSet: Fiber, + ) { if (__DEBUG__) { - debug('recordResetChildren()', childSet, fiber); + debug('recordResetChildren()', childSet, parentInstance); } // The frontend only really cares about the displayName, key, and children. // The first two don't really change, so we are only concerned with the order of children here. @@ -2407,7 +2456,7 @@ export function attach( return; } pushOperation(TREE_OPERATION_REORDER_CHILDREN); - pushOperation(getFiberIDThrows(fiber)); + pushOperation(parentInstance.id); pushOperation(numChildren); for (let i = 0; i < nextChildren.length; i++) { pushOperation(nextChildren[i]); @@ -2450,13 +2499,15 @@ export function attach( function updateFiberRecursively( nextFiber: Fiber, prevFiber: Fiber, - parentFiber: Fiber | null, + parentInstance: DevToolsInstance | null, traceNearestHostComponentUpdate: boolean, ): boolean { - const id = getOrGenerateFiberID(nextFiber); + // TODO: Do we really need to give this an instance eagerly if it's filtered? + const fiberInstance = getOrGenerateFiberInstance(nextFiber); + const id = fiberInstance.id; if (__DEBUG__) { - debug('updateFiberRecursively()', nextFiber, parentFiber); + debug('updateFiberRecursively()', nextFiber, parentInstance); } if (traceUpdatesEnabled) { @@ -2495,6 +2546,9 @@ export function attach( } const shouldIncludeInTree = !shouldFilterFiber(nextFiber); + const newParentInstance = shouldIncludeInTree + ? fiberInstance + : parentInstance; const isSuspense = nextFiber.tag === SuspenseComponent; let shouldResetChildren = false; // The behavior of timed-out Suspense trees is unique. @@ -2526,7 +2580,7 @@ export function attach( if (prevFallbackChildSet == null && nextFallbackChildSet != null) { mountFiberRecursively( nextFallbackChildSet, - shouldIncludeInTree ? nextFiber : parentFiber, + newParentInstance, true, traceNearestHostComponentUpdate, ); @@ -2540,7 +2594,7 @@ export function attach( updateFiberRecursively( nextFallbackChildSet, prevFallbackChildSet, - nextFiber, + newParentInstance, traceNearestHostComponentUpdate, ) ) { @@ -2555,7 +2609,7 @@ export function attach( if (nextPrimaryChildSet !== null) { mountFiberRecursively( nextPrimaryChildSet, - shouldIncludeInTree ? nextFiber : parentFiber, + newParentInstance, true, traceNearestHostComponentUpdate, ); @@ -2575,7 +2629,7 @@ export function attach( if (nextFallbackChildSet != null) { mountFiberRecursively( nextFallbackChildSet, - shouldIncludeInTree ? nextFiber : parentFiber, + newParentInstance, true, traceNearestHostComponentUpdate, ); @@ -2600,7 +2654,7 @@ export function attach( updateFiberRecursively( nextChild, prevChild, - shouldIncludeInTree ? nextFiber : parentFiber, + newParentInstance, traceNearestHostComponentUpdate, ) ) { @@ -2618,7 +2672,7 @@ export function attach( } else { mountFiberRecursively( nextChild, - shouldIncludeInTree ? nextFiber : parentFiber, + newParentInstance, false, traceNearestHostComponentUpdate, ); @@ -2642,7 +2696,7 @@ export function attach( // we should fall back to recursively marking the nearest host descendants for highlight. if (traceNearestHostComponentUpdate) { const hostFibers = findAllCurrentHostFibers( - getFiberIDThrows(nextFiber), + getFiberInstanceThrows(nextFiber), ); hostFibers.forEach(hostFiber => { traceUpdatesForNodes.add(hostFiber.stateNode); @@ -2670,7 +2724,7 @@ export function attach( nextChildSet = nextFiberChild ? nextFiberChild.sibling : null; } if (nextChildSet != null) { - recordResetChildren(nextFiber, nextChildSet); + recordResetChildren(fiberInstance, nextChildSet); } // We've handled the child order change for this Fiber. // Since it's included, there's no need to invalidate parent child order. @@ -2726,7 +2780,7 @@ export function attach( } // If we have not been profiling, then we can just walk the tree and build up its current state as-is. hook.getFiberRoots(rendererID).forEach(root => { - currentRootID = getOrGenerateFiberID(root.current); + currentRootID = getOrGenerateFiberInstance(root.current).id; setRootPseudoKey(currentRootID, root.current); // Handle multi-renderer edge-case where only some v16 renderers support profiling. @@ -2794,7 +2848,7 @@ export function attach( // If we don't do this, we might end up double-deleting Fibers in some cases (like Legacy Suspense). untrackFibers(); - currentRootID = getOrGenerateFiberID(current); + currentRootID = getOrGenerateFiberInstance(current).id; // Before the traversals, remember to start tracking // our path in case we have selection to restore. @@ -2889,19 +2943,11 @@ export function attach( currentRootID = -1; } - function findAllCurrentHostFibers(id: number): $ReadOnlyArray { + function findAllCurrentHostFibers( + fiberInstance: FiberInstance, + ): $ReadOnlyArray { const fibers = []; - const devtoolsInstance = idToDevToolsInstanceMap.get(id); - if (devtoolsInstance === undefined) { - console.warn(`Could not find DevToolsInstance with id "${id}"`); - return fibers; - } - if (devtoolsInstance.kind !== FIBER_INSTANCE) { - // TODO: Handle VirtualInstance. - return fibers; - } - const fiber = - findCurrentFiberUsingSlowPathByFiberInstance(devtoolsInstance); + const fiber = findCurrentFiberUsingSlowPathByFiberInstance(fiberInstance); if (!fiber) { return fibers; } @@ -2950,7 +2996,7 @@ export function attach( return null; } - const hostFibers = findAllCurrentHostFibers(id); + const hostFibers = findAllCurrentHostFibers(devtoolsInstance); return hostFibers.map(hostFiber => hostFiber.stateNode).filter(Boolean); } catch (err) { // The fiber might have unmounted by now. @@ -2983,15 +3029,43 @@ export function attach( function getElementIDForHostInstance( hostInstance: HostInstance, findNearestUnfilteredAncestor: boolean = false, - ) { + ): number | null { let fiber = renderer.findFiberByHostInstance(hostInstance); if (fiber != null) { - if (findNearestUnfilteredAncestor) { - while (fiber !== null && shouldFilterFiber(fiber)) { - fiber = fiber.return; + if (!findNearestUnfilteredAncestor) { + // TODO: Remove this option. It's not used. + return getFiberIDThrows(fiber); + } + while (fiber !== null) { + const fiberInstance = getFiberInstanceUnsafe(fiber); + if (fiberInstance !== null) { + // TODO: Ideally we would not have any filtered FiberInstances which + // would make this logic much simpler. Unfortunately, we sometimes + // eagerly add to the map and some times don't eagerly clean it up. + // TODO: If the fiber is filtered, the FiberInstance wouldn't really + // exist which would mean that we also don't have a way to get to the + // VirtualInstances. + if (!shouldFilterFiber(fiberInstance.data)) { + return fiberInstance.id; + } + // We couldn't use this Fiber but we might have a VirtualInstance + // that is the nearest unfiltered instance. + let parentInstance = fiberInstance.parent; + while (parentInstance !== null) { + if (parentInstance.kind === FIBER_INSTANCE) { + // If we find a parent Fiber, it might not be the nearest parent + // so we break out and continue walking the Fiber tree instead. + break; + } else { + if (!shouldFilterVirtual(parentInstance.data)) { + return parentInstance.id; + } + } + parentInstance = parentInstance.parent; + } } + fiber = fiber.return; } - return getFiberIDThrows(((fiber: any): Fiber)); } return null; } @@ -3341,6 +3415,7 @@ export function attach( let parent = fiber.return; while (parent !== null) { if (isErrorBoundary(parent)) { + // TODO: If this boundary is filtered it won't have an ID. return getFiberIDUnsafe(parent); } parent = parent.return; diff --git a/packages/react-devtools-shared/src/backend/shared/DevToolsComponentStackFrame.js b/packages/react-devtools-shared/src/backend/shared/DevToolsComponentStackFrame.js index ea895f201dde9..840a298869da8 100644 --- a/packages/react-devtools-shared/src/backend/shared/DevToolsComponentStackFrame.js +++ b/packages/react-devtools-shared/src/backend/shared/DevToolsComponentStackFrame.js @@ -30,7 +30,7 @@ export function describeBuiltInComponentFrame(name: string): string { } } let suffix = ''; - if (__IS_CHROME__ || __IS_EDGE__) { + if (__IS_CHROME__ || __IS_EDGE__ || __IS_NATIVE__) { suffix = ' ()'; } else if (__IS_FIREFOX__) { suffix = '@unknown:0:0'; diff --git a/packages/react-devtools-shared/src/bridge.js b/packages/react-devtools-shared/src/bridge.js index f8d418f1005ba..f4da08be6bdbe 100644 --- a/packages/react-devtools-shared/src/bridge.js +++ b/packages/react-devtools-shared/src/bridge.js @@ -19,8 +19,6 @@ import type { } from 'react-devtools-shared/src/backend/types'; import type {StyleAndLayout as StyleAndLayoutPayload} from 'react-devtools-shared/src/backend/NativeStyleEditor/types'; -const BATCH_DURATION = 100; - // This message specifies the version of the DevTools protocol currently supported by the backend, // as well as the earliest NPM version (e.g. "4.13.0") that protocol is supported by on the frontend. // This enables an older frontend to display an upgrade message to users for a newer, unsupported backend. @@ -276,7 +274,7 @@ class Bridge< }> { _isShutdown: boolean = false; _messageQueue: Array = []; - _timeoutID: TimeoutID | null = null; + _scheduledFlush: boolean = false; _wall: Wall; _wallUnlisten: Function | null = null; @@ -324,8 +322,19 @@ class Bridge< // (or we're waiting for our setTimeout-0 to fire), then _timeoutID will // be set, and we'll simply add to the queue and wait for that this._messageQueue.push(event, payload); - if (!this._timeoutID) { - this._timeoutID = setTimeout(this._flush, 0); + if (!this._scheduledFlush) { + this._scheduledFlush = true; + // $FlowFixMe + if (typeof devtoolsJestTestScheduler === 'function') { + // This exists just for our own jest tests. + // They're written in such a way that we can neither mock queueMicrotask + // because then we break React DOM and we can't not mock it because then + // we can't synchronously flush it. So they need to be rewritten. + // $FlowFixMe + devtoolsJestTestScheduler(this._flush); // eslint-disable-line no-undef + } else { + queueMicrotask(this._flush); + } } } @@ -363,34 +372,23 @@ class Bridge< do { this._flush(); } while (this._messageQueue.length); - - // Make sure once again that there is no dangling timer. - if (this._timeoutID !== null) { - clearTimeout(this._timeoutID); - this._timeoutID = null; - } } _flush: () => void = () => { // This method is used after the bridge is marked as destroyed in shutdown sequence, // so we do not bail out if the bridge marked as destroyed. // It is a private method that the bridge ensures is only called at the right times. - - if (this._timeoutID !== null) { - clearTimeout(this._timeoutID); - this._timeoutID = null; - } - - if (this._messageQueue.length) { - for (let i = 0; i < this._messageQueue.length; i += 2) { - this._wall.send(this._messageQueue[i], ...this._messageQueue[i + 1]); + try { + if (this._messageQueue.length) { + for (let i = 0; i < this._messageQueue.length; i += 2) { + this._wall.send(this._messageQueue[i], ...this._messageQueue[i + 1]); + } + this._messageQueue.length = 0; } - this._messageQueue.length = 0; - - // Check again for queued messages in BATCH_DURATION ms. This will keep - // flushing in a loop as long as messages continue to be added. Once no - // more are, the timer expires. - this._timeoutID = setTimeout(this._flush, BATCH_DURATION); + } finally { + // We set this at the end in case new messages are added synchronously above. + // They're already handled so they shouldn't queue more flushes. + this._scheduledFlush = false; } }; diff --git a/packages/react-devtools-shared/src/devtools/views/Components/Components.js b/packages/react-devtools-shared/src/devtools/views/Components/Components.js index 1575ab242c2ce..f0b309d3f1a2a 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/Components.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/Components.js @@ -8,14 +8,7 @@ */ import * as React from 'react'; -import { - Fragment, - Suspense, - useEffect, - useLayoutEffect, - useReducer, - useRef, -} from 'react'; +import {Fragment, useEffect, useLayoutEffect, useReducer, useRef} from 'react'; import Tree from './Tree'; import {OwnersListContextController} from './OwnersListContext'; import portaledContent from '../portaledContent'; @@ -169,11 +162,9 @@ function Components(_: {}) {
- }> - - - - + + +
@@ -186,10 +177,6 @@ function Components(_: {}) { ); } -function Loading() { - return
Loading...
; -} - const LOCAL_STORAGE_KEY = 'React::DevTools::createResizeReducer'; const VERTICAL_MODE_MAX_WIDTH = 600; const MINIMUM_SIZE = 50; diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js index 0e198fc5b7999..f6549fc243e49 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js @@ -66,7 +66,7 @@ export type Props = { export function InspectedElementContextController({ children, }: Props): React.Node { - const {selectedElementID} = useContext(TreeStateContext); + const {inspectedElementID} = useContext(TreeStateContext); const fetchFileWithCaching = useContext(FetchFileWithCachingContext); const bridge = useContext(BridgeContext); const store = useContext(StoreContext); @@ -93,7 +93,9 @@ export function InspectedElementContextController({ }); const element = - selectedElementID !== null ? store.getElementByID(selectedElementID) : null; + inspectedElementID !== null + ? store.getElementByID(inspectedElementID) + : null; const alreadyLoadedHookNames = element != null && hasAlreadyLoadedHookNames(element); diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorBoundary.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorBoundary.js index c070662f05ff5..9a5612ad8c91d 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorBoundary.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorBoundary.js @@ -27,7 +27,7 @@ export default function InspectedElementErrorBoundaryWrapper({ }: WrapperProps): React.Node { // Key on the selected element ID so that changing the selected element automatically hides the boundary. // This seems best since an error inspecting one element isn't likely to be relevant to another element. - const {selectedElementID} = useContext(TreeStateContext); + const {inspectedElementID} = useContext(TreeStateContext); const refresh = useCacheRefresh(); const handleDsmiss = useCallback(() => { @@ -37,7 +37,7 @@ export default function InspectedElementErrorBoundaryWrapper({ return (
{children} diff --git a/packages/react-devtools-shared/src/devtools/views/Components/Tree.js b/packages/react-devtools-shared/src/devtools/views/Components/Tree.js index d7ddc636060d0..136baa1205de2 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/Tree.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/Tree.js @@ -586,7 +586,10 @@ function InnerElementType({children, style}) { // A lot of options were considered; this seemed the one that requires the least code. // See https://github.com/bvaughn/react-devtools-experimental/issues/9 return ( -
+
{children}
diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 28d4cf73aab7b..d70d1fdbb381c 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -14,7 +14,7 @@ import type { IntersectionObserverOptions, ObserveVisibleRectsCallback, } from 'react-reconciler/src/ReactTestSelectors'; -import type {ReactScopeInstance} from 'shared/ReactTypes'; +import type {ReactContext, ReactScopeInstance} from 'shared/ReactTypes'; import type {AncestorInfoDev} from './validateDOMNesting'; import type {FormStatus} from 'react-dom-bindings/src/shared/ReactDOMFormActions'; import type { @@ -32,6 +32,7 @@ import {getCurrentRootHostContainer} from 'react-reconciler/src/ReactFiberHostCo import hasOwnProperty from 'shared/hasOwnProperty'; import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion'; +import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols'; export { setCurrentUpdatePriority, @@ -3564,6 +3565,14 @@ function insertStylesheetIntoRoot( } export const NotPendingTransition: TransitionStatus = NotPending; +export const HostTransitionContext: ReactContext = { + $$typeof: REACT_CONTEXT_TYPE, + Provider: (null: any), + Consumer: (null: any), + _currentValue: NotPendingTransition, + _currentValue2: NotPendingTransition, + _threadCount: 0, +}; export type FormInstance = HTMLFormElement; export function resetFormInstance(form: FormInstance): void { diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzStatic-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzStatic-test.js index 5dcc3c9c3b93d..1f3bfa7b3308d 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzStatic-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzStatic-test.js @@ -398,4 +398,60 @@ describe('ReactDOMFizzStatic', () => {
, ); }); + + // @gate enablePostpone + it('does not fatally error when aborting with a postpone during a prerender', async () => { + let postponedValue; + try { + React.unstable_postpone('aborting with postpone'); + } catch (e) { + postponedValue = e; + } + + const controller = new AbortController(); + const infinitePromise = new Promise(() => {}); + function App() { + React.use(infinitePromise); + return
aborted
; + } + + const pendingResult = ReactDOMFizzStatic.prerenderToNodeStream(, { + signal: controller.signal, + }); + pendingResult.catch(() => {}); + + await Promise.resolve(); + controller.abort(postponedValue); + + const result = await pendingResult; + + await act(async () => { + result.prelude.pipe(writable); + }); + expect(getVisibleChildren(container)).toEqual(undefined); + }); + + // @gate enablePostpone + it('does not fatally error when aborting with a postpone during a prerender from within', async () => { + let postponedValue; + try { + React.unstable_postpone('aborting with postpone'); + } catch (e) { + postponedValue = e; + } + + const controller = new AbortController(); + function App() { + controller.abort(postponedValue); + return
aborted
; + } + + const result = await ReactDOMFizzStatic.prerenderToNodeStream(, { + signal: controller.signal, + }); + await act(async () => { + result.prelude.pipe(writable); + }); + expect(getVisibleChildren(container)).toEqual(undefined); + }); }); diff --git a/packages/react-native-renderer/src/ReactFiberConfigFabric.js b/packages/react-native-renderer/src/ReactFiberConfigFabric.js index 8e04289fb1c0b..80215d11ec14c 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigFabric.js +++ b/packages/react-native-renderer/src/ReactFiberConfigFabric.js @@ -60,6 +60,8 @@ import { enableFabricCompleteRootInCommitPhase, passChildrenWhenCloningPersistedNodes, } from 'shared/ReactFeatureFlags'; +import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols'; +import type {ReactContext} from 'shared/ReactTypes'; export {default as rendererVersion} from 'shared/ReactVersion'; // TODO: Consider exporting the react-native version. export const rendererPackageName = 'react-native-renderer'; @@ -544,6 +546,14 @@ export function waitForCommitToBeReady(): null { } export const NotPendingTransition: TransitionStatus = null; +export const HostTransitionContext: ReactContext = { + $$typeof: REACT_CONTEXT_TYPE, + Provider: (null: any), + Consumer: (null: any), + _currentValue: NotPendingTransition, + _currentValue2: NotPendingTransition, + _threadCount: 0, +}; export type FormInstance = Instance; export function resetFormInstance(form: Instance): void {} diff --git a/packages/react-native-renderer/src/ReactFiberConfigNative.js b/packages/react-native-renderer/src/ReactFiberConfigNative.js index c9c6aadcf919c..0a95e0f818cdb 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigNative.js +++ b/packages/react-native-renderer/src/ReactFiberConfigNative.js @@ -32,6 +32,9 @@ import { } from 'react-reconciler/src/ReactEventPriorities'; import type {Fiber} from 'react-reconciler/src/ReactInternalTypes'; +import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols'; +import type {ReactContext} from 'shared/ReactTypes'; + import { getInspectorDataForViewTag, getInspectorDataForViewAtPoint, @@ -561,6 +564,14 @@ export function waitForCommitToBeReady(): null { } export const NotPendingTransition: TransitionStatus = null; +export const HostTransitionContext: ReactContext = { + $$typeof: REACT_CONTEXT_TYPE, + Provider: (null: any), + Consumer: (null: any), + _currentValue: NotPendingTransition, + _currentValue2: NotPendingTransition, + _threadCount: 0, +}; export type FormInstance = Instance; export function resetFormInstance(form: Instance): void {} diff --git a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js index 99d21ea933ecd..d55be5efd24bf 100644 --- a/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js +++ b/packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js @@ -279,6 +279,130 @@ describe('ReactFabric', () => { expect(nativeFabricUIManager.completeRoot).toBeCalled(); }); + // @gate enablePersistedModeClonedFlag + it('should not clone nodes when layout effects are used', async () => { + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: {foo: true}, + uiViewClassName: 'RCTView', + })); + + const ComponentWithEffect = () => { + React.useLayoutEffect(() => {}); + return null; + }; + + await act(() => + ReactFabric.render( + + + , + 11, + ), + ); + expect(nativeFabricUIManager.completeRoot).toBeCalled(); + jest.clearAllMocks(); + + await act(() => + ReactFabric.render( + + + , + 11, + ), + ); + expect(nativeFabricUIManager.cloneNode).not.toBeCalled(); + expect(nativeFabricUIManager.cloneNodeWithNewChildren).not.toBeCalled(); + expect(nativeFabricUIManager.cloneNodeWithNewProps).not.toBeCalled(); + expect( + nativeFabricUIManager.cloneNodeWithNewChildrenAndProps, + ).not.toBeCalled(); + expect(nativeFabricUIManager.completeRoot).not.toBeCalled(); + }); + + // @gate enablePersistedModeClonedFlag + it('should not clone nodes when insertion effects are used', async () => { + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: {foo: true}, + uiViewClassName: 'RCTView', + })); + + const ComponentWithRef = () => { + React.useInsertionEffect(() => {}); + return null; + }; + + await act(() => + ReactFabric.render( + + + , + 11, + ), + ); + expect(nativeFabricUIManager.completeRoot).toBeCalled(); + jest.clearAllMocks(); + + await act(() => + ReactFabric.render( + + + , + 11, + ), + ); + expect(nativeFabricUIManager.cloneNode).not.toBeCalled(); + expect(nativeFabricUIManager.cloneNodeWithNewChildren).not.toBeCalled(); + expect(nativeFabricUIManager.cloneNodeWithNewProps).not.toBeCalled(); + expect( + nativeFabricUIManager.cloneNodeWithNewChildrenAndProps, + ).not.toBeCalled(); + expect(nativeFabricUIManager.completeRoot).not.toBeCalled(); + }); + + // @gate enablePersistedModeClonedFlag + it('should not clone nodes when useImperativeHandle is used', async () => { + const View = createReactNativeComponentClass('RCTView', () => ({ + validAttributes: {foo: true}, + uiViewClassName: 'RCTView', + })); + + const ComponentWithImperativeHandle = props => { + React.useImperativeHandle(props.ref, () => ({greet: () => 'hello'})); + return null; + }; + + const ref = React.createRef(); + + await act(() => + ReactFabric.render( + + + , + 11, + ), + ); + expect(nativeFabricUIManager.completeRoot).toBeCalled(); + expect(ref.current.greet()).toBe('hello'); + jest.clearAllMocks(); + + await act(() => + ReactFabric.render( + + + , + 11, + ), + ); + expect(nativeFabricUIManager.cloneNode).not.toBeCalled(); + expect(nativeFabricUIManager.cloneNodeWithNewChildren).not.toBeCalled(); + expect(nativeFabricUIManager.cloneNodeWithNewProps).not.toBeCalled(); + expect( + nativeFabricUIManager.cloneNodeWithNewChildrenAndProps, + ).not.toBeCalled(); + expect(nativeFabricUIManager.completeRoot).not.toBeCalled(); + expect(ref.current.greet()).toBe('hello'); + }); + it('should call dispatchCommand for native refs', async () => { const View = createReactNativeComponentClass('RCTView', () => ({ validAttributes: {foo: true}, diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index eae5dc5a964ff..34176bb83b5c9 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -178,6 +178,7 @@ import { isPrimaryRenderer, getResource, createHoistableInstance, + HostTransitionContext, } from './ReactFiberConfig'; import type {SuspenseInstance} from './ReactFiberConfig'; import {shouldError, shouldSuspend} from './ReactFiberReconciler'; @@ -185,7 +186,6 @@ import { pushHostContext, pushHostContainer, getRootHostContainer, - HostTransitionContext, } from './ReactFiberHostContext'; import { suspenseStackCursor, diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 773e5d8f485d9..0f8fa68d0ca79 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -42,6 +42,7 @@ import type { import { alwaysThrottleRetries, enableCreateEventHandleAPI, + enablePersistedModeClonedFlag, enableProfilerTimer, enableProfilerCommitHooks, enableProfilerNestedUpdatePhase, @@ -98,6 +99,7 @@ import { ShouldSuspendCommit, MaySuspendCommit, FormReset, + Cloned, } from './ReactFiberFlags'; import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; import {runWithFiberInDEV} from './ReactCurrentFiber'; @@ -2554,7 +2556,10 @@ function recursivelyTraverseMutationEffects( } } - if (parentFiber.subtreeFlags & MutationMask) { + if ( + parentFiber.subtreeFlags & + (enablePersistedModeClonedFlag ? MutationMask | Cloned : MutationMask) + ) { let child = parentFiber.child; while (child !== null) { if (__DEV__) { diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index dd797b8d09717..c5d0c1575be04 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -35,6 +35,7 @@ import { enableLegacyHidden, enableSuspenseCallback, enableScopeAPI, + enablePersistedModeClonedFlag, enableProfilerTimer, enableCache, enableTransitionTracing, @@ -90,6 +91,7 @@ import { MaySuspendCommit, ScheduleRetry, ShouldSuspendCommit, + Cloned, } from './ReactFiberFlags'; import { @@ -182,6 +184,16 @@ function markUpdate(workInProgress: Fiber) { workInProgress.flags |= Update; } +/** + * Tag the fiber with Cloned in persistent mode to signal that + * it received an update that requires a clone of the tree above. + */ +function markCloned(workInProgress: Fiber) { + if (supportsPersistence && enablePersistedModeClonedFlag) { + workInProgress.flags |= Cloned; + } +} + /** * In persistent mode, return whether this update needs to clone the subtree. */ @@ -199,9 +211,12 @@ function doesRequireClone(current: null | Fiber, completedWork: Fiber) { // then we only have to check the `completedWork.subtreeFlags`. let child = completedWork.child; while (child !== null) { + const checkedFlags = enablePersistedModeClonedFlag + ? Cloned | Visibility | Placement | ChildDeletion + : MutationMask; if ( - (child.flags & MutationMask) !== NoFlags || - (child.subtreeFlags & MutationMask) !== NoFlags + (child.flags & checkedFlags) !== NoFlags || + (child.subtreeFlags & checkedFlags) !== NoFlags ) { return true; } @@ -450,6 +465,7 @@ function updateHostComponent( let newChildSet = null; if (requiresClone && passChildrenWhenCloningPersistedNodes) { + markCloned(workInProgress); newChildSet = createContainerChildSet(); // If children might have changed, we have to add them all to the set. appendAllChildrenToContainer( @@ -473,6 +489,8 @@ function updateHostComponent( // Note that this might release a previous clone. workInProgress.stateNode = currentInstance; return; + } else { + markCloned(workInProgress); } // Certain renderers require commit-time effects for initial mount. @@ -485,12 +503,14 @@ function updateHostComponent( } workInProgress.stateNode = newInstance; if (!requiresClone) { - // If there are no other effects in this tree, we need to flag this node as having one. - // Even though we're not going to use it for anything. - // Otherwise parents won't know that there are new children to propagate upwards. - markUpdate(workInProgress); + if (!enablePersistedModeClonedFlag) { + // If there are no other effects in this tree, we need to flag this node as having one. + // Even though we're not going to use it for anything. + // Otherwise parents won't know that there are new children to propagate upwards. + markUpdate(workInProgress); + } } else if (!passChildrenWhenCloningPersistedNodes) { - // If children might have changed, we have to add them all to the set. + // If children have changed, we have to add them all to the set. appendAllChildren( newInstance, workInProgress, @@ -618,15 +638,18 @@ function updateHostText( // If the text content differs, we'll create a new text instance for it. const rootContainerInstance = getRootHostContainer(); const currentHostContext = getHostContext(); + markCloned(workInProgress); workInProgress.stateNode = createTextInstance( newText, rootContainerInstance, currentHostContext, workInProgress, ); - // We'll have to mark it as having an effect, even though we won't use the effect for anything. - // This lets the parents know that at least one of their children has changed. - markUpdate(workInProgress); + if (!enablePersistedModeClonedFlag) { + // We'll have to mark it as having an effect, even though we won't use the effect for anything. + // This lets the parents know that at least one of their children has changed. + markUpdate(workInProgress); + } } else { workInProgress.stateNode = current.stateNode; } @@ -1229,6 +1252,7 @@ function completeWork( ); // TODO: For persistent renderers, we should pass children as part // of the initial instance creation + markCloned(workInProgress); appendAllChildren(instance, workInProgress, false, false); workInProgress.stateNode = instance; @@ -1284,6 +1308,7 @@ function completeWork( if (wasHydrated) { prepareToHydrateHostTextInstance(workInProgress); } else { + markCloned(workInProgress); workInProgress.stateNode = createTextInstance( newText, rootContainerInstance, diff --git a/packages/react-reconciler/src/ReactFiberFlags.js b/packages/react-reconciler/src/ReactFiberFlags.js index 49aebe2f9b11c..66e9249f15026 100644 --- a/packages/react-reconciler/src/ReactFiberFlags.js +++ b/packages/react-reconciler/src/ReactFiberFlags.js @@ -20,7 +20,7 @@ export const Hydrating = /* */ 0b0000000000000001000000000000 // You can change the rest (and add more). export const Update = /* */ 0b0000000000000000000000000100; -/* Skipped value: 0b0000000000000000000000001000; */ +export const Cloned = /* */ 0b0000000000000000000000001000; export const ChildDeletion = /* */ 0b0000000000000000000000010000; export const ContentReset = /* */ 0b0000000000000000000000100000; diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 78c9c8a9e05ff..363646eb7ddb4 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -28,6 +28,7 @@ import type {Flags} from './ReactFiberFlags'; import type {TransitionStatus} from './ReactFiberConfig'; import { + HostTransitionContext, NotPendingTransition as NoPendingHostTransition, setCurrentUpdatePriority, getCurrentUpdatePriority, @@ -156,7 +157,6 @@ import { peekEntangledActionThenable, chainThenableValue, } from './ReactFiberAsyncAction'; -import {HostTransitionContext} from './ReactFiberHostContext'; import {requestTransitionLane} from './ReactFiberRootScheduler'; import {isCurrentTreeHidden} from './ReactFiberHiddenContext'; import {requestCurrentTransition} from './ReactFiberTransition'; @@ -3276,8 +3276,7 @@ function useHostTransitionStatus(): TransitionStatus { if (!enableAsyncActions) { throw new Error('Not implemented.'); } - const status: TransitionStatus | null = readContext(HostTransitionContext); - return status !== null ? status : NoPendingHostTransition; + return readContext(HostTransitionContext); } function mountId(): string { diff --git a/packages/react-reconciler/src/ReactFiberHostContext.js b/packages/react-reconciler/src/ReactFiberHostContext.js index f930799f56387..a3b525b1a0bf0 100644 --- a/packages/react-reconciler/src/ReactFiberHostContext.js +++ b/packages/react-reconciler/src/ReactFiberHostContext.js @@ -9,21 +9,17 @@ import type {Fiber} from './ReactInternalTypes'; import type {StackCursor} from './ReactFiberStack'; -import type { - Container, - HostContext, - TransitionStatus, -} from './ReactFiberConfig'; +import type {Container, HostContext} from './ReactFiberConfig'; import type {Hook} from './ReactFiberHooks'; -import type {ReactContext} from 'shared/ReactTypes'; import { getChildHostContext, getRootHostContext, + HostTransitionContext, + NotPendingTransition, isPrimaryRenderer, } from './ReactFiberConfig'; import {createCursor, push, pop} from './ReactFiberStack'; -import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols'; import {enableAsyncActions} from 'shared/ReactFeatureFlags'; const contextStackCursor: StackCursor = createCursor(null); @@ -38,21 +34,6 @@ const rootInstanceStackCursor: StackCursor = const hostTransitionProviderCursor: StackCursor = createCursor(null); -// TODO: This should initialize to NotPendingTransition, a constant -// imported from the fiber config. However, because of a cycle in the module -// graph, that value isn't defined during this module's initialization. I can't -// think of a way to work around this without moving that value out of the -// fiber config. For now, the "no provider" case is handled when reading, -// inside useHostTransitionStatus. -export const HostTransitionContext: ReactContext = { - $$typeof: REACT_CONTEXT_TYPE, - Provider: (null: any), - Consumer: (null: any), - _currentValue: null, - _currentValue2: null, - _threadCount: 0, -}; - function requiredContext(c: Value | null): Value { if (__DEV__) { if (c === null) { @@ -150,13 +131,13 @@ function popHostContext(fiber: Fiber): void { pop(hostTransitionProviderCursor, fiber); // When popping the transition provider, we reset the context value back - // to `null`. We can do this because you're not allowd to nest forms. If + // to `NotPendingTransition`. We can do this because you're not allowed to nest forms. If // we allowed for multiple nested host transition providers, then we'd // need to reset this to the parent provider's status. if (isPrimaryRenderer) { - HostTransitionContext._currentValue = null; + HostTransitionContext._currentValue = NotPendingTransition; } else { - HostTransitionContext._currentValue2 = null; + HostTransitionContext._currentValue2 = NotPendingTransition; } } } diff --git a/packages/react-reconciler/src/ReactFiberNewContext.js b/packages/react-reconciler/src/ReactFiberNewContext.js index cc908167e1e4f..57004e499bc28 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.js @@ -20,7 +20,7 @@ import type {SharedQueue} from './ReactFiberClassUpdateQueue'; import type {TransitionStatus} from './ReactFiberConfig'; import type {Hook} from './ReactFiberHooks'; -import {isPrimaryRenderer} from './ReactFiberConfig'; +import {isPrimaryRenderer, HostTransitionContext} from './ReactFiberConfig'; import {createCursor, push, pop} from './ReactFiberStack'; import { ContextProvider, @@ -48,10 +48,7 @@ import { enableAsyncActions, enableRenderableContext, } from 'shared/ReactFeatureFlags'; -import { - getHostTransitionProvider, - HostTransitionContext, -} from './ReactFiberHostContext'; +import {getHostTransitionProvider} from './ReactFiberHostContext'; import isArray from '../../shared/isArray'; import {enableContextProfiling} from '../../shared/ReactFeatureFlags'; diff --git a/packages/react-reconciler/src/__tests__/ReactPersistent-test.js b/packages/react-reconciler/src/__tests__/ReactPersistent-test.js index 7900ccdadd451..dba30d6d267bd 100644 --- a/packages/react-reconciler/src/__tests__/ReactPersistent-test.js +++ b/packages/react-reconciler/src/__tests__/ReactPersistent-test.js @@ -12,6 +12,8 @@ let React; let ReactNoopPersistent; + +let act; let waitForAll; describe('ReactPersistent', () => { @@ -20,8 +22,7 @@ describe('ReactPersistent', () => { React = require('react'); ReactNoopPersistent = require('react-noop-renderer/persistent'); - const InternalTestUtils = require('internal-test-utils'); - waitForAll = InternalTestUtils.waitForAll; + ({act, waitForAll} = require('internal-test-utils')); }); // Inlined from shared folder so we can run this test on a bundle. @@ -213,4 +214,25 @@ describe('ReactPersistent', () => { // The original is unchanged. expect(newPortalChildren).toEqual([div(span(), 'Hello ', 'World')]); }); + + it('remove children', async () => { + function Wrapper({children}) { + return children; + } + + const root = ReactNoopPersistent.createRoot(); + await act(() => { + root.render( + + + , + ); + }); + expect(root.getChildrenAsJSX()).toEqual(); + + await act(() => { + root.render(); + }); + expect(root.getChildrenAsJSX()).toEqual(null); + }); }); diff --git a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js index faafe7159e25a..0826f9d95ee0d 100644 --- a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js @@ -83,6 +83,7 @@ export const startSuspendingCommit = $$$config.startSuspendingCommit; export const suspendInstance = $$$config.suspendInstance; export const waitForCommitToBeReady = $$$config.waitForCommitToBeReady; export const NotPendingTransition = $$$config.NotPendingTransition; +export const HostTransitionContext = $$$config.HostTransitionContext; export const resetFormInstance = $$$config.resetFormInstance; export const bindToConsole = $$$config.bindToConsole; diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index d89de01da5a6f..a6c77f8bafb7f 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -3794,12 +3794,22 @@ function abortTask(task: Task, request: Request, error: mixed): void { error.$$typeof === REACT_POSTPONE_TYPE ) { const postponeInstance: Postpone = (error: any); - const fatal = new Error( - 'The render was aborted with postpone when the shell is incomplete. Reason: ' + - postponeInstance.message, - ); - logRecoverableError(request, fatal, errorInfo, null); - fatalError(request, fatal, errorInfo, null); + const trackedPostpones = request.trackedPostpones; + + if (trackedPostpones !== null && segment !== null) { + // We are prerendering. We don't want to fatal when the shell postpones + // we just need to mark it as postponed. + logPostpone(request, postponeInstance.message, errorInfo, null); + trackPostpone(request, trackedPostpones, task, segment); + finishedTask(request, null, segment); + } else { + const fatal = new Error( + 'The render was aborted with postpone when the shell is incomplete. Reason: ' + + postponeInstance.message, + ); + logRecoverableError(request, fatal, errorInfo, null); + fatalError(request, fatal, errorInfo, null); + } } else { logRecoverableError(request, error, errorInfo, null); fatalError(request, error, errorInfo, null); @@ -4102,7 +4112,7 @@ function retryRenderTask( task.abortSet.delete(task); segment.status = COMPLETED; finishedTask(request, task.blockedBoundary, segment); - } catch (thrownValue) { + } catch (thrownValue: mixed) { resetHooksState(); // Reset the write pointers to where we started. @@ -4117,7 +4127,9 @@ function retryRenderTask( // (unstable) API for suspending. This implementation detail can change // later, once we deprecate the old API in favor of `use`. getSuspendedThenable() - : thrownValue; + : thrownValue === AbortSigil + ? request.fatalError + : thrownValue; if (typeof x === 'object' && x !== null) { // $FlowFixMe[method-unbinding] @@ -4126,7 +4138,8 @@ function retryRenderTask( segment.status = PENDING; task.thenableState = getThenableStateAfterSuspending(); const ping = task.ping; - x.then(ping, ping); + // We've asserted that x is a thenable above + (x: any).then(ping, ping); return; } else if ( enablePostpone && @@ -4156,25 +4169,14 @@ function retryRenderTask( const errorInfo = getThrownInfo(task.componentStack); task.abortSet.delete(task); - if (x === AbortSigil) { - segment.status = ABORTED; - erroredTask( - request, - task.blockedBoundary, - request.fatalError, - errorInfo, - __DEV__ && enableOwnerStacks ? task.debugTask : null, - ); - } else { - segment.status = ERRORED; - erroredTask( - request, - task.blockedBoundary, - x, - errorInfo, - __DEV__ && enableOwnerStacks ? task.debugTask : null, - ); - } + segment.status = ERRORED; + erroredTask( + request, + task.blockedBoundary, + x, + errorInfo, + __DEV__ && enableOwnerStacks ? task.debugTask : null, + ); return; } finally { if (__DEV__) { diff --git a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js index 1bb04e8d32130..9d62e8a19ccf2 100644 --- a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js +++ b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js @@ -7,7 +7,10 @@ * @flow */ +import type {ReactContext} from 'shared/ReactTypes'; + import isArray from 'shared/isArray'; +import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols'; import { DefaultEventPriority, NoEventPriority, @@ -352,6 +355,14 @@ export function waitForCommitToBeReady(): null { } export const NotPendingTransition: TransitionStatus = null; +export const HostTransitionContext: ReactContext = { + $$typeof: REACT_CONTEXT_TYPE, + Provider: (null: any), + Consumer: (null: any), + _currentValue: NotPendingTransition, + _currentValue2: NotPendingTransition, + _threadCount: 0, +}; export type FormInstance = Instance; export function resetFormInstance(form: Instance): void {} diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 190f2b6b70335..f33391b36e5c6 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -134,6 +134,12 @@ export const passChildrenWhenCloningPersistedNodes = false; export const enableServerComponentLogs = __EXPERIMENTAL__; +/** + * Enables a new Fiber flag used in persisted mode to reduce the number + * of cloned host components. + */ +export const enablePersistedModeClonedFlag = false; + export const enableAddPropertiesFastPath = false; export const enableOwnerStacks = __EXPERIMENTAL__; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js index 14009ab6711ad..2426206bc825d 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js @@ -20,6 +20,7 @@ export const alwaysThrottleRetries = __VARIANT__; export const enableAddPropertiesFastPath = __VARIANT__; export const enableObjectFiber = __VARIANT__; +export const enablePersistedModeClonedFlag = __VARIANT__; export const enableShallowPropDiffing = __VARIANT__; export const passChildrenWhenCloningPersistedNodes = __VARIANT__; export const enableFabricCompleteRootInCommitPhase = __VARIANT__; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 2c680f7738eec..110c152b59400 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -23,6 +23,7 @@ export const { enableAddPropertiesFastPath, enableFabricCompleteRootInCommitPhase, enableObjectFiber, + enablePersistedModeClonedFlag, enableShallowPropDiffing, passChildrenWhenCloningPersistedNodes, enableLazyContextPropagation, diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 9f9c3698d3d85..737ecf01e9669 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -58,6 +58,7 @@ export const enableLegacyHidden = false; export const enableNoCloningMemoCache = false; export const enableObjectFiber = false; export const enableOwnerStacks = false; +export const enablePersistedModeClonedFlag = false; export const enablePostpone = false; export const enableReactTestRendererWarning = false; export const enableRefAsProp = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 2a75c9ad20e2d..eb6bef3b29754 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -70,6 +70,7 @@ export const enableAsyncActions = true; export const alwaysThrottleRetries = true; export const passChildrenWhenCloningPersistedNodes = false; +export const enablePersistedModeClonedFlag = false; export const enableUseDeferredValueInitialArg = __EXPERIMENTAL__; export const disableClientCache = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index f20f0e16a17d3..0436964f889d2 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -49,6 +49,7 @@ export const enableLegacyHidden = false; export const enableNoCloningMemoCache = false; export const enableObjectFiber = false; export const enableOwnerStacks = false; +export const enablePersistedModeClonedFlag = false; export const enablePostpone = false; export const enableProfilerCommitHooks = __PROFILE__; export const enableProfilerNestedUpdatePhase = __PROFILE__; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index b77f040c24c23..9ce67044464a8 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -73,6 +73,7 @@ export const enableAsyncActions = true; export const alwaysThrottleRetries = true; export const passChildrenWhenCloningPersistedNodes = false; +export const enablePersistedModeClonedFlag = false; export const enableUseDeferredValueInitialArg = true; export const disableClientCache = true; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 125d3c9c0e767..18fc5c8823d38 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -105,6 +105,8 @@ export const enableFizzExternalRuntime = true; export const passChildrenWhenCloningPersistedNodes = false; +export const enablePersistedModeClonedFlag = false; + export const enableAsyncDebugInfo = false; export const disableClientCache = true; diff --git a/scripts/flow/react-devtools.js b/scripts/flow/react-devtools.js index 2b2d6b38bec48..4e5fe0db1c625 100644 --- a/scripts/flow/react-devtools.js +++ b/scripts/flow/react-devtools.js @@ -15,3 +15,4 @@ declare const __TEST__: boolean; declare const __IS_FIREFOX__: boolean; declare const __IS_CHROME__: boolean; declare const __IS_EDGE__: boolean; +declare const __IS_NATIVE__: boolean; diff --git a/scripts/jest/devtools/setupEnv.js b/scripts/jest/devtools/setupEnv.js index a782bb493ebf0..a797c0951435f 100644 --- a/scripts/jest/devtools/setupEnv.js +++ b/scripts/jest/devtools/setupEnv.js @@ -14,6 +14,7 @@ global.__TEST__ = true; global.__IS_FIREFOX__ = false; global.__IS_CHROME__ = false; global.__IS_EDGE__ = false; +global.__IS_NATIVE__ = false; const ReactVersionTestingAgainst = process.env.REACT_VERSION || ReactVersion;