diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.expect.md new file mode 100644 index 0000000000..b8c7f8d422 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.expect.md @@ -0,0 +1,129 @@ + +## Input + +```javascript +import {makeArray, mutate} from 'shared-runtime'; + +/** + * Bug repro: + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) + * {"bar":4,"x":{"foo":3,"wat0":"joe"}} + * {"bar":5,"x":{"foo":3,"wat0":"joe"}} + * Forget: + * (kind: ok) + * {"bar":4,"x":{"foo":3,"wat0":"joe"}} + * {"bar":5,"x":{"foo":3,"wat0":"joe","wat1":"joe"}} + * + * Fork of `capturing-func-alias-captured-mutate`, but instead of directly + * aliasing `y` via `[y]`, we make an opaque call. + * + * Note that the bug here is that we don't infer that `a = makeArray(y)` + * potentially captures a context variable into a local variable. As a result, + * we don't understand that `a[0].x = b` captures `x` into `y` -- instead, we're + * currently inferring that this lambda captures `y` (for a potential later + * mutation) and simply reads `x`. + * + * Concretely `InferReferenceEffects.hasContextRefOperand` is incorrectly not + * used when we analyze CallExpressions. + */ +function Component({foo, bar}: {foo: number; bar: number}) { + let x = {foo}; + let y: {bar: number; x?: {foo: number}} = {bar}; + const f0 = function () { + let a = makeArray(y); // a = [y] + let b = x; + // this writes y.x = x + a[0].x = b; + }; + f0(); + mutate(y.x); + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{foo: 3, bar: 4}], + sequentialRenders: [ + {foo: 3, bar: 4}, + {foo: 3, bar: 5}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { makeArray, mutate } from "shared-runtime"; + +/** + * Bug repro: + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) + * {"bar":4,"x":{"foo":3,"wat0":"joe"}} + * {"bar":5,"x":{"foo":3,"wat0":"joe"}} + * Forget: + * (kind: ok) + * {"bar":4,"x":{"foo":3,"wat0":"joe"}} + * {"bar":5,"x":{"foo":3,"wat0":"joe","wat1":"joe"}} + * + * Fork of `capturing-func-alias-captured-mutate`, but instead of directly + * aliasing `y` via `[y]`, we make an opaque call. + * + * Note that the bug here is that we don't infer that `a = makeArray(y)` + * potentially captures a context variable into a local variable. As a result, + * we don't understand that `a[0].x = b` captures `x` into `y` -- instead, we're + * currently inferring that this lambda captures `y` (for a potential later + * mutation) and simply reads `x`. + * + * Concretely `InferReferenceEffects.hasContextRefOperand` is incorrectly not + * used when we analyze CallExpressions. + */ +function Component(t0) { + const $ = _c(5); + const { foo, bar } = t0; + let t1; + if ($[0] !== foo) { + t1 = { foo }; + $[0] = foo; + $[1] = t1; + } else { + t1 = $[1]; + } + const x = t1; + let y; + if ($[2] !== bar || $[3] !== x) { + y = { bar }; + const f0 = function () { + const a = makeArray(y); + const b = x; + + a[0].x = b; + }; + + f0(); + mutate(y.x); + $[2] = bar; + $[3] = x; + $[4] = y; + } else { + y = $[4]; + } + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ foo: 3, bar: 4 }], + sequentialRenders: [ + { foo: 3, bar: 4 }, + { foo: 3, bar: 5 }, + ], +}; + +``` + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.ts new file mode 100644 index 0000000000..ca7076fda4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.ts @@ -0,0 +1,48 @@ +import {makeArray, mutate} from 'shared-runtime'; + +/** + * Bug repro: + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) + * {"bar":4,"x":{"foo":3,"wat0":"joe"}} + * {"bar":5,"x":{"foo":3,"wat0":"joe"}} + * Forget: + * (kind: ok) + * {"bar":4,"x":{"foo":3,"wat0":"joe"}} + * {"bar":5,"x":{"foo":3,"wat0":"joe","wat1":"joe"}} + * + * Fork of `capturing-func-alias-captured-mutate`, but instead of directly + * aliasing `y` via `[y]`, we make an opaque call. + * + * Note that the bug here is that we don't infer that `a = makeArray(y)` + * potentially captures a context variable into a local variable. As a result, + * we don't understand that `a[0].x = b` captures `x` into `y` -- instead, we're + * currently inferring that this lambda captures `y` (for a potential later + * mutation) and simply reads `x`. + * + * Concretely `InferReferenceEffects.hasContextRefOperand` is incorrectly not + * used when we analyze CallExpressions. + */ +function Component({foo, bar}: {foo: number; bar: number}) { + let x = {foo}; + let y: {bar: number; x?: {foo: number}} = {bar}; + const f0 = function () { + let a = makeArray(y); // a = [y] + let b = x; + // this writes y.x = x + a[0].x = b; + }; + f0(); + mutate(y.x); + return y; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{foo: 3, bar: 4}], + sequentialRenders: [ + {foo: 3, bar: 4}, + {foo: 3, bar: 5}, + ], +}; diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index 36b3e92f96..3610707396 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -479,6 +479,7 @@ const skipFilter = new Set([ // bugs 'fbt/bug-fbt-plural-multiple-function-calls', 'fbt/bug-fbt-plural-multiple-mixed-call-tag', + `bug-capturing-func-maybealias-captured-mutate`, 'bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr', 'bug-invalid-hoisting-functionexpr', 'bug-aliased-capture-aliased-mutate', diff --git a/fixtures/flight/src/App.js b/fixtures/flight/src/App.js index 49bfc9e051..69fbb5e0af 100644 --- a/fixtures/flight/src/App.js +++ b/fixtures/flight/src/App.js @@ -27,7 +27,8 @@ function Foo({children}) { return
{children}
; } -function Bar({children}) { +async function Bar({children}) { + await new Promise(resolve => setTimeout(() => resolve('deferred text'), 10)); return
{children}
; } @@ -81,7 +82,7 @@ export default async function App({prerender}) { {dedupedChild} - {dedupedChild} + {Promise.resolve([dedupedChild])} diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 0456136a7e..8e00df7c1c 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -69,7 +69,11 @@ import {createBoundServerReference} from './ReactFlightReplyClient'; import {readTemporaryReference} from './ReactFlightTemporaryReferences'; -import {logComponentRender} from './ReactFlightPerformanceTrack'; +import { + markAllTracksInOrder, + logComponentRender, + logDedupedComponentRender, +} from './ReactFlightPerformanceTrack'; import { REACT_LAZY_TYPE, @@ -127,6 +131,7 @@ export type JSONValue = type ProfilingResult = { track: number, endTime: number, + component: null | ReactComponentInfo, }; const ROW_ID = 0; @@ -643,7 +648,14 @@ export function reportGlobalError(response: Response, error: Error): void { } }); if (enableProfilerTimer && enableComponentPerformanceTrack) { - flushComponentPerformance(getChunk(response, 0), 0, -Infinity); + markAllTracksInOrder(); + flushComponentPerformance( + response, + getChunk(response, 0), + 0, + -Infinity, + -Infinity, + ); } } @@ -2742,9 +2754,11 @@ function resolveTypedArray( } function flushComponentPerformance( + response: Response, root: SomeChunk, trackIdx: number, // Next available track - trackTime: number, // The time after which it is available + trackTime: number, // The time after which it is available, + parentEndTime: number, ): ProfilingResult { if (!enableProfilerTimer || !enableComponentPerformanceTrack) { // eslint-disable-next-line react-internal/prod-error-codes @@ -2761,6 +2775,22 @@ function flushComponentPerformance( // chunk in two places. We should extend the current end time as if it was // rendered as part of this tree. const previousResult: ProfilingResult = root._children; + const previousEndTime = previousResult.endTime; + if ( + parentEndTime > -Infinity && + parentEndTime < previousEndTime && + previousResult.component !== null + ) { + // Log a placeholder for the deduped value under this child starting + // from the end of the self time of the parent and spanning until the + // the deduped end. + logDedupedComponentRender( + previousResult.component, + trackIdx, + parentEndTime, + previousEndTime, + ); + } // Since we didn't bump the track this time, we just return the same track. previousResult.track = trackIdx; return previousResult; @@ -2788,25 +2818,42 @@ function flushComponentPerformance( // The start time of this component is before the end time of the previous // component on this track so we need to bump the next one to a parallel track. trackIdx++; - trackTime = startTime; } + trackTime = startTime; break; } } } + for (let i = debugInfo.length - 1; i >= 0; i--) { + const info = debugInfo[i]; + if (typeof info.time === 'number') { + if (info.time > parentEndTime) { + parentEndTime = info.time; + } + } + } } - const result: ProfilingResult = {track: trackIdx, endTime: -Infinity}; + const result: ProfilingResult = { + track: trackIdx, + endTime: -Infinity, + component: null, + }; root._children = result; let childrenEndTime = -Infinity; let childTrackIdx = trackIdx; let childTrackTime = trackTime; for (let i = 0; i < children.length; i++) { const childResult = flushComponentPerformance( + response, children[i], childTrackIdx, childTrackTime, + parentEndTime, ); + if (childResult.component !== null) { + result.component = childResult.component; + } childTrackIdx = childResult.track; const childEndTime = childResult.endTime; childTrackTime = childEndTime; @@ -2837,7 +2884,10 @@ function flushComponentPerformance( startTime, endTime, childrenEndTime, + response._rootEnvironmentName, ); + // Track the root most component of the result for deduping logging. + result.component = componentInfo; } } } diff --git a/packages/react-client/src/ReactFlightPerformanceTrack.js b/packages/react-client/src/ReactFlightPerformanceTrack.js index b8bd9ca2be..d2860e407c 100644 --- a/packages/react-client/src/ReactFlightPerformanceTrack.js +++ b/packages/react-client/src/ReactFlightPerformanceTrack.js @@ -19,6 +19,26 @@ const supportsUserTiming = const COMPONENTS_TRACK = 'Server Components ⚛'; +const componentsTrackMarker = { + startTime: 0.001, + detail: { + devtools: { + color: 'primary-light', + track: 'Primary', + trackGroup: COMPONENTS_TRACK, + }, + }, +}; + +export function markAllTracksInOrder() { + if (supportsUserTiming) { + // Ensure we create the Server Component track groups earlier than the Client Scheduler + // and Client Components. We can always add the 0 time slot even if it's in the past. + // That's still considered for ordering. + performance.mark('Server Components Track', componentsTrackMarker); + } +} + // Reused to avoid thrashing the GC. const reusableComponentDevToolDetails = { color: 'primary', @@ -52,21 +72,49 @@ export function logComponentRender( startTime: number, endTime: number, childrenEndTime: number, + rootEnv: string, ): void { if (supportsUserTiming && childrenEndTime >= 0 && trackIdx < 10) { + const env = componentInfo.env; const name = componentInfo.name; + const isPrimaryEnv = env === rootEnv; const selfTime = endTime - startTime; reusableComponentDevToolDetails.color = selfTime < 0.5 - ? 'primary-light' + ? isPrimaryEnv + ? 'primary-light' + : 'secondary-light' : selfTime < 50 - ? 'primary' + ? isPrimaryEnv + ? 'primary' + : 'secondary' : selfTime < 500 - ? 'primary-dark' + ? isPrimaryEnv + ? 'primary-dark' + : 'secondary-dark' : 'error'; reusableComponentDevToolDetails.track = trackNames[trackIdx]; reusableComponentOptions.start = startTime < 0 ? 0 : startTime; reusableComponentOptions.end = childrenEndTime; - performance.measure(name, reusableComponentOptions); + const entryName = + isPrimaryEnv || env === undefined ? name : name + ' [' + env + ']'; + performance.measure(entryName, reusableComponentOptions); + } +} + +export function logDedupedComponentRender( + componentInfo: ReactComponentInfo, + trackIdx: number, + startTime: number, + endTime: number, +): void { + if (supportsUserTiming && endTime >= 0 && trackIdx < 10) { + const name = componentInfo.name; + reusableComponentDevToolDetails.color = 'tertiary-light'; + reusableComponentDevToolDetails.track = trackNames[trackIdx]; + reusableComponentOptions.start = startTime < 0 ? 0 : startTime; + reusableComponentOptions.end = endTime; + const entryName = name + ' [deduped]'; + performance.measure(entryName, reusableComponentOptions); } } diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index eccaa6a67b..d2cea81f57 100644 --- a/packages/react-debug-tools/src/ReactDebugHooks.js +++ b/packages/react-debug-tools/src/ReactDebugHooks.js @@ -20,7 +20,6 @@ import type { Dependencies, Fiber, Dispatcher as DispatcherType, - ContextDependencyWithSelect, } from 'react-reconciler/src/ReactInternalTypes'; import type {TransitionStatus} from 'react-reconciler/src/ReactFiberConfig'; @@ -76,13 +75,6 @@ function getPrimitiveStackCache(): Map> { try { // Use all hooks here to add them to the hook log. Dispatcher.useContext(({_currentValue: null}: any)); - if (typeof Dispatcher.unstable_useContextWithBailout === 'function') { - // This type check is for Flow only. - Dispatcher.unstable_useContextWithBailout( - ({_currentValue: null}: any), - null, - ); - } Dispatcher.useState(null); Dispatcher.useReducer((s: mixed, a: mixed) => s, null); Dispatcher.useRef(null); @@ -150,10 +142,7 @@ function getPrimitiveStackCache(): Map> { let currentFiber: null | Fiber = null; let currentHook: null | Hook = null; -let currentContextDependency: - | null - | ContextDependency - | ContextDependencyWithSelect = null; +let currentContextDependency: null | ContextDependency = null; function nextHook(): null | Hook { const hook = currentHook; @@ -274,22 +263,6 @@ function useContext(context: ReactContext): T { return value; } -function unstable_useContextWithBailout( - context: ReactContext, - select: (T => Array) | null, -): T { - const value = readContext(context); - hookLog.push({ - displayName: context.displayName || null, - primitive: 'ContextWithBailout', - stackError: new Error(), - value: value, - debugInfo: null, - dispatcherHookName: 'ContextWithBailout', - }); - return value; -} - function useState( initialState: (() => S) | S, ): [S, Dispatch>] { @@ -764,7 +737,6 @@ const Dispatcher: DispatcherType = { useCacheRefresh, useCallback, useContext, - unstable_useContextWithBailout, useEffect, useImperativeHandle, useDebugValue, diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 8affe4804b..771b60cbb0 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -39,12 +39,11 @@ import { enableLazyContextPropagation, enableTransitionTracing, enableUseEffectEventHook, + enableUseResourceEffectHook, enableLegacyCache, debugRenderPhaseSideEffectsForStrictMode, disableLegacyMode, enableNoCloningMemoCache, - enableContextProfiling, - enableUseResourceEffectHook, } from 'shared/ReactFeatureFlags'; import { REACT_CONTEXT_TYPE, @@ -78,11 +77,7 @@ import { ContinuousEventPriority, higherEventPriority, } from './ReactEventPriorities'; -import { - readContext, - readContextAndCompare, - checkIfContextChanged, -} from './ReactFiberNewContext'; +import {readContext, checkIfContextChanged} from './ReactFiberNewContext'; import {HostRoot, CacheComponent, HostComponent} from './ReactWorkTags'; import { LayoutStatic as LayoutStaticEffect, @@ -1111,16 +1106,6 @@ function updateWorkInProgressHook(): Hook { return workInProgressHook; } -function unstable_useContextWithBailout( - context: ReactContext, - select: (T => Array) | null, -): T { - if (select === null) { - return readContext(context); - } - return readContextAndCompare(context, select); -} - function createFunctionComponentUpdateQueue(): FunctionComponentUpdateQueue { return { lastEffect: null, @@ -3958,10 +3943,6 @@ if (enableUseEffectEventHook) { if (enableUseResourceEffectHook) { (ContextOnlyDispatcher: Dispatcher).useResourceEffect = throwInvalidHookError; } -if (enableContextProfiling) { - (ContextOnlyDispatcher: Dispatcher).unstable_useContextWithBailout = - throwInvalidHookError; -} const HooksDispatcherOnMount: Dispatcher = { readContext, @@ -3995,10 +3976,6 @@ if (enableUseEffectEventHook) { if (enableUseResourceEffectHook) { (HooksDispatcherOnMount: Dispatcher).useResourceEffect = mountResourceEffect; } -if (enableContextProfiling) { - (HooksDispatcherOnMount: Dispatcher).unstable_useContextWithBailout = - unstable_useContextWithBailout; -} const HooksDispatcherOnUpdate: Dispatcher = { readContext, @@ -4033,10 +4010,6 @@ if (enableUseResourceEffectHook) { (HooksDispatcherOnUpdate: Dispatcher).useResourceEffect = updateResourceEffect; } -if (enableContextProfiling) { - (HooksDispatcherOnUpdate: Dispatcher).unstable_useContextWithBailout = - unstable_useContextWithBailout; -} const HooksDispatcherOnRerender: Dispatcher = { readContext, @@ -4071,10 +4044,6 @@ if (enableUseResourceEffectHook) { (HooksDispatcherOnRerender: Dispatcher).useResourceEffect = updateResourceEffect; } -if (enableContextProfiling) { - (HooksDispatcherOnRerender: Dispatcher).unstable_useContextWithBailout = - unstable_useContextWithBailout; -} let HooksDispatcherOnMountInDEV: Dispatcher | null = null; let HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher | null = null; @@ -4296,17 +4265,6 @@ if (__DEV__) { ); }; } - if (enableContextProfiling) { - (HooksDispatcherOnMountInDEV: Dispatcher).unstable_useContextWithBailout = - function ( - context: ReactContext, - select: (T => Array) | null, - ): T { - currentHookNameInDev = 'useContext'; - mountHookTypesDev(); - return unstable_useContextWithBailout(context, select); - }; - } HooksDispatcherOnMountWithHookTypesInDEV = { readContext(context: ReactContext): T { @@ -4494,17 +4452,6 @@ if (__DEV__) { ); }; } - if (enableContextProfiling) { - (HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher).unstable_useContextWithBailout = - function ( - context: ReactContext, - select: (T => Array) | null, - ): T { - currentHookNameInDev = 'useContext'; - updateHookTypesDev(); - return unstable_useContextWithBailout(context, select); - }; - } HooksDispatcherOnUpdateInDEV = { readContext(context: ReactContext): T { @@ -4692,17 +4639,6 @@ if (__DEV__) { ); }; } - if (enableContextProfiling) { - (HooksDispatcherOnUpdateInDEV: Dispatcher).unstable_useContextWithBailout = - function ( - context: ReactContext, - select: (T => Array) | null, - ): T { - currentHookNameInDev = 'useContext'; - updateHookTypesDev(); - return unstable_useContextWithBailout(context, select); - }; - } HooksDispatcherOnRerenderInDEV = { readContext(context: ReactContext): T { @@ -4890,17 +4826,6 @@ if (__DEV__) { ); }; } - if (enableContextProfiling) { - (HooksDispatcherOnRerenderInDEV: Dispatcher).unstable_useContextWithBailout = - function ( - context: ReactContext, - select: (T => Array) | null, - ): T { - currentHookNameInDev = 'useContext'; - updateHookTypesDev(); - return unstable_useContextWithBailout(context, select); - }; - } InvalidNestedHooksDispatcherOnMountInDEV = { readContext(context: ReactContext): T { @@ -5114,18 +5039,6 @@ if (__DEV__) { ); }; } - if (enableContextProfiling) { - (InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher).unstable_useContextWithBailout = - function ( - context: ReactContext, - select: (T => Array) | null, - ): T { - currentHookNameInDev = 'useContext'; - warnInvalidHookAccess(); - mountHookTypesDev(); - return unstable_useContextWithBailout(context, select); - }; - } InvalidNestedHooksDispatcherOnUpdateInDEV = { readContext(context: ReactContext): T { @@ -5339,18 +5252,6 @@ if (__DEV__) { ); }; } - if (enableContextProfiling) { - (InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher).unstable_useContextWithBailout = - function ( - context: ReactContext, - select: (T => Array) | null, - ): T { - currentHookNameInDev = 'useContext'; - warnInvalidHookAccess(); - updateHookTypesDev(); - return unstable_useContextWithBailout(context, select); - }; - } InvalidNestedHooksDispatcherOnRerenderInDEV = { readContext(context: ReactContext): T { @@ -5564,16 +5465,4 @@ if (__DEV__) { ); }; } - if (enableContextProfiling) { - (InvalidNestedHooksDispatcherOnRerenderInDEV: Dispatcher).unstable_useContextWithBailout = - function ( - context: ReactContext, - select: (T => Array) | null, - ): T { - currentHookNameInDev = 'useContext'; - warnInvalidHookAccess(); - updateHookTypesDev(); - return unstable_useContextWithBailout(context, select); - }; - } } diff --git a/packages/react-reconciler/src/ReactFiberNewContext.js b/packages/react-reconciler/src/ReactFiberNewContext.js index 190475519e..6a779cfd4a 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.js @@ -12,7 +12,6 @@ import type { Fiber, ContextDependency, Dependencies, - ContextDependencyWithSelect, } from './ReactInternalTypes'; import type {StackCursor} from './ReactFiberStack'; import type {Lanes} from './ReactFiberLane'; @@ -48,8 +47,6 @@ import { enableRenderableContext, } from 'shared/ReactFeatureFlags'; import {getHostTransitionProvider} from './ReactFiberHostContext'; -import isArray from '../../shared/isArray'; -import {enableContextProfiling} from '../../shared/ReactFeatureFlags'; const valueCursor: StackCursor = createCursor(null); @@ -69,10 +66,7 @@ if (__DEV__) { } let currentlyRenderingFiber: Fiber | null = null; -let lastContextDependency: - | ContextDependency - | ContextDependencyWithSelect - | null = null; +let lastContextDependency: ContextDependency | null = null; let isDisallowedContextReadInDEV: boolean = false; @@ -401,23 +395,6 @@ function propagateContextChanges( const context: ReactContext = contexts[i]; // Check if the context matches. if (dependency.context === context) { - if (enableContextProfiling) { - const select = dependency.select; - if (select != null && dependency.lastSelectedValue != null) { - const newValue = isPrimaryRenderer - ? dependency.context._currentValue - : dependency.context._currentValue2; - if ( - !checkIfSelectedContextValuesChanged( - dependency.lastSelectedValue, - select(newValue), - ) - ) { - // Compared value hasn't changed. Bail out early. - continue findContext; - } - } - } // Match! Schedule an update on this fiber. // In the lazy implementation, don't mark a dirty flag on the @@ -657,29 +634,6 @@ function propagateParentContextChanges( workInProgress.flags |= DidPropagateContext; } -function checkIfSelectedContextValuesChanged( - oldComparedValue: Array, - newComparedValue: Array, -): boolean { - // We have an implicit contract that compare functions must return arrays. - // This allows us to compare multiple values in the same context access - // since compiling to additional hook calls regresses perf. - if (isArray(oldComparedValue) && isArray(newComparedValue)) { - if (oldComparedValue.length !== newComparedValue.length) { - return true; - } - - for (let i = 0; i < oldComparedValue.length; i++) { - if (!is(newComparedValue[i], oldComparedValue[i])) { - return true; - } - } - } else { - throw new Error('Compared context values must be arrays'); - } - return false; -} - export function checkIfContextChanged( currentDependencies: Dependencies, ): boolean { @@ -698,23 +652,8 @@ export function checkIfContextChanged( ? context._currentValue : context._currentValue2; const oldValue = dependency.memoizedValue; - if ( - enableContextProfiling && - dependency.select != null && - dependency.lastSelectedValue != null - ) { - if ( - checkIfSelectedContextValuesChanged( - dependency.lastSelectedValue, - dependency.select(newValue), - ) - ) { - return true; - } - } else { - if (!is(newValue, oldValue)) { - return true; - } + if (!is(newValue, oldValue)) { + return true; } dependency = dependency.next; } @@ -747,21 +686,6 @@ export function prepareToReadContext( } } -export function readContextAndCompare( - context: ReactContext, - select: C => Array, -): C { - if (!(enableLazyContextPropagation && enableContextProfiling)) { - throw new Error('Not implemented.'); - } - - return readContextForConsumer_withSelect( - currentlyRenderingFiber, - context, - select, - ); -} - export function readContext(context: ReactContext): T { if (__DEV__) { // This warning would fire if you read context inside a Hook like useMemo. @@ -789,59 +713,10 @@ export function readContextDuringReconciliation( return readContextForConsumer(consumer, context); } -function readContextForConsumer_withSelect( +function readContextForConsumer( consumer: Fiber | null, - context: ReactContext, - select: C => Array, -): C { - const value = isPrimaryRenderer - ? context._currentValue - : context._currentValue2; - - const contextItem = { - context: ((context: any): ReactContext), - memoizedValue: value, - next: null, - select: ((select: any): (context: mixed) => Array), - lastSelectedValue: select(value), - }; - - if (lastContextDependency === null) { - if (consumer === null) { - throw new Error( - 'Context can only be read while React is rendering. ' + - 'In classes, you can read it in the render method or getDerivedStateFromProps. ' + - 'In function components, you can read it directly in the function body, but not ' + - 'inside Hooks like useReducer() or useMemo().', - ); - } - - // This is the first dependency for this component. Create a new list. - lastContextDependency = contextItem; - consumer.dependencies = __DEV__ - ? { - lanes: NoLanes, - firstContext: contextItem, - _debugThenableState: null, - } - : { - lanes: NoLanes, - firstContext: contextItem, - }; - if (enableLazyContextPropagation) { - consumer.flags |= NeedsPropagation; - } - } else { - // Append a new context item. - lastContextDependency = lastContextDependency.next = contextItem; - } - return value; -} - -function readContextForConsumer( - consumer: Fiber | null, - context: ReactContext, -): C { + context: ReactContext, +): T { const value = isPrimaryRenderer ? context._currentValue : context._currentValue2; diff --git a/packages/react-reconciler/src/ReactFiberPerformanceTrack.js b/packages/react-reconciler/src/ReactFiberPerformanceTrack.js index 7e9e246d8b..d94fd5ee48 100644 --- a/packages/react-reconciler/src/ReactFiberPerformanceTrack.js +++ b/packages/react-reconciler/src/ReactFiberPerformanceTrack.js @@ -63,7 +63,7 @@ export function setCurrentTrackFromLanes(lanes: Lanes): void { } const blockingLaneMarker = { - startTime: 0, + startTime: 0.003, detail: { devtools: { color: 'primary-light', @@ -74,7 +74,7 @@ const blockingLaneMarker = { }; const transitionLaneMarker = { - startTime: 0, + startTime: 0.003, detail: { devtools: { color: 'primary-light', @@ -85,7 +85,7 @@ const transitionLaneMarker = { }; const suspenseLaneMarker = { - startTime: 0, + startTime: 0.003, detail: { devtools: { color: 'primary-light', @@ -96,7 +96,7 @@ const suspenseLaneMarker = { }; const idleLaneMarker = { - startTime: 0, + startTime: 0.003, detail: { devtools: { color: 'primary-light', diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index 2ef3a2da4f..afe853202f 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -63,27 +63,18 @@ export type HookType = | 'useFormState' | 'useActionState'; -export type ContextDependency = { - context: ReactContext, - next: ContextDependency | ContextDependencyWithSelect | null, - memoizedValue: C, -}; - -export type ContextDependencyWithSelect = { - context: ReactContext, - next: ContextDependency | ContextDependencyWithSelect | null, - memoizedValue: C, - select: C => Array, - lastSelectedValue: ?Array, +export type ContextDependency = { + context: ReactContext, + next: ContextDependency | null, + memoizedValue: T, + ... }; export type Dependencies = { lanes: Lanes, - firstContext: - | ContextDependency - | ContextDependencyWithSelect - | null, + firstContext: ContextDependency | null, _debugThenableState?: null | ThenableState, // DEV-only + ... }; export type MemoCache = { @@ -401,10 +392,6 @@ export type Dispatcher = { initialArg: I, init?: (I) => S, ): [S, Dispatch], - unstable_useContextWithBailout?: ( - context: ReactContext, - select: (T => Array) | null, - ) => T, useContext(context: ReactContext): T, useRef(initialValue: T): {current: T}, useEffect( diff --git a/packages/react-reconciler/src/__tests__/ReactContextWithBailout-test.js b/packages/react-reconciler/src/__tests__/ReactContextWithBailout-test.js deleted file mode 100644 index c510206148..0000000000 --- a/packages/react-reconciler/src/__tests__/ReactContextWithBailout-test.js +++ /dev/null @@ -1,217 +0,0 @@ -let React; -let ReactNoop; -let Scheduler; -let act; -let assertLog; -let useState; -let useContext; -let unstable_useContextWithBailout; - -describe('ReactContextWithBailout', () => { - beforeEach(() => { - jest.resetModules(); - - React = require('react'); - ReactNoop = require('react-noop-renderer'); - Scheduler = require('scheduler'); - const testUtils = require('internal-test-utils'); - act = testUtils.act; - assertLog = testUtils.assertLog; - useState = React.useState; - useContext = React.useContext; - unstable_useContextWithBailout = React.unstable_useContextWithBailout; - }); - - function Text({text}) { - Scheduler.log(text); - return text; - } - - // @gate enableLazyContextPropagation && enableContextProfiling - test('unstable_useContextWithBailout basic usage', async () => { - const Context = React.createContext(); - - let setContext; - function App() { - const [context, _setContext] = useState({a: 'A0', b: 'B0', c: 'C0'}); - setContext = _setContext; - return ( - - - - ); - } - - // Intermediate parent that bails out. Children will only re-render when the - // context changes. - const Indirection = React.memo(() => { - return ( - <> - A: , B: , C: , AB: - - ); - }); - - function A() { - const {a} = unstable_useContextWithBailout(Context, context => [ - context.a, - ]); - return ; - } - - function B() { - const {b} = unstable_useContextWithBailout(Context, context => [ - context.b, - ]); - return ; - } - - function C() { - const {c} = unstable_useContextWithBailout(Context, context => [ - context.c, - ]); - return ; - } - - function AB() { - const {a, b} = unstable_useContextWithBailout(Context, context => [ - context.a, - context.b, - ]); - return ; - } - - const root = ReactNoop.createRoot(); - await act(async () => { - root.render(); - }); - assertLog(['A0', 'B0', 'C0', 'A0B0']); - expect(root).toMatchRenderedOutput('A: A0, B: B0, C: C0, AB: A0B0'); - - // Update a. Only the A and AB consumer should re-render. - await act(async () => { - setContext({a: 'A1', c: 'C0', b: 'B0'}); - }); - assertLog(['A1', 'A1B0']); - expect(root).toMatchRenderedOutput('A: A1, B: B0, C: C0, AB: A1B0'); - - // Update b. Only the B and AB consumer should re-render. - await act(async () => { - setContext({a: 'A1', b: 'B1', c: 'C0'}); - }); - assertLog(['B1', 'A1B1']); - expect(root).toMatchRenderedOutput('A: A1, B: B1, C: C0, AB: A1B1'); - - // Update c. Only the C consumer should re-render. - await act(async () => { - setContext({a: 'A1', b: 'B1', c: 'C1'}); - }); - assertLog(['C1']); - expect(root).toMatchRenderedOutput('A: A1, B: B1, C: C1, AB: A1B1'); - }); - - // @gate enableLazyContextPropagation && enableContextProfiling - test('unstable_useContextWithBailout and useContext subscribing to same context in same component', async () => { - const Context = React.createContext(); - - let setContext; - function App() { - const [context, _setContext] = useState({a: 0, b: 0, unrelated: 0}); - setContext = _setContext; - return ( - - - - ); - } - - // Intermediate parent that bails out. Children will only re-render when the - // context changes. - const Indirection = React.memo(() => { - return ; - }); - - function Child() { - const {a} = unstable_useContextWithBailout(Context, context => [ - context.a, - ]); - const context = useContext(Context); - return ; - } - - const root = ReactNoop.createRoot(); - await act(async () => { - root.render(); - }); - assertLog(['A: 0, B: 0']); - expect(root).toMatchRenderedOutput('A: 0, B: 0'); - - // Update an unrelated field that isn't used by the component. The context - // attempts to bail out, but the normal context forces an update. - await act(async () => { - setContext({a: 0, b: 0, unrelated: 1}); - }); - assertLog(['A: 0, B: 0']); - expect(root).toMatchRenderedOutput('A: 0, B: 0'); - }); - - // @gate enableLazyContextPropagation && enableContextProfiling - test('unstable_useContextWithBailout and useContext subscribing to different contexts in same component', async () => { - const ContextA = React.createContext(); - const ContextB = React.createContext(); - - let setContextA; - let setContextB; - function App() { - const [a, _setContextA] = useState({a: 0, unrelated: 0}); - const [b, _setContextB] = useState(0); - setContextA = _setContextA; - setContextB = _setContextB; - return ( - - - - - - ); - } - - // Intermediate parent that bails out. Children will only re-render when the - // context changes. - const Indirection = React.memo(() => { - return ; - }); - - function Child() { - const {a} = unstable_useContextWithBailout(ContextA, context => [ - context.a, - ]); - const b = useContext(ContextB); - return ; - } - - const root = ReactNoop.createRoot(); - await act(async () => { - root.render(); - }); - assertLog(['A: 0, B: 0']); - expect(root).toMatchRenderedOutput('A: 0, B: 0'); - - // Update a field in A that isn't part of the compared context. It should - // bail out. - await act(async () => { - setContextA({a: 0, unrelated: 1}); - }); - assertLog([]); - expect(root).toMatchRenderedOutput('A: 0, B: 0'); - - // Now update the same a field again, but this time, also update a different - // context in the same batch. The other context prevents a bail out. - await act(async () => { - setContextA({a: 0, unrelated: 1}); - setContextB(1); - }); - assertLog(['A: 0, B: 1']); - expect(root).toMatchRenderedOutput('A: 0, B: 1'); - }); -}); diff --git a/packages/react/index.fb.js b/packages/react/index.fb.js index 8cded91b98..84128cf0ea 100644 --- a/packages/react/index.fb.js +++ b/packages/react/index.fb.js @@ -42,7 +42,6 @@ export { use, useActionState, useCallback, - unstable_useContextWithBailout, useContext, useDebugValue, useDeferredValue, diff --git a/packages/react/src/ReactClient.js b/packages/react/src/ReactClient.js index 90ce18b133..02625e3af9 100644 --- a/packages/react/src/ReactClient.js +++ b/packages/react/src/ReactClient.js @@ -37,7 +37,6 @@ import {postpone} from './ReactPostpone'; import { getCacheForType, useCallback, - unstable_useContextWithBailout, useContext, useEffect, useEffectEvent, @@ -86,7 +85,6 @@ export { cache, postpone as unstable_postpone, useCallback, - unstable_useContextWithBailout, useContext, useEffect, useEffectEvent as experimental_useEffectEvent, diff --git a/packages/react/src/ReactHooks.js b/packages/react/src/ReactHooks.js index 40c8a23d37..32f4926888 100644 --- a/packages/react/src/ReactHooks.js +++ b/packages/react/src/ReactHooks.js @@ -19,10 +19,6 @@ import {REACT_CONSUMER_TYPE} from 'shared/ReactSymbols'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import {enableUseResourceEffectHook} from 'shared/ReactFeatureFlags'; -import { - enableContextProfiling, - enableLazyContextPropagation, -} from '../../shared/ReactFeatureFlags'; type BasicStateAction = (S => S) | S; type Dispatch = A => void; @@ -69,27 +65,6 @@ export function useContext(Context: ReactContext): T { return dispatcher.useContext(Context); } -export function unstable_useContextWithBailout( - context: ReactContext, - select: (T => Array) | null, -): T { - if (!(enableLazyContextPropagation && enableContextProfiling)) { - throw new Error('Not implemented.'); - } - - const dispatcher = resolveDispatcher(); - if (__DEV__) { - if (context.$$typeof === REACT_CONSUMER_TYPE) { - console.error( - 'Calling useContext(Context.Consumer) is not supported and will cause bugs. ' + - 'Did you mean to call useContext(Context) instead?', - ); - } - } - // $FlowFixMe[not-a-function] This is unstable, thus optional - return dispatcher.unstable_useContextWithBailout(context, select); -} - export function useState( initialState: (() => S) | S, ): [S, Dispatch>] { diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index d1dc6cb7f7..8caf79fbf2 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -101,9 +101,6 @@ export const enableTransitionTracing = false; export const enableLazyContextPropagation = true; -// Expose unstable useContext for performance testing -export const enableContextProfiling = false; - // FB-only usage. The new API has different semantics. export const enableLegacyHidden = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 498af214b0..6c14a9ece3 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -53,7 +53,6 @@ export const enableFizzExternalRuntime = true; export const enableGetInspectorDataForInstanceInProduction = true; export const enableHalt = false; export const enableInfiniteRenderLoopDetection = false; -export const enableContextProfiling = false; export const enableLazyContextPropagation = true; export const enableLegacyCache = false; export const enableLegacyFBSupport = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index fb996a43ad..6dce24e57f 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -43,7 +43,6 @@ export const enableHalt = false; export const enableHiddenSubtreeInsertionEffectCleanup = false; export const enableInfiniteRenderLoopDetection = false; export const enableLazyContextPropagation = true; -export const enableContextProfiling = false; export const enableLegacyCache = false; export const enableLegacyFBSupport = false; export const enableLegacyHidden = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 6646a126a0..b332c485a0 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -49,7 +49,6 @@ export const transitionLaneExpirationMs = 5000; export const disableSchedulerTimeoutInWorkLoop = false; export const enableLazyContextPropagation = true; -export const enableContextProfiling = false; export const enableLegacyHidden = false; export const enableTransitionTracing = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index dae38936f8..20ecdb44b4 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -51,7 +51,6 @@ export const transitionLaneExpirationMs = 5000; export const disableSchedulerTimeoutInWorkLoop = false; export const enableLazyContextPropagation = true; -export const enableContextProfiling = false; export const enableLegacyHidden = false; export const enableTransitionTracing = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 060600286e..8054a14bbc 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -79,8 +79,6 @@ export const enablePostpone = false; export const enableHalt = false; -export const enableContextProfiling = true; - // TODO: www currently relies on this feature. It's disabled in open source. // Need to remove it. export const disableCommentsAsDOMContainers = false; diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 65961e313b..3ad0aa6f60 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -530,4 +530,3 @@ "542": "Suspense Exception: This is not a real error! It's an implementation detail of `useActionState` to interrupt the current render. You must either rethrow it immediately, or move the `useActionState` call outside of the `try/catch` block. Capturing without rethrowing will lead to unexpected behavior.\n\nTo handle async errors, wrap your component in an error boundary.", "543": "Expected a ResourceEffectUpdate to be pushed together with ResourceEffectIdentity. This is a bug in React." } -