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 8711b251ea168..1c96e65c71ef5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -452,17 +452,17 @@ function* runWithEnvironment( value: reactiveFunction, }); - promoteUsedTemporaries(reactiveFunction); + pruneUnusedLValues(reactiveFunction); yield log({ kind: 'reactive', - name: 'PromoteUsedTemporaries', + name: 'PruneUnusedLValues', value: reactiveFunction, }); - pruneUnusedLValues(reactiveFunction); + promoteUsedTemporaries(reactiveFunction); yield log({ kind: 'reactive', - name: 'PruneUnusedLValues', + name: 'PromoteUsedTemporaries', value: reactiveFunction, }); diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index 73645a3c953bc..7a83f7e3a0b76 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -67,6 +67,20 @@ export const InstrumentationSchema = z export type ExternalFunction = z.infer; +export const MacroMethodSchema = z.union([ + z.object({type: z.literal('wildcard')}), + z.object({type: z.literal('name'), name: z.string()}), +]); + +// Would like to change this to drop the string option, but breaks compatibility with existing configs +export const MacroSchema = z.union([ + z.string(), + z.tuple([z.string(), z.array(MacroMethodSchema)]), +]); + +export type Macro = z.infer; +export type MacroMethod = z.infer; + const HookSchema = z.object({ /* * The effect of arguments to this hook. Describes whether the hook may or may @@ -133,7 +147,7 @@ const EnvironmentConfigSchema = z.object({ * plugin since it looks specifically for the name of the function being invoked, not * following aliases. */ - customMacros: z.nullable(z.array(z.string())).default(null), + customMacros: z.nullable(z.array(MacroSchema)).default(null), /** * Enable a check that resets the memoization cache when the source code of the file changes. @@ -490,7 +504,19 @@ export function parseConfigPragma(pragma: string): EnvironmentConfig { } if (key === 'customMacros' && val) { - maybeConfig[key] = [val]; + const valSplit = val.split('.'); + if (valSplit.length > 0) { + const props = []; + for (const elt of valSplit.slice(1)) { + if (elt === '*') { + props.push({type: 'wildcard'}); + } else if (elt.length > 0) { + props.push({type: 'name', name: elt}); + } + } + console.log([valSplit[0], props.map(x => x.name ?? '*').join('.')]); + maybeConfig[key] = [[valSplit[0], props]]; + } continue; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts index 978596dc67dfd..bd3e97f23aba7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -1209,7 +1209,7 @@ function codegenInstructionNullable( value = null; } else { lvalue = instr.value.lvalue.pattern; - let hasReasign = false; + let hasReassign = false; let hasDeclaration = false; for (const place of eachPatternOperand(lvalue)) { if ( @@ -1219,10 +1219,10 @@ function codegenInstructionNullable( cx.temp.set(place.identifier.declarationId, null); } const isDeclared = cx.hasDeclared(place.identifier); - hasReasign ||= isDeclared; + hasReassign ||= isDeclared; hasDeclaration ||= !isDeclared; } - if (hasReasign && hasDeclaration) { + if (hasReassign && hasDeclaration) { CompilerError.invariant(false, { reason: 'Encountered a destructuring operation where some identifiers are already declared (reassignments) but others are not (declarations)', @@ -1230,7 +1230,7 @@ function codegenInstructionNullable( loc: instr.loc, suggestions: null, }); - } else if (hasReasign) { + } else if (hasReassign) { kind = InstructionKind.Reassign; } value = codegenPlaceToExpression(cx, instr.value.value); diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts index 1f3f8b1271814..022890c1f25c2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts @@ -13,6 +13,7 @@ import { Place, ReactiveValue, } from '../HIR'; +import {Macro, MacroMethod} from '../HIR/Environment'; import {eachReactiveValueOperand} from './visitors'; /** @@ -43,15 +44,17 @@ import {eachReactiveValueOperand} from './visitors'; export function memoizeFbtAndMacroOperandsInSameScope( fn: HIRFunction, ): Set { - const fbtMacroTags = new Set([ - ...FBT_TAGS, + const fbtMacroTags = new Set([ + ...Array.from(FBT_TAGS).map((tag): Macro => [tag, []]), ...(fn.env.config.customMacros ?? []), ]); const fbtValues: Set = new Set(); + const macroMethods = new Map>>(); while (true) { - let size = fbtValues.size; - visit(fn, fbtMacroTags, fbtValues); - if (size === fbtValues.size) { + let vsize = fbtValues.size; + let msize = macroMethods.size; + visit(fn, fbtMacroTags, fbtValues, macroMethods); + if (vsize === fbtValues.size && msize === macroMethods.size) { break; } } @@ -71,8 +74,9 @@ export const SINGLE_CHILD_FBT_TAGS: Set = new Set([ function visit( fn: HIRFunction, - fbtMacroTags: Set, + fbtMacroTags: Set, fbtValues: Set, + macroMethods: Map>>, ): void { for (const [, block] of fn.body.blocks) { for (const instruction of block.instructions) { @@ -83,7 +87,7 @@ function visit( if ( value.kind === 'Primitive' && typeof value.value === 'string' && - fbtMacroTags.has(value.value) + matchesExactTag(value.value, fbtMacroTags) ) { /* * We don't distinguish between tag names and strings, so record @@ -92,10 +96,38 @@ function visit( fbtValues.add(lvalue.identifier.id); } else if ( value.kind === 'LoadGlobal' && - fbtMacroTags.has(value.binding.name) + matchesExactTag(value.binding.name, fbtMacroTags) ) { // Record references to `fbt` as a global fbtValues.add(lvalue.identifier.id); + } else if ( + value.kind === 'LoadGlobal' && + matchTagRoot(value.binding.name, fbtMacroTags) !== null + ) { + const methods = matchTagRoot(value.binding.name, fbtMacroTags)!; + macroMethods.set(lvalue.identifier.id, methods); + } else if ( + value.kind === 'PropertyLoad' && + macroMethods.has(value.object.identifier.id) + ) { + const methods = macroMethods.get(value.object.identifier.id)!; + const newMethods = []; + for (const method of methods) { + if ( + method.length > 0 && + (method[0].type === 'wildcard' || + (method[0].type === 'name' && method[0].name === value.property)) + ) { + if (method.length > 1) { + newMethods.push(method.slice(1)); + } else { + fbtValues.add(lvalue.identifier.id); + } + } + } + if (newMethods.length > 0) { + macroMethods.set(lvalue.identifier.id, newMethods); + } } else if (isFbtCallExpression(fbtValues, value)) { const fbtScope = lvalue.identifier.scope; if (fbtScope === null) { @@ -167,17 +199,48 @@ function visit( } } +function matchesExactTag(s: string, tags: Set): boolean { + return Array.from(tags).some(macro => + typeof macro === 'string' + ? s === macro + : macro[1].length === 0 && macro[0] === s, + ); +} + +function matchTagRoot( + s: string, + tags: Set, +): Array> | null { + const methods: Array> = []; + for (const macro of tags) { + if (typeof macro === 'string') { + continue; + } + const [tag, rest] = macro; + if (tag === s && rest.length > 0) { + methods.push(rest); + } + } + if (methods.length > 0) { + return methods; + } else { + return null; + } +} + function isFbtCallExpression( fbtValues: Set, value: ReactiveValue, ): boolean { return ( - value.kind === 'CallExpression' && fbtValues.has(value.callee.identifier.id) + (value.kind === 'CallExpression' && + fbtValues.has(value.callee.identifier.id)) || + (value.kind === 'MethodCall' && fbtValues.has(value.property.identifier.id)) ); } function isFbtJsxExpression( - fbtMacroTags: Set, + fbtMacroTags: Set, fbtValues: Set, value: ReactiveValue, ): boolean { @@ -185,7 +248,8 @@ function isFbtJsxExpression( value.kind === 'JsxExpression' && ((value.tag.kind === 'Identifier' && fbtValues.has(value.tag.identifier.id)) || - (value.tag.kind === 'BuiltinTag' && fbtMacroTags.has(value.tag.name))) + (value.tag.kind === 'BuiltinTag' && + matchesExactTag(value.tag.name, fbtMacroTags))) ); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts index d897b924ce68c..12cbde01b20ff 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts @@ -15,13 +15,17 @@ import { PrunedReactiveScopeBlock, ReactiveFunction, ReactiveScope, + ReactiveInstruction, ReactiveScopeBlock, ReactiveValue, ScopeId, + SpreadPattern, promoteTemporary, promoteTemporaryJsxTag, + IdentifierId, } from '../HIR/HIR'; import {ReactiveFunctionVisitor, visitReactiveFunction} from './visitors'; +import {eachInstructionValueLValue, eachPatternOperand} from '../HIR/visitors'; /** * Phase 2: Promote identifiers which are used in a place that requires a named variable. @@ -225,6 +229,199 @@ class CollectPromotableTemporaries extends ReactiveFunctionVisitor { } } +type InterState = Map; +class PromoteInterposedTemporaries extends ReactiveFunctionVisitor { + #promotable: State; + #consts: Set = new Set(); + #globals: Set = new Set(); + + /* + * Unpromoted temporaries will be emitted at their use sites rather than as separate + * declarations. However, this causes errors if an interposing temporary has been + * promoted, or if an interposing instruction has had its lvalues deleted, because such + * temporaries will be emitted as separate statements, which can effectively cause + * code to be reordered, and when that code has side effects that changes program behavior. + * This visitor promotes temporarties that have such interposing instructions to preserve + * source ordering. + */ + constructor(promotable: State, params: Array) { + super(); + params.forEach(param => { + switch (param.kind) { + case 'Identifier': + this.#consts.add(param.identifier.id); + break; + case 'Spread': + this.#consts.add(param.place.identifier.id); + break; + } + }); + this.#promotable = promotable; + } + + override visitPlace( + _id: InstructionId, + place: Place, + state: InterState, + ): void { + const promo = state.get(place.identifier.id); + if (promo) { + const [identifier, needsPromotion] = promo; + if ( + needsPromotion && + identifier.name === null && + !this.#consts.has(identifier.id) + ) { + /* + * If the identifier hasn't been promoted but is marked as needing + * promotion by the logic in `visitInstruction`, and we've seen a + * use of it after said marking, promote it + */ + promoteIdentifier(identifier, this.#promotable); + } + } + } + + override visitInstruction( + instruction: ReactiveInstruction, + state: InterState, + ): void { + for (const lval of eachInstructionValueLValue(instruction.value)) { + CompilerError.invariant(lval.identifier.name != null, { + reason: + 'PromoteInterposedTemporaries: Assignment targets not expected to be temporaries', + loc: instruction.loc, + }); + } + + switch (instruction.value.kind) { + case 'CallExpression': + case 'MethodCall': + case 'Await': + case 'PropertyStore': + case 'PropertyDelete': + case 'ComputedStore': + case 'ComputedDelete': + case 'PostfixUpdate': + case 'PrefixUpdate': + case 'StoreLocal': + case 'StoreContext': + case 'StoreGlobal': + case 'Destructure': { + let constStore = false; + + if ( + (instruction.value.kind === 'StoreContext' || + instruction.value.kind === 'StoreLocal') && + (instruction.value.lvalue.kind === 'Const' || + instruction.value.lvalue.kind === 'HoistedConst') + ) { + /* + * If an identifier is const, we don't need to worry about it + * being mutated between being loaded and being used + */ + this.#consts.add(instruction.value.lvalue.place.identifier.id); + constStore = true; + } + if ( + instruction.value.kind === 'Destructure' && + (instruction.value.lvalue.kind === 'Const' || + instruction.value.lvalue.kind === 'HoistedConst') + ) { + [...eachPatternOperand(instruction.value.lvalue.pattern)].forEach( + ident => this.#consts.add(ident.identifier.id), + ); + constStore = true; + } + if (instruction.value.kind === 'MethodCall') { + // Treat property of method call as constlike so we don't promote it. + this.#consts.add(instruction.value.property.identifier.id); + } + + super.visitInstruction(instruction, state); + if ( + !constStore && + (instruction.lvalue == null || + instruction.lvalue.identifier.name != null) + ) { + /* + * If we've stripped the lvalue or promoted the lvalue, then we will emit this instruction + * as a statement in codegen. + * + * If this instruction will be emitted directly as a statement rather than as a temporary + * during codegen, then it can interpose between the defs and the uses of other temporaries. + * Since this instruction could potentially mutate those defs, it's not safe to relocate + * the definition of those temporaries to after this instruction. Mark all those temporaries + * as needing promotion, but don't promote them until we actually see them being used. + */ + for (const [key, [ident, _]] of state.entries()) { + state.set(key, [ident, true]); + } + } + if (instruction.lvalue && instruction.lvalue.identifier.name === null) { + // Add this instruction's lvalue to the state, initially not marked as needing promotion + state.set(instruction.lvalue.identifier.id, [ + instruction.lvalue.identifier, + false, + ]); + } + break; + } + case 'DeclareContext': + case 'DeclareLocal': { + if ( + instruction.value.lvalue.kind === 'Const' || + instruction.value.lvalue.kind === 'HoistedConst' + ) { + this.#consts.add(instruction.value.lvalue.place.identifier.id); + } + super.visitInstruction(instruction, state); + break; + } + case 'LoadContext': + case 'LoadLocal': { + if (instruction.lvalue && instruction.lvalue.identifier.name === null) { + if (this.#consts.has(instruction.value.place.identifier.id)) { + this.#consts.add(instruction.lvalue.identifier.id); + } + state.set(instruction.lvalue.identifier.id, [ + instruction.lvalue.identifier, + false, + ]); + } + super.visitInstruction(instruction, state); + break; + } + case 'PropertyLoad': + case 'ComputedLoad': { + if (instruction.lvalue) { + if (this.#globals.has(instruction.value.object.identifier.id)) { + this.#globals.add(instruction.lvalue.identifier.id); + this.#consts.add(instruction.lvalue.identifier.id); + } + if (instruction.lvalue.identifier.name === null) { + state.set(instruction.lvalue.identifier.id, [ + instruction.lvalue.identifier, + false, + ]); + } + } + super.visitInstruction(instruction, state); + break; + } + case 'LoadGlobal': { + instruction.lvalue && + this.#globals.add(instruction.lvalue.identifier.id); + super.visitInstruction(instruction, state); + break; + } + default: { + super.visitInstruction(instruction, state); + } + } + } +} + export function promoteUsedTemporaries(fn: ReactiveFunction): void { const state: State = { tags: new Set(), @@ -239,6 +436,12 @@ export function promoteUsedTemporaries(fn: ReactiveFunction): void { } } visitReactiveFunction(fn, new PromoteTemporaries(), state); + + visitReactiveFunction( + fn, + new PromoteInterposedTemporaries(state, fn.params), + new Map(), + ); visitReactiveFunction( fn, new PromoteAllInstancedOfPromotedTemporaries(), diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-codegen-inline-iife.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-codegen-inline-iife.expect.md deleted file mode 100644 index 4d36117ec7281..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-codegen-inline-iife.expect.md +++ /dev/null @@ -1,92 +0,0 @@ - -## Input - -```javascript -import {makeArray, print} from 'shared-runtime'; - -/** - * Exposes bug involving iife inlining + codegen. - * We currently inline iifes to labeled blocks (not value-blocks). - * - * Here, print(1) and the evaluation of makeArray(...) get the same scope - * as the compiler infers that the makeArray call may mutate its arguments. - * Since print(1) does not get its own scope (and is thus not a declaration - * or dependency), it does not get promoted. - * As a result, print(1) gets reordered across the labeled-block instructions - * to be inlined at the makeArray callsite. - * - * Current evaluator results: - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) [null,2] - * logs: [1,2] - * Forget: - * (kind: ok) [null,2] - * logs: [2,1] - */ -function useTest() { - return makeArray( - print(1), - (function foo() { - print(2); - return 2; - })(), - ); -} - -export const FIXTURE_ENTRYPOINT = { - fn: useTest, - params: [], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; -import { makeArray, print } from "shared-runtime"; - -/** - * Exposes bug involving iife inlining + codegen. - * We currently inline iifes to labeled blocks (not value-blocks). - * - * Here, print(1) and the evaluation of makeArray(...) get the same scope - * as the compiler infers that the makeArray call may mutate its arguments. - * Since print(1) does not get its own scope (and is thus not a declaration - * or dependency), it does not get promoted. - * As a result, print(1) gets reordered across the labeled-block instructions - * to be inlined at the makeArray callsite. - * - * Current evaluator results: - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) [null,2] - * logs: [1,2] - * Forget: - * (kind: ok) [null,2] - * logs: [2,1] - */ -function useTest() { - const $ = _c(1); - let t0; - if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - let t1; - - print(2); - t1 = 2; - t0 = makeArray(print(1), t1); - $[0] = t0; - } else { - t0 = $[0]; - } - return t0; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useTest, - params: [], -}; - -``` - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-codegen-inline-iife.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-codegen-inline-iife.ts deleted file mode 100644 index 0aa4a95bd5241..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-codegen-inline-iife.ts +++ /dev/null @@ -1,36 +0,0 @@ -import {makeArray, print} from 'shared-runtime'; - -/** - * Exposes bug involving iife inlining + codegen. - * We currently inline iifes to labeled blocks (not value-blocks). - * - * Here, print(1) and the evaluation of makeArray(...) get the same scope - * as the compiler infers that the makeArray call may mutate its arguments. - * Since print(1) does not get its own scope (and is thus not a declaration - * or dependency), it does not get promoted. - * As a result, print(1) gets reordered across the labeled-block instructions - * to be inlined at the makeArray callsite. - * - * Current evaluator results: - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) [null,2] - * logs: [1,2] - * Forget: - * (kind: ok) [null,2] - * logs: [2,1] - */ -function useTest() { - return makeArray( - print(1), - (function foo() { - print(2); - return 2; - })(), - ); -} - -export const FIXTURE_ENTRYPOINT = { - fn: useTest, - params: [], -}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife-reassign.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife-reassign.expect.md new file mode 100644 index 0000000000000..d17c934b3b42c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife-reassign.expect.md @@ -0,0 +1,62 @@ + +## Input + +```javascript +import {makeArray, print} from 'shared-runtime'; + +function useTest() { + let w = {}; + return makeArray( + (w = 42), + w, + (function foo() { + w = 999; + return 2; + })(), + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: useTest, + params: [], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { makeArray, print } from "shared-runtime"; + +function useTest() { + const $ = _c(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + let w; + w = {}; + + const t1 = (w = 42); + const t2 = w; + + w; + let t3; + w = 999; + t3 = 2; + t0 = makeArray(t1, t2, t3); + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useTest, + params: [], +}; + +``` + +### Eval output +(kind: ok) [42,42,2] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife-reassign.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife-reassign.ts new file mode 100644 index 0000000000000..8dd93a8482b92 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife-reassign.ts @@ -0,0 +1,18 @@ +import {makeArray, print} from 'shared-runtime'; + +function useTest() { + let w = {}; + return makeArray( + (w = 42), + w, + (function foo() { + w = 999; + return 2; + })(), + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: useTest, + params: [], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife-storeprop.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife-storeprop.expect.md new file mode 100644 index 0000000000000..58c54ddaab891 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife-storeprop.expect.md @@ -0,0 +1,60 @@ + +## Input + +```javascript +import {makeArray, print} from 'shared-runtime'; + +function useTest() { + let w = {}; + return makeArray( + (w.x = 42), + w.x, + (function foo() { + w.x = 999; + return 2; + })(), + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: useTest, + params: [], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { makeArray, print } from "shared-runtime"; + +function useTest() { + const $ = _c(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + const w = {}; + + const t1 = (w.x = 42); + const t2 = w.x; + let t3; + + w.x = 999; + t3 = 2; + t0 = makeArray(t1, t2, t3); + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useTest, + params: [], +}; + +``` + +### Eval output +(kind: ok) [42,42,2] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife-storeprop.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife-storeprop.ts new file mode 100644 index 0000000000000..1b9f10856e48f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife-storeprop.ts @@ -0,0 +1,18 @@ +import {makeArray, print} from 'shared-runtime'; + +function useTest() { + let w = {}; + return makeArray( + (w.x = 42), + w.x, + (function foo() { + w.x = 999; + return 2; + })(), + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: useTest, + params: [], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife.expect.md new file mode 100644 index 0000000000000..25a08bc3329cb --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife.expect.md @@ -0,0 +1,56 @@ + +## Input + +```javascript +import {makeArray, print} from 'shared-runtime'; + +function useTest() { + return makeArray( + print(1), + (function foo() { + print(2); + return 2; + })(), + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: useTest, + params: [], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { makeArray, print } from "shared-runtime"; + +function useTest() { + const $ = _c(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + const t1 = print(1); + let t2; + + print(2); + t2 = 2; + t0 = makeArray(t1, t2); + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useTest, + params: [], +}; + +``` + +### Eval output +(kind: ok) [null,2] +logs: [1,2] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife.ts new file mode 100644 index 0000000000000..52b1e8a9278e1 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/codegen-inline-iife.ts @@ -0,0 +1,16 @@ +import {makeArray, print} from 'shared-runtime'; + +function useTest() { + return makeArray( + print(1), + (function foo() { + print(2); + return 2; + })(), + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: useTest, + params: [], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/component-declaration-basic.flow.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/component-declaration-basic.flow.expect.md index 42dc6ad88a367..12184dafbbecc 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/component-declaration-basic.flow.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/component-declaration-basic.flow.expect.md @@ -7,8 +7,17 @@ export default component Foo(bar: number) { return ; } +component Bar(bar: number) { + return
{bar}
; +} + function shouldNotCompile() {} +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{bar: 42}], +}; + ``` ## Code @@ -29,7 +38,28 @@ export default function Foo(t0) { return t1; } +function Bar(t0) { + const $ = _c(2); + const { bar } = t0; + let t1; + if ($[0] !== bar) { + t1 =
{bar}
; + $[0] = bar; + $[1] = t1; + } else { + t1 = $[1]; + } + return t1; +} + function shouldNotCompile() {} +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{ bar: 42 }], +}; + ``` - \ No newline at end of file + +### Eval output +(kind: ok)
42
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/component-declaration-basic.flow.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/component-declaration-basic.flow.js index ed69217d58b5d..d29777d4f08d6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/component-declaration-basic.flow.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/component-declaration-basic.flow.js @@ -3,4 +3,13 @@ export default component Foo(bar: number) { return ; } +component Bar(bar: number) { + return
{bar}
; +} + function shouldNotCompile() {} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{bar: 42}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hook-declaration-basic.flow.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hook-declaration-basic.flow.expect.md index dcaa443f26ddb..f75514e8a87cc 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hook-declaration-basic.flow.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hook-declaration-basic.flow.expect.md @@ -7,6 +7,11 @@ export default hook useFoo(bar: number) { return [bar]; } +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [42], +}; + ``` ## Code @@ -26,5 +31,12 @@ export default function useFoo(bar) { return t0; } +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [42], +}; + ``` - \ No newline at end of file + +### Eval output +(kind: ok) [42] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hook-declaration-basic.flow.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hook-declaration-basic.flow.js index b726e1218d3be..217fbe16365d9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hook-declaration-basic.flow.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/hook-declaration-basic.flow.js @@ -2,3 +2,8 @@ export default hook useFoo(bar: number) { return [bar]; } + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [42], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining-wildcard.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining-wildcard.expect.md new file mode 100644 index 0000000000000..dbc90978d5ee0 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining-wildcard.expect.md @@ -0,0 +1,122 @@ + +## Input + +```javascript +// @customMacros(idx.*.b) + +function Component(props) { + // outlined + const groupName1 = idx(props, _ => _.group.label); + // outlined + const groupName2 = idx.a(props, _ => _.group.label); + // not outlined + const groupName3 = idx.a.b(props, _ => _.group.label); + // not outlined + const groupName4 = idx.hello_world.b(props, _ => _.group.label); + // outlined + const groupName5 = idx.hello_world.b.c(props, _ => _.group.label); + return ( +
+ {groupName1} + {groupName2} + {groupName3} + {groupName4} + {groupName5} +
+ ); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @customMacros(idx.*.b) + +function Component(props) { + const $ = _c(16); + let t0; + if ($[0] !== props) { + t0 = idx(props, _temp); + $[0] = props; + $[1] = t0; + } else { + t0 = $[1]; + } + const groupName1 = t0; + let t1; + if ($[2] !== props) { + t1 = idx.a(props, _temp2); + $[2] = props; + $[3] = t1; + } else { + t1 = $[3]; + } + const groupName2 = t1; + let t2; + if ($[4] !== props) { + t2 = idx.a.b(props, (__1) => __1.group.label); + $[4] = props; + $[5] = t2; + } else { + t2 = $[5]; + } + const groupName3 = t2; + let t3; + if ($[6] !== props) { + t3 = idx.hello_world.b(props, (__2) => __2.group.label); + $[6] = props; + $[7] = t3; + } else { + t3 = $[7]; + } + const groupName4 = t3; + let t4; + if ($[8] !== props) { + t4 = idx.hello_world.b.c(props, _temp3); + $[8] = props; + $[9] = t4; + } else { + t4 = $[9]; + } + const groupName5 = t4; + let t5; + if ( + $[10] !== groupName1 || + $[11] !== groupName2 || + $[12] !== groupName3 || + $[13] !== groupName4 || + $[14] !== groupName5 + ) { + t5 = ( +
+ {groupName1} + {groupName2} + {groupName3} + {groupName4} + {groupName5} +
+ ); + $[10] = groupName1; + $[11] = groupName2; + $[12] = groupName3; + $[13] = groupName4; + $[14] = groupName5; + $[15] = t5; + } else { + t5 = $[15]; + } + return t5; +} +function _temp3(__3) { + return __3.group.label; +} +function _temp2(__0) { + return __0.group.label; +} +function _temp(_) { + return _.group.label; +} + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining-wildcard.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining-wildcard.js new file mode 100644 index 0000000000000..4b944ddccac5a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining-wildcard.js @@ -0,0 +1,23 @@ +// @customMacros(idx.*.b) + +function Component(props) { + // outlined + const groupName1 = idx(props, _ => _.group.label); + // outlined + const groupName2 = idx.a(props, _ => _.group.label); + // not outlined + const groupName3 = idx.a.b(props, _ => _.group.label); + // not outlined + const groupName4 = idx.hello_world.b(props, _ => _.group.label); + // outlined + const groupName5 = idx.hello_world.b.c(props, _ => _.group.label); + return ( +
+ {groupName1} + {groupName2} + {groupName3} + {groupName4} + {groupName5} +
+ ); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining.expect.md new file mode 100644 index 0000000000000..f1bced727b718 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining.expect.md @@ -0,0 +1,85 @@ + +## Input + +```javascript +// @customMacros(idx.a) + +function Component(props) { + // outlined + const groupName1 = idx(props, _ => _.group.label); + // not outlined + const groupName2 = idx.a(props, _ => _.group.label); + // outlined + const groupName3 = idx.a.b(props, _ => _.group.label); + return ( +
+ {groupName1} + {groupName2} + {groupName3} +
+ ); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @customMacros(idx.a) + +function Component(props) { + const $ = _c(10); + let t0; + if ($[0] !== props) { + t0 = idx(props, _temp); + $[0] = props; + $[1] = t0; + } else { + t0 = $[1]; + } + const groupName1 = t0; + let t1; + if ($[2] !== props) { + t1 = idx.a(props, (__0) => __0.group.label); + $[2] = props; + $[3] = t1; + } else { + t1 = $[3]; + } + const groupName2 = t1; + let t2; + if ($[4] !== props) { + t2 = idx.a.b(props, _temp2); + $[4] = props; + $[5] = t2; + } else { + t2 = $[5]; + } + const groupName3 = t2; + let t3; + if ($[6] !== groupName1 || $[7] !== groupName2 || $[8] !== groupName3) { + t3 = ( +
+ {groupName1} + {groupName2} + {groupName3} +
+ ); + $[6] = groupName1; + $[7] = groupName2; + $[8] = groupName3; + $[9] = t3; + } else { + t3 = $[9]; + } + return t3; +} +function _temp2(__1) { + return __1.group.label; +} +function _temp(_) { + return _.group.label; +} + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining.js new file mode 100644 index 0000000000000..f5b034b91d67f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/idx-method-no-outlining.js @@ -0,0 +1,17 @@ +// @customMacros(idx.a) + +function Component(props) { + // outlined + const groupName1 = idx(props, _ => _.group.label); + // not outlined + const groupName2 = idx.a(props, _ => _.group.label); + // outlined + const groupName3 = idx.a.b(props, _ => _.group.label); + return ( +
+ {groupName1} + {groupName2} + {groupName3} +
+ ); +} diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index c13cc007ea05f..2d875430263c2 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -390,11 +390,12 @@ const skipFilter = new Set([ 'template-literal', 'multi-arrow-expr-export-default-gating-test', - // TODO: we should be able to support these - 'component-declaration-basic.flow', - 'hook-declaration-basic.flow', + // works, but appears differently when printing + // due to optional function argument 'nested-function-with-param-as-captured-dep', 'deeply-nested-function-expressions-with-params', + + // TODO: we should be able to support these 'readonly-object-method-calls', 'readonly-object-method-calls-mutable-lambda', 'preserve-memo-validation/useMemo-with-refs.flow', @@ -483,7 +484,6 @@ const skipFilter = new Set([ 'rules-of-hooks/rules-of-hooks-69521d94fa03', // bugs - 'bug-renaming-jsx-tag-lowercase', 'fbt/bug-fbt-plural-multiple-function-calls', 'fbt/bug-fbt-plural-multiple-mixed-call-tag', 'bug-invalid-hoisting-functionexpr', @@ -493,13 +493,16 @@ const skipFilter = new Set([ // 'react-compiler-runtime' not yet supported 'flag-enable-emit-hook-guards', - 'fast-refresh-refresh-on-const-changes-dev', 'useState-pruned-dependency-change-detect', 'useState-unpruned-dependency', 'useState-and-other-hook-unpruned-dependency', 'change-detect-reassign', + // Depends on external functions + 'idx-method-no-outlining-wildcard', + 'idx-method-no-outlining', + // needs to be executed as a module 'meta-property', diff --git a/compiler/packages/snap/src/compiler.ts b/compiler/packages/snap/src/compiler.ts index a9e705ea3b9cf..de0184f0bddb1 100644 --- a/compiler/packages/snap/src/compiler.ts +++ b/compiler/packages/snap/src/compiler.ts @@ -20,7 +20,11 @@ import type { PluginOptions, } from 'babel-plugin-react-compiler/src/Entrypoint'; import type {Effect, ValueKind} from 'babel-plugin-react-compiler/src/HIR'; -import type {parseConfigPragma as ParseConfigPragma} from 'babel-plugin-react-compiler/src/HIR/Environment'; +import type { + Macro, + MacroMethod, + parseConfigPragma as ParseConfigPragma, +} from 'babel-plugin-react-compiler/src/HIR/Environment'; import * as HermesParser from 'hermes-parser'; import invariant from 'invariant'; import path from 'path'; @@ -46,7 +50,7 @@ function makePluginOptions( // TODO(@mofeiZ) rewrite snap fixtures to @validatePreserveExistingMemo:false let validatePreserveExistingMemoizationGuarantees = false; let enableChangeDetectionForDebugging = null; - let customMacros = null; + let customMacros: null | Array = null; let validateBlocklistedImports = null; if (firstLine.indexOf('@compilationMode(annotation)') !== -1) { @@ -150,10 +154,27 @@ function makePluginOptions( customMacrosMatch.length > 1 && customMacrosMatch[1].trim().length > 0 ) { - customMacros = customMacrosMatch[1] + const customMacrosStrs = customMacrosMatch[1] .split(' ') .map(s => s.trim()) .filter(s => s.length > 0); + if (customMacrosStrs.length > 0) { + customMacros = []; + for (const customMacroStr of customMacrosStrs) { + const props: Array = []; + const customMacroSplit = customMacroStr.split('.'); + if (customMacroSplit.length > 0) { + for (const elt of customMacroSplit.slice(1)) { + if (elt === '*') { + props.push({type: 'wildcard'}); + } else if (elt.length > 0) { + props.push({type: 'name', name: elt}); + } + } + customMacros.push([customMacroSplit[0], props]); + } + } + } } const validateBlocklistedImportsMatch = diff --git a/compiler/scripts/release/publish.js b/compiler/scripts/release/publish.js index 3262cbabd1509..928994e1e4892 100755 --- a/compiler/scripts/release/publish.js +++ b/compiler/scripts/release/publish.js @@ -162,7 +162,9 @@ async function main() { 'publish', ...opts, '--registry=https://registry.npmjs.org', - '--tag=experimental', + // For now, since the compiler is experimental only, to simplify installation we push + // to the `latest` tag + '--tag=latest', ], { cwd: pkgDir, diff --git a/packages/react-devtools-shared/package.json b/packages/react-devtools-shared/package.json index e503f017fe71e..94f3f9411b0db 100644 --- a/packages/react-devtools-shared/package.json +++ b/packages/react-devtools-shared/package.json @@ -22,8 +22,6 @@ "jsc-safe-url": "^0.2.4", "json5": "^2.1.3", "local-storage-fallback": "^4.1.1", - "lodash.throttle": "^4.1.1", - "memoize-one": "^3.1.1", "react-virtualized-auto-sizer": "^1.0.23", "react-window": "^1.8.10" } diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js index ee6c4431b9526..a4cc492c83efc 100644 --- a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js +++ b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js @@ -34,6 +34,8 @@ describe('InspectedElement', () => { let SettingsContextController; let StoreContext; let TreeContextController; + let TreeStateContext; + let TreeDispatcherContext; let TestUtilsAct; let TestRendererAct; @@ -73,6 +75,10 @@ describe('InspectedElement', () => { require('react-devtools-shared/src/devtools/views/context').StoreContext; TreeContextController = require('react-devtools-shared/src/devtools/views/Components/TreeContext').TreeContextController; + TreeStateContext = + require('react-devtools-shared/src/devtools/views/Components/TreeContext').TreeStateContext; + TreeDispatcherContext = + require('react-devtools-shared/src/devtools/views/Components/TreeContext').TreeDispatcherContext; // Used by inspectElementAtIndex() helper function utils.act(() => { @@ -2142,7 +2148,7 @@ describe('InspectedElement', () => { "context": null, "events": undefined, "hooks": null, - "id": 2, + "id": 4, "owners": null, "props": {}, "rootType": "createRoot()", @@ -2893,7 +2899,7 @@ describe('InspectedElement', () => { "compiledWithForget": false, "displayName": "Child", "hocDisplayNames": null, - "id": 5, + "id": 8, "key": null, "type": 5, }, @@ -2901,7 +2907,7 @@ describe('InspectedElement', () => { "compiledWithForget": false, "displayName": "App", "hocDisplayNames": null, - "id": 4, + "id": 7, "key": null, "type": 5, }, @@ -3016,4 +3022,132 @@ describe('InspectedElement', () => { ); }); }); + + it('should properly handle when components filters are updated', async () => { + const Wrapper = ({children}) => children; + + let state; + let dispatch; + const Capture = () => { + dispatch = React.useContext(TreeDispatcherContext); + state = React.useContext(TreeStateContext); + return null; + }; + + function Child({logError = false, logWarning = false}) { + if (logError === true) { + console.error('test-only: error'); + } + if (logWarning === true) { + console.warn('test-only: warning'); + } + return null; + } + + async function selectNextErrorOrWarning() { + await utils.actAsync( + () => + dispatch({type: 'SELECT_NEXT_ELEMENT_WITH_ERROR_OR_WARNING_IN_TREE'}), + false, + ); + } + + async function selectPreviousErrorOrWarning() { + await utils.actAsync( + () => + dispatch({ + type: 'SELECT_PREVIOUS_ELEMENT_WITH_ERROR_OR_WARNING_IN_TREE', + }), + false, + ); + } + + withErrorsOrWarningsIgnored(['test-only:'], () => + utils.act(() => + render( + + + + + + + + + + , + ), + ), + ); + + utils.act(() => + TestRenderer.create( + + + , + ), + ); + expect(state).toMatchInlineSnapshot(` + ✕ 0, ⚠ 2 + [root] + ▾ + ⚠ + ▾ + ▾ + ⚠ + `); + + await selectNextErrorOrWarning(); + expect(state).toMatchInlineSnapshot(` + ✕ 0, ⚠ 2 + [root] + ▾ + → ⚠ + ▾ + ▾ + ⚠ + `); + + await utils.actAsync(() => { + store.componentFilters = [utils.createDisplayNameFilter('Wrapper')]; + }, false); + + expect(state).toMatchInlineSnapshot(` + ✕ 0, ⚠ 2 + [root] + → ⚠ + ⚠ + `); + + await selectNextErrorOrWarning(); + expect(state).toMatchInlineSnapshot(` + ✕ 0, ⚠ 2 + [root] + ⚠ + → ⚠ + `); + + await utils.actAsync(() => { + store.componentFilters = []; + }, false); + expect(state).toMatchInlineSnapshot(` + ✕ 0, ⚠ 2 + [root] + ▾ + ⚠ + ▾ + ▾ + → ⚠ + `); + + await selectPreviousErrorOrWarning(); + expect(state).toMatchInlineSnapshot(` + ✕ 0, ⚠ 2 + [root] + ▾ + → ⚠ + ▾ + ▾ + ⚠ + `); + }); }); diff --git a/packages/react-devtools-shared/src/__tests__/ownersListContext-test.js b/packages/react-devtools-shared/src/__tests__/ownersListContext-test.js index 599492bed54a1..2792541eace13 100644 --- a/packages/react-devtools-shared/src/__tests__/ownersListContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/ownersListContext-test.js @@ -156,7 +156,6 @@ describe('OwnersListContext', () => { expect(await getOwnersListForOwner(firstChild)).toMatchInlineSnapshot(` [ "Grandparent", - "Parent", "Child", ] `); diff --git a/packages/react-devtools-shared/src/__tests__/profilingCache-test.js b/packages/react-devtools-shared/src/__tests__/profilingCache-test.js index 90075ec7ea066..4778daffcec2b 100644 --- a/packages/react-devtools-shared/src/__tests__/profilingCache-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilingCache-test.js @@ -1251,8 +1251,8 @@ describe('ProfilingCache', () => { Map { 1 => 16, 2 => 16, + 3 => 1, 4 => 1, - 6 => 1, } `); @@ -1260,8 +1260,8 @@ describe('ProfilingCache', () => { Map { 1 => 0, 2 => 10, + 3 => 1, 4 => 1, - 6 => 1, } `); }); @@ -1322,13 +1322,13 @@ describe('ProfilingCache', () => { `); expect(commitData[1].fiberActualDurations).toMatchInlineSnapshot(` Map { - 7 => 3, + 5 => 3, 3 => 3, } `); expect(commitData[1].fiberSelfDurations).toMatchInlineSnapshot(` Map { - 7 => 3, + 5 => 3, 3 => 0, } `); diff --git a/packages/react-devtools-shared/src/__tests__/treeContext-test.js b/packages/react-devtools-shared/src/__tests__/treeContext-test.js index 8bd5a8e17fa0b..fa2031c6b5c8d 100644 --- a/packages/react-devtools-shared/src/__tests__/treeContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/treeContext-test.js @@ -2286,91 +2286,6 @@ describe('TreeListContext', () => { `); }); - it('should properly handle when components filters are updated', () => { - const Wrapper = ({children}) => children; - - withErrorsOrWarningsIgnored(['test-only:'], () => - utils.act(() => - render( - - - - - - - - - - , - ), - ), - ); - - utils.act(() => TestRenderer.create()); - expect(state).toMatchInlineSnapshot(` - ✕ 0, ⚠ 2 - [root] - ▾ - ⚠ - ▾ - ▾ - ⚠ - `); - - selectNextErrorOrWarning(); - expect(state).toMatchInlineSnapshot(` - ✕ 0, ⚠ 2 - [root] - ▾ - → ⚠ - ▾ - ▾ - ⚠ - `); - - utils.act(() => { - store.componentFilters = [utils.createDisplayNameFilter('Wrapper')]; - }); - expect(state).toMatchInlineSnapshot(` - ✕ 0, ⚠ 2 - [root] - → ⚠ - ⚠ - `); - - selectNextErrorOrWarning(); - expect(state).toMatchInlineSnapshot(` - ✕ 0, ⚠ 2 - [root] - ⚠ - → ⚠ - `); - - utils.act(() => { - store.componentFilters = []; - }); - expect(state).toMatchInlineSnapshot(` - ✕ 0, ⚠ 2 - [root] - ▾ - ⚠ - ▾ - ▾ - → ⚠ - `); - - selectPreviousErrorOrWarning(); - expect(state).toMatchInlineSnapshot(` - ✕ 0, ⚠ 2 - [root] - ▾ - → ⚠ - ▾ - ▾ - ⚠ - `); - }); - it('should preserve errors for fibers even if they are filtered out of the tree initially', () => { const Wrapper = ({children}) => children; diff --git a/packages/react-devtools-shared/src/backend/agent.js b/packages/react-devtools-shared/src/backend/agent.js index 886435c289f61..e81af56ebdf76 100644 --- a/packages/react-devtools-shared/src/backend/agent.js +++ b/packages/react-devtools-shared/src/backend/agent.js @@ -8,7 +8,6 @@ */ import EventEmitter from '../events'; -import throttle from 'lodash.throttle'; import { SESSION_STORAGE_LAST_SELECTION_KEY, SESSION_STORAGE_RELOAD_AND_PROFILE_KEY, @@ -483,7 +482,13 @@ export default class Agent extends EventEmitter<{ this._persistedSelection = null; this._persistedSelectionMatch = null; renderer.setTrackedPath(null); - this._throttledPersistSelection(rendererID, id); + // Throttle persisting the selection. + this._lastSelectedElementID = id; + this._lastSelectedRendererID = rendererID; + if (!this._persistSelectionTimerScheduled) { + this._persistSelectionTimerScheduled = true; + setTimeout(this._persistSelection, 1000); + } } // TODO: If there was a way to change the selected DOM element @@ -790,10 +795,23 @@ export default class Agent extends EventEmitter<{ updateComponentFilters: (componentFilters: Array) => void = componentFilters => { - for (const rendererID in this._rendererInterfaces) { + for (const rendererIDString in this._rendererInterfaces) { + const rendererID = +rendererIDString; const renderer = ((this._rendererInterfaces[ (rendererID: any) ]: any): RendererInterface); + if (this._lastSelectedRendererID === rendererID) { + // Changing component filters will unmount and remount the DevTools tree. + // Track the last selection's path so we can restore the selection. + const path = renderer.getPathForElement(this._lastSelectedElementID); + if (path !== null) { + renderer.setTrackedPath(path); + this._persistedSelection = { + rendererID, + path, + }; + } + } renderer.updateComponentFilters(componentFilters); } }; @@ -893,22 +911,25 @@ export default class Agent extends EventEmitter<{ this._bridge.send('unsupportedRendererVersion', rendererID); } - _throttledPersistSelection: any = throttle( - (rendererID: number, id: number) => { - // This is throttled, so both renderer and selected ID - // might not be available by the time we read them. - // This is why we need the defensive checks here. - const renderer = this._rendererInterfaces[rendererID]; - const path = renderer != null ? renderer.getPathForElement(id) : null; - if (path !== null) { - sessionStorageSetItem( - SESSION_STORAGE_LAST_SELECTION_KEY, - JSON.stringify(({rendererID, path}: PersistedSelection)), - ); - } else { - sessionStorageRemoveItem(SESSION_STORAGE_LAST_SELECTION_KEY); - } - }, - 1000, - ); + _persistSelectionTimerScheduled: boolean = false; + _lastSelectedRendererID: number = -1; + _lastSelectedElementID: number = -1; + _persistSelection: any = () => { + this._persistSelectionTimerScheduled = false; + const rendererID = this._lastSelectedRendererID; + const id = this._lastSelectedElementID; + // This is throttled, so both renderer and selected ID + // might not be available by the time we read them. + // This is why we need the defensive checks here. + const renderer = this._rendererInterfaces[rendererID]; + const path = renderer != null ? renderer.getPathForElement(id) : null; + if (path !== null) { + sessionStorageSetItem( + SESSION_STORAGE_LAST_SELECTION_KEY, + JSON.stringify(({rendererID, path}: PersistedSelection)), + ); + } else { + sessionStorageRemoveItem(SESSION_STORAGE_LAST_SELECTION_KEY); + } + }; } diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index d5b016d387358..d1bb0429e7c48 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -63,7 +63,6 @@ import { SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY, TREE_OPERATION_ADD, TREE_OPERATION_REMOVE, - TREE_OPERATION_REMOVE_ROOT, TREE_OPERATION_REORDER_CHILDREN, TREE_OPERATION_SET_SUBTREE_MODE, TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS, @@ -856,8 +855,14 @@ export function attach( // (due to e.g. Suspense or error boundaries). // onErrorOrWarning() adds Fibers and recordPendingErrorsAndWarnings() later clears them. const fibersWithChangedErrorOrWarningCounts: Set = new Set(); - const pendingFiberToErrorsMap: Map> = new Map(); - const pendingFiberToWarningsMap: Map> = new Map(); + const pendingFiberToErrorsMap: WeakMap< + Fiber, + Map, + > = new WeakMap(); + const pendingFiberToWarningsMap: WeakMap< + Fiber, + Map, + > = new WeakMap(); function clearErrorsAndWarnings() { // eslint-disable-next-line no-for-of-loops/no-for-of-loops @@ -876,7 +881,7 @@ export function attach( function clearMessageCountHelper( instanceID: number, - pendingFiberToMessageCountMap: Map>, + pendingFiberToMessageCountMap: WeakMap>, forError: boolean, ) { const devtoolsInstance = idToDevToolsInstanceMap.get(instanceID); @@ -902,7 +907,7 @@ export function attach( if (devtoolsInstance.kind === FIBER_INSTANCE) { const fiber = devtoolsInstance.data; // Throw out any pending changes. - pendingFiberToErrorsMap.delete(fiber); + pendingFiberToMessageCountMap.delete(fiber); if (changed) { // If previous flushed counts have changed, schedule an update too. @@ -1143,11 +1148,8 @@ export function attach( // Recursively unmount all roots. hook.getFiberRoots(rendererID).forEach(root => { - 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. - pushOperation(TREE_OPERATION_REMOVE_ROOT); + currentRootID = getFiberInstanceThrows(root.current).id; + unmountFiberRecursively(root.current); flushPendingEvents(root); currentRootID = -1; }); @@ -1159,7 +1161,22 @@ export function attach( // Recursively re-mount all roots with new filter criteria applied. hook.getFiberRoots(rendererID).forEach(root => { - currentRootID = getOrGenerateFiberInstance(root.current).id; + const current = root.current; + const alternate = current.alternate; + const newRoot = createFiberInstance(current); + idToDevToolsInstanceMap.set(newRoot.id, newRoot); + fiberToFiberInstanceMap.set(current, newRoot); + if (alternate) { + fiberToFiberInstanceMap.set(alternate, newRoot); + } + + // Before the traversals, remember to start tracking + // our path in case we have selection to restore. + if (trackedPath !== null) { + mightBeOnTrackedPath = true; + } + + currentRootID = newRoot.id; setRootPseudoKey(currentRootID, root.current); mountFiberRecursively(root.current, false); flushPendingEvents(root); @@ -1322,43 +1339,6 @@ export function attach( // When a mount or update is in progress, this value tracks the root that is being operated on. let currentRootID: number = -1; - // 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 getOrGenerateFiberInstance(fiber: Fiber): FiberInstance { - let fiberInstance = fiberToFiberInstanceMap.get(fiber); - if (fiberInstance === undefined) { - const {alternate} = fiber; - if (alternate !== null) { - fiberInstance = fiberToFiberInstanceMap.get(alternate); - if (fiberInstance !== undefined) { - // We found the other pair, so we need to make sure we track the other side. - fiberToFiberInstanceMap.set(fiber, fiberInstance); - } - } - } - - let didGenerateID = false; - if (fiberInstance === undefined) { - didGenerateID = true; - fiberInstance = createFiberInstance(fiber); - fiberToFiberInstanceMap.set(fiber, fiberInstance); - idToDevToolsInstanceMap.set(fiberInstance.id, fiberInstance); - } - - if (__DEBUG__) { - if (didGenerateID) { - debug( - 'getOrGenerateFiberInstance()', - fiber, - fiberInstance.parent, - 'Generated a new UID', - ); - } - } - - 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); @@ -1412,9 +1392,21 @@ export function attach( idToDevToolsInstanceMap.delete(fiberInstance.id); - // Also clear any errors/warnings associated with this fiber. - clearErrorsForElementID(fiberInstance.id); - clearWarningsForElementID(fiberInstance.id); + const fiber = fiberInstance.data; + + // Restore any errors/warnings associated with this fiber to the pending + // map. I.e. treat it as before we tracked the instances. This lets us + // restore them if we remount the same Fibers later. Otherwise we rely + // on the GC of the Fibers to clean them up. + if (fiberInstance.errors !== null) { + pendingFiberToErrorsMap.set(fiber, fiberInstance.errors); + fiberInstance.errors = null; + } + if (fiberInstance.warnings !== null) { + pendingFiberToWarningsMap.set(fiber, fiberInstance.warnings); + fiberInstance.warnings = null; + } + if (fiberInstance.flags & FORCE_ERROR) { fiberInstance.flags &= ~FORCE_ERROR; forceErrorCount--; @@ -1430,7 +1422,6 @@ export function attach( } } - const fiber = fiberInstance.data; fiberToFiberInstanceMap.delete(fiber); const {alternate} = fiber; if (alternate !== null) { @@ -1745,7 +1736,6 @@ export function attach( const pendingOperations: OperationsArray = []; const pendingRealUnmountedIDs: Array = []; - const pendingSimulatedUnmountedIDs: Array = []; let pendingOperationsQueue: Array | null = []; const pendingStringTable: Map = new Map(); let pendingStringTableLength: number = 0; @@ -1776,7 +1766,6 @@ export function attach( return ( pendingOperations.length === 0 && pendingRealUnmountedIDs.length === 0 && - pendingSimulatedUnmountedIDs.length === 0 && pendingUnmountedRootID === null ); } @@ -1856,7 +1845,7 @@ export function attach( function mergeMapsAndGetCountHelper( fiber: Fiber, fiberID: number, - pendingFiberToMessageCountMap: Map>, + pendingFiberToMessageCountMap: WeakMap>, forError: boolean, ): number { let newCount = 0; @@ -1932,18 +1921,18 @@ export function attach( pushOperation(fiberID); pushOperation(errorCount); pushOperation(warningCount); - } - // Always clean up so that we don't leak. - pendingFiberToErrorsMap.delete(fiber); - pendingFiberToWarningsMap.delete(fiber); + // Only clear the ones that we've already shown. Leave others in case + // they mount later. + pendingFiberToErrorsMap.delete(fiber); + pendingFiberToWarningsMap.delete(fiber); + } }); fibersWithChangedErrorOrWarningCounts.clear(); } function flushPendingEvents(root: Object): void { // Add any pending errors and warnings to the operations array. - // We do this just before flushing, so we can ignore errors for no-longer-mounted Fibers. recordPendingErrorsAndWarnings(); if (shouldBailoutWithPendingOperations()) { @@ -1960,7 +1949,6 @@ export function attach( const numUnmountIDs = pendingRealUnmountedIDs.length + - pendingSimulatedUnmountedIDs.length + (pendingUnmountedRootID === null ? 0 : 1); const operations = new Array( @@ -2013,15 +2001,6 @@ export function attach( for (let j = 0; j < pendingRealUnmountedIDs.length; j++) { operations[i++] = pendingRealUnmountedIDs[j]; } - // Fill in the simulated unmounts (hidden Suspense subtrees) in their order. - // (We want children to go before parents.) - // They go *after* the real unmounts because we know for sure they won't be - // children of already pushed "real" IDs. If they were, we wouldn't be able - // to discover them during the traversal, as they would have been deleted. - for (let j = 0; j < pendingSimulatedUnmountedIDs.length; j++) { - operations[i + j] = pendingSimulatedUnmountedIDs[j]; - } - i += pendingSimulatedUnmountedIDs.length; // The root ID should always be unmounted last. if (pendingUnmountedRootID !== null) { operations[i] = pendingUnmountedRootID; @@ -2040,7 +2019,6 @@ export function attach( // Reset all of the pending state now that we've told the frontend about it. pendingOperations.length = 0; pendingRealUnmountedIDs.length = 0; - pendingSimulatedUnmountedIDs.length = 0; pendingUnmountedRootID = null; pendingStringTable.clear(); pendingStringTableLength = 0; @@ -2078,7 +2056,24 @@ export function attach( parentInstance: DevToolsInstance | null, ): FiberInstance { const isRoot = fiber.tag === HostRoot; - const fiberInstance = getOrGenerateFiberInstance(fiber); + let fiberInstance; + if (isRoot) { + const entry = fiberToFiberInstanceMap.get(fiber); + if (entry === undefined) { + throw new Error('The root should have been registered at this point'); + } + fiberInstance = entry; + } else if ( + fiberToFiberInstanceMap.has(fiber) || + (fiber.alternate !== null && fiberToFiberInstanceMap.has(fiber.alternate)) + ) { + throw new Error('Did not expect to see this fiber being mounted twice.'); + } else { + fiberInstance = createFiberInstance(fiber); + } + fiberToFiberInstanceMap.set(fiber, fiberInstance); + idToDevToolsInstanceMap.set(fiberInstance.id, fiberInstance); + const id = fiberInstance.id; if (__DEBUG__) { @@ -2131,7 +2126,12 @@ export function attach( let ownerID: number; if (debugOwner != null) { if (typeof debugOwner.tag === 'number') { - ownerID = getOrGenerateFiberInstance((debugOwner: any)).id; + const ownerFiberInstance = getFiberInstanceUnsafe((debugOwner: any)); + if (ownerFiberInstance !== null) { + ownerID = ownerFiberInstance.id; + } else { + ownerID = 0; + } } else { // TODO: Track Server Component Owners. ownerID = 0; @@ -2183,17 +2183,9 @@ export function attach( return fiberInstance; } - function recordUnmount( - fiber: Fiber, - isSimulated: boolean, - ): null | FiberInstance { + function recordUnmount(fiber: Fiber): null | FiberInstance { if (__DEBUG__) { - debug( - 'recordUnmount()', - fiber, - null, - isSimulated ? 'unmount is simulated' : '', - ); + debug('recordUnmount()', fiber, null); } if (trackedPathMatchFiber !== null) { @@ -2223,28 +2215,22 @@ export function attach( 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. + // Roots must be removed only after all children have been removed. // So we track it separately. pendingUnmountedRootID = id; } else if (!shouldFilterFiber(fiber)) { // To maintain child-first ordering, // we'll push it into one of these queues, // and later arrange them in the correct order. - if (isSimulated) { - pendingSimulatedUnmountedIDs.push(id); - } else { - pendingRealUnmountedIDs.push(id); - } + pendingRealUnmountedIDs.push(id); } - if (!fiber._debugNeedsRemount) { - untrackFiber(fiberInstance); + untrackFiber(fiberInstance); - const isProfilingSupported = fiber.hasOwnProperty('treeBaseDuration'); - if (isProfilingSupported) { - idToRootMap.delete(id); - idToTreeBaseDurationMap.delete(id); - } + const isProfilingSupported = fiber.hasOwnProperty('treeBaseDuration'); + if (isProfilingSupported) { + idToRootMap.delete(id); + idToTreeBaseDurationMap.delete(id); } return fiberInstance; } @@ -2323,7 +2309,7 @@ export function attach( let child = remainingReconcilingChildren; while (child !== null) { if (child.kind === FIBER_INSTANCE) { - unmountFiberRecursively(child.data, false); + unmountFiberRecursively(child.data); } removeChild(child); child = remainingReconcilingChildren; @@ -2347,10 +2333,6 @@ export function attach( fiber: Fiber, traceNearestHostComponentUpdate: boolean, ): void { - // Generate an ID even for filtered Fibers, in case it's needed later (e.g. for Profiling). - // TODO: Do we really need to do this eagerly? - getOrGenerateFiberInstance(fiber); - if (__DEBUG__) { debug('mountFiberRecursively()', fiber, reconcilingParent); } @@ -2452,7 +2434,7 @@ export function attach( // We use this to simulate unmounting for Suspense trees // when we switch from primary to fallback, or deleting a subtree. - function unmountFiberRecursively(fiber: Fiber, isSimulated: boolean) { + function unmountFiberRecursively(fiber: Fiber) { if (__DEBUG__) { debug('unmountFiberRecursively()', fiber, null); } @@ -2493,7 +2475,7 @@ export function attach( child = fallbackChildFragment ? fallbackChildFragment.child : null; } - unmountChildrenRecursively(child, isSimulated); + unmountChildrenRecursively(child); } finally { if (shouldIncludeInTree) { reconcilingParent = stashedParent; @@ -2502,20 +2484,17 @@ export function attach( } } if (fiberInstance !== null) { - recordUnmount(fiber, isSimulated); + recordUnmount(fiber); removeChild(fiberInstance); } } - function unmountChildrenRecursively( - firstChild: null | Fiber, - isSimulated: boolean, - ) { + function unmountChildrenRecursively(firstChild: null | Fiber) { let child: null | Fiber = firstChild; while (child !== null) { // Record simulated unmounts children-first. // We skip nodes without return because those are real unmounts. - unmountFiberRecursively(child, isSimulated); + unmountFiberRecursively(child); child = child.sibling; } } @@ -2725,10 +2704,6 @@ export function attach( prevFiber: Fiber, traceNearestHostComponentUpdate: boolean, ): boolean { - // 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, reconcilingParent); } @@ -2758,26 +2733,38 @@ export function attach( } } - if ( - mostRecentlyInspectedElement !== null && - mostRecentlyInspectedElement.id === id && - didFiberRender(prevFiber, nextFiber) - ) { - // If this Fiber has updated, clear cached inspected data. - // If it is inspected again, it may need to be re-run to obtain updated hooks values. - hasElementUpdatedSinceLastInspected = true; - } - + let fiberInstance: null | FiberInstance = null; const shouldIncludeInTree = !shouldFilterFiber(nextFiber); if (shouldIncludeInTree) { + const entry = fiberToFiberInstanceMap.get(prevFiber); + if (entry === undefined) { + throw new Error( + 'The previous version of the fiber should have already been registered.', + ); + } + fiberInstance = entry; + // Register the new alternate in case it's not already in. + fiberToFiberInstanceMap.set(nextFiber, fiberInstance); + // Update the Fiber so we that we always keep the current Fiber on the data. fiberInstance.data = nextFiber; moveChild(fiberInstance); + + if ( + mostRecentlyInspectedElement !== null && + mostRecentlyInspectedElement.id === fiberInstance.id && + didFiberRender(prevFiber, nextFiber) + ) { + // If this Fiber has updated, clear cached inspected data. + // If it is inspected again, it may need to be re-run to obtain updated hooks values. + hasElementUpdatedSinceLastInspected = true; + } } + const stashedParent = reconcilingParent; const stashedPrevious = previouslyReconciledSibling; const stashedRemaining = remainingReconcilingChildren; - if (shouldIncludeInTree) { + if (fiberInstance !== null) { // Push a new DevTools instance parent while reconciling this subtree. reconcilingParent = fiberInstance; previouslyReconciledSibling = null; @@ -2856,9 +2843,7 @@ export function attach( } else if (!prevDidTimeout && nextDidTimeOut) { // Primary -> Fallback: // 1. Hide primary set - // This is not a real unmount, so it won't get reported by React. - // We need to manually walk the previous tree and record unmounts. - unmountChildrenRecursively(prevFiber.child, true); + unmountChildrenRecursively(prevFiber.child); // 2. Mount fallback set const nextFiberChild = nextFiber.child; const nextFallbackChildSet = nextFiberChild @@ -2886,7 +2871,7 @@ export function attach( } } else { // Children are unchanged. - if (shouldIncludeInTree) { + if (fiberInstance !== null) { // All the remaining children will be children of this same fiber so we can just reuse them. // I.e. we just restore them by undoing what we did above. fiberInstance.firstChild = remainingReconcilingChildren; @@ -3003,7 +2988,15 @@ 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 = getOrGenerateFiberInstance(root.current).id; + const current = root.current; + const alternate = current.alternate; + const newRoot = createFiberInstance(current); + idToDevToolsInstanceMap.set(newRoot.id, newRoot); + fiberToFiberInstanceMap.set(current, newRoot); + if (alternate) { + fiberToFiberInstanceMap.set(alternate, newRoot); + } + currentRootID = newRoot.id; setRootPseudoKey(currentRootID, root.current); // Handle multi-renderer edge-case where only some v16 renderers support profiling. @@ -3060,7 +3053,20 @@ export function attach( const current = root.current; const alternate = current.alternate; - currentRootID = getOrGenerateFiberInstance(current).id; + const existingRoot = + fiberToFiberInstanceMap.get(current) || + (alternate && fiberToFiberInstanceMap.get(alternate)); + if (!existingRoot) { + const newRoot = createFiberInstance(current); + idToDevToolsInstanceMap.set(newRoot.id, newRoot); + fiberToFiberInstanceMap.set(current, newRoot); + if (alternate) { + fiberToFiberInstanceMap.set(alternate, newRoot); + } + currentRootID = newRoot.id; + } else { + currentRootID = existingRoot.id; + } // Before the traversals, remember to start tracking // our path in case we have selection to restore. @@ -3117,7 +3123,7 @@ export function attach( } else if (wasMounted && !isMounted) { // Unmount an existing root. removeRootPseudoKey(currentRootID); - unmountFiberRecursively(alternate, false); + unmountFiberRecursively(alternate); } } else { // Mount a new root. @@ -3617,7 +3623,9 @@ export function attach( while (owner != null) { if (typeof owner.tag === 'number') { const ownerFiber: Fiber = (owner: any); // Refined - owners.unshift(fiberToSerializedElement(ownerFiber)); + if (!shouldFilterFiber(ownerFiber)) { + owners.unshift(fiberToSerializedElement(ownerFiber)); + } owner = ownerFiber._debugOwner; } else { // TODO: Track Server Component Owners. diff --git a/packages/react-devtools-shared/src/backend/views/Highlighter/index.js b/packages/react-devtools-shared/src/backend/views/Highlighter/index.js index 7fefa837e2fc5..419fefc4ffcf8 100644 --- a/packages/react-devtools-shared/src/backend/views/Highlighter/index.js +++ b/packages/react-devtools-shared/src/backend/views/Highlighter/index.js @@ -7,8 +7,6 @@ * @flow */ -import memoize from 'memoize-one'; -import throttle from 'lodash.throttle'; import Agent from 'react-devtools-shared/src/backend/agent'; import {hideOverlay, showOverlay} from './Highlighter'; @@ -189,18 +187,12 @@ export default function setupHighlighter( event.stopPropagation(); } - const selectElementForNode = throttle( - memoize((node: HTMLElement) => { - const id = agent.getIDForHostInstance(node); - if (id !== null) { - bridge.send('selectElement', id); - } - }), - 200, - // Don't change the selection in the very first 200ms - // because those are usually unintentional as you lift the cursor. - {leading: false}, - ); + const selectElementForNode = (node: HTMLElement) => { + const id = agent.getIDForHostInstance(node); + if (id !== null) { + bridge.send('selectElement', id); + } + }; function getEventTarget(event: MouseEvent): HTMLElement { if (event.composed) { diff --git a/packages/react-devtools-shared/src/devtools/views/hooks.js b/packages/react-devtools-shared/src/devtools/views/hooks.js index 6d0afb5c0a0ca..f2debdfa8f580 100644 --- a/packages/react-devtools-shared/src/devtools/views/hooks.js +++ b/packages/react-devtools-shared/src/devtools/views/hooks.js @@ -7,7 +7,6 @@ * @flow */ -import throttle from 'lodash.throttle'; import { useCallback, useEffect, @@ -125,10 +124,8 @@ export function useIsOverflowing( const container = ((containerRef.current: any): HTMLDivElement); - const handleResize = throttle( - () => setIsOverflowing(container.clientWidth <= totalChildWidth), - 100, - ); + const handleResize = () => + setIsOverflowing(container.clientWidth <= totalChildWidth); handleResize();