From f7b1273da2c96057d3908f52f8587379d4418f66 Mon Sep 17 00:00:00 2001 From: Ricky Date: Mon, 16 Dec 2024 11:18:03 -0500 Subject: [PATCH 01/18] Flag for requestPaint (#31805) Will run a quick experiment for this. --- packages/scheduler/src/SchedulerFeatureFlags.js | 1 + packages/scheduler/src/__tests__/Scheduler-test.js | 6 +++++- packages/scheduler/src/forks/Scheduler.js | 11 ++++++++--- .../src/forks/SchedulerFeatureFlags.www-dynamic.js | 1 + .../scheduler/src/forks/SchedulerFeatureFlags.www.js | 1 + 5 files changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/scheduler/src/SchedulerFeatureFlags.js b/packages/scheduler/src/SchedulerFeatureFlags.js index c86de5e35dc2a..08a0d979b621b 100644 --- a/packages/scheduler/src/SchedulerFeatureFlags.js +++ b/packages/scheduler/src/SchedulerFeatureFlags.js @@ -14,3 +14,4 @@ export const frameYieldMs = 5; export const userBlockingPriorityTimeout = 250; export const normalPriorityTimeout = 5000; export const lowPriorityTimeout = 10000; +export const enableRequestPaint = true; diff --git a/packages/scheduler/src/__tests__/Scheduler-test.js b/packages/scheduler/src/__tests__/Scheduler-test.js index 9e0813e461e61..fee1216d15730 100644 --- a/packages/scheduler/src/__tests__/Scheduler-test.js +++ b/packages/scheduler/src/__tests__/Scheduler-test.js @@ -18,6 +18,7 @@ let scheduleCallback; let requestPaint; let shouldYield; let NormalPriority; +let SchedulerFeatureFlags; // The Scheduler implementation uses browser APIs like `MessageChannel` and // `setTimeout` to schedule work on the main thread. Most of our tests treat @@ -42,6 +43,7 @@ describe('SchedulerBrowser', () => { NormalPriority = Scheduler.unstable_NormalPriority; requestPaint = Scheduler.unstable_requestPaint; shouldYield = Scheduler.unstable_shouldYield; + SchedulerFeatureFlags = require('../SchedulerFeatureFlags'); }); afterEach(() => { @@ -199,7 +201,9 @@ describe('SchedulerBrowser', () => { runtime.assertLog([ 'Message Event', 'Task', - 'Yield at 0ms', + SchedulerFeatureFlags.enableRequestPaint + ? 'Yield at 0ms' + : `Yield at ${SchedulerFeatureFlags.frameYieldMs}ms`, 'Post Message', ]); diff --git a/packages/scheduler/src/forks/Scheduler.js b/packages/scheduler/src/forks/Scheduler.js index 0e85e0a99b7b0..68149a9fec14e 100644 --- a/packages/scheduler/src/forks/Scheduler.js +++ b/packages/scheduler/src/forks/Scheduler.js @@ -18,6 +18,7 @@ import { userBlockingPriorityTimeout, lowPriorityTimeout, normalPriorityTimeout, + enableRequestPaint, } from '../SchedulerFeatureFlags'; import {push, pop, peek} from '../SchedulerMinHeap'; @@ -458,7 +459,7 @@ let frameInterval = frameYieldMs; let startTime = -1; function shouldYieldToHost(): boolean { - if (needsPaint) { + if (enableRequestPaint && needsPaint) { // Yield now. return true; } @@ -473,7 +474,9 @@ function shouldYieldToHost(): boolean { } function requestPaint() { - needsPaint = true; + if (enableRequestPaint) { + needsPaint = true; + } } function forceFrameRate(fps: number) { @@ -494,7 +497,9 @@ function forceFrameRate(fps: number) { } const performWorkUntilDeadline = () => { - needsPaint = false; + if (enableRequestPaint) { + needsPaint = false; + } if (isMessageLoopRunning) { const currentTime = getCurrentTime(); // Keep track of the start time so we can measure how long the main thread diff --git a/packages/scheduler/src/forks/SchedulerFeatureFlags.www-dynamic.js b/packages/scheduler/src/forks/SchedulerFeatureFlags.www-dynamic.js index 8bbe8385b7f63..9346663083bc6 100644 --- a/packages/scheduler/src/forks/SchedulerFeatureFlags.www-dynamic.js +++ b/packages/scheduler/src/forks/SchedulerFeatureFlags.www-dynamic.js @@ -14,3 +14,4 @@ export const userBlockingPriorityTimeout = 250; export const normalPriorityTimeout = 5000; export const lowPriorityTimeout = 10000; +export const enableRequestPaint = __VARIANT__; diff --git a/packages/scheduler/src/forks/SchedulerFeatureFlags.www.js b/packages/scheduler/src/forks/SchedulerFeatureFlags.www.js index 5c42786c94d42..ec8498ec6adb5 100644 --- a/packages/scheduler/src/forks/SchedulerFeatureFlags.www.js +++ b/packages/scheduler/src/forks/SchedulerFeatureFlags.www.js @@ -14,6 +14,7 @@ export const { userBlockingPriorityTimeout, normalPriorityTimeout, lowPriorityTimeout, + enableRequestPaint, } = dynamicFeatureFlags; export const frameYieldMs = 10; From 031230d2e047c1a354037795a5790ed421d51c55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 16 Dec 2024 11:58:25 -0500 Subject: [PATCH 02/18] [Flight] Stack Parallel Components in Separate Tracks (#31735) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on https://github.com/facebook/react/pull/31729 Screenshot 2024-12-11 at 3 36 41 PM The Server Components visualization is currently a tree flame graph where parent spans the child. This makes it equivalent to the Client Components visualization. However, since Server Components can be async and therefore parallel, we need to do something when two children are executed in parallel. This PR bumps parallel children into a separate track and then within that track if that child has more children it can grow within that track. I currently just cut off more than 10 parallel tracks. Synchronous Server Components are still in sequence but it's unlikely because even a simple microtasky Async Component is still parallel. Screenshot 2024-12-11 at 4 31 17 PM I think this is probably not a very useful visualization for Server Components but we can try it out. I'm also going to try a different visualization where parent-child relationship is horizontal and parallel vertical instead, but it might not be possible to make that line up in this tool. It makes it a little harder to see how much different components (including their children) impact the overall tree. If that's the only visualization it's also confusing why it's different dimensions than the Client Component version. --- .../react-client/src/ReactFlightClient.js | 61 ++++++++++++++++--- .../src/ReactFlightPerformanceTrack.js | 20 +++++- 2 files changed, 71 insertions(+), 10 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 0e2bb5b9f4606..0456136a7e6cf 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -125,6 +125,7 @@ export type JSONValue = | $ReadOnlyArray; type ProfilingResult = { + track: number, endTime: number, }; @@ -642,7 +643,7 @@ export function reportGlobalError(response: Response, error: Error): void { } }); if (enableProfilerTimer && enableComponentPerformanceTrack) { - flushComponentPerformance(getChunk(response, 0)); + flushComponentPerformance(getChunk(response, 0), 0, -Infinity); } } @@ -2740,9 +2741,16 @@ function resolveTypedArray( resolveBuffer(response, id, view); } -function flushComponentPerformance(root: SomeChunk): number { +function flushComponentPerformance( + root: SomeChunk, + trackIdx: number, // Next available track + trackTime: number, // The time after which it is available +): ProfilingResult { if (!enableProfilerTimer || !enableComponentPerformanceTrack) { - return 0; + // eslint-disable-next-line react-internal/prod-error-codes + throw new Error( + 'flushComponentPerformance should never be called in production mode. This is a bug in React.', + ); } // Write performance.measure() entries for Server Components in tree order. // This must be done at the end to collect the end time from the whole tree. @@ -2753,7 +2761,9 @@ function flushComponentPerformance(root: SomeChunk): number { // 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; - return previousResult.endTime; + // Since we didn't bump the track this time, we just return the same track. + previousResult.track = trackIdx; + return previousResult; } const children = root._children; if (root.status === RESOLVED_MODEL) { @@ -2762,16 +2772,49 @@ function flushComponentPerformance(root: SomeChunk): number { // the performance characteristics of the app by profiling. initializeModelChunk(root); } - const result: ProfilingResult = {endTime: -Infinity}; + + // First find the start time of the first component to know if it was running + // in parallel with the previous. + const debugInfo = root._debugInfo; + if (debugInfo) { + for (let i = 1; i < debugInfo.length; i++) { + const info = debugInfo[i]; + if (typeof info.name === 'string') { + // $FlowFixMe: Refined. + const startTimeInfo = debugInfo[i - 1]; + if (typeof startTimeInfo.time === 'number') { + const startTime = startTimeInfo.time; + if (startTime < trackTime) { + // 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; + } + break; + } + } + } + } + + const result: ProfilingResult = {track: trackIdx, endTime: -Infinity}; root._children = result; let childrenEndTime = -Infinity; + let childTrackIdx = trackIdx; + let childTrackTime = trackTime; for (let i = 0; i < children.length; i++) { - const childEndTime = flushComponentPerformance(children[i]); + const childResult = flushComponentPerformance( + children[i], + childTrackIdx, + childTrackTime, + ); + childTrackIdx = childResult.track; + const childEndTime = childResult.endTime; + childTrackTime = childEndTime; if (childEndTime > childrenEndTime) { childrenEndTime = childEndTime; } } - const debugInfo = root._debugInfo; + if (debugInfo) { let endTime = 0; for (let i = debugInfo.length - 1; i >= 0; i--) { @@ -2790,6 +2833,7 @@ function flushComponentPerformance(root: SomeChunk): number { const startTime = startTimeInfo.time; logComponentRender( componentInfo, + trackIdx, startTime, endTime, childrenEndTime, @@ -2798,7 +2842,8 @@ function flushComponentPerformance(root: SomeChunk): number { } } } - return (result.endTime = childrenEndTime); + result.endTime = childrenEndTime; + return result; } function processFullBinaryRow( diff --git a/packages/react-client/src/ReactFlightPerformanceTrack.js b/packages/react-client/src/ReactFlightPerformanceTrack.js index f1e7b30280722..b8bd9ca2be935 100644 --- a/packages/react-client/src/ReactFlightPerformanceTrack.js +++ b/packages/react-client/src/ReactFlightPerformanceTrack.js @@ -22,7 +22,8 @@ const COMPONENTS_TRACK = 'Server Components ⚛'; // Reused to avoid thrashing the GC. const reusableComponentDevToolDetails = { color: 'primary', - track: COMPONENTS_TRACK, + track: '', + trackGroup: COMPONENTS_TRACK, }; const reusableComponentOptions = { start: -0, @@ -32,13 +33,27 @@ const reusableComponentOptions = { }, }; +const trackNames = [ + 'Primary', + 'Parallel', + 'Parallel\u200b', // Padded with zero-width space to give each track a unique name. + 'Parallel\u200b\u200b', + 'Parallel\u200b\u200b\u200b', + 'Parallel\u200b\u200b\u200b\u200b', + 'Parallel\u200b\u200b\u200b\u200b\u200b', + 'Parallel\u200b\u200b\u200b\u200b\u200b\u200b', + 'Parallel\u200b\u200b\u200b\u200b\u200b\u200b\u200b', + 'Parallel\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b', +]; + export function logComponentRender( componentInfo: ReactComponentInfo, + trackIdx: number, startTime: number, endTime: number, childrenEndTime: number, ): void { - if (supportsUserTiming && childrenEndTime >= 0) { + if (supportsUserTiming && childrenEndTime >= 0 && trackIdx < 10) { const name = componentInfo.name; const selfTime = endTime - startTime; reusableComponentDevToolDetails.color = @@ -49,6 +64,7 @@ export function logComponentRender( : selfTime < 500 ? 'primary-dark' : 'error'; + reusableComponentDevToolDetails.track = trackNames[trackIdx]; reusableComponentOptions.start = startTime < 0 ? 0 : startTime; reusableComponentOptions.end = childrenEndTime; performance.measure(name, reusableComponentOptions); From 909ed63e0adc162a95a4704d3ed07a956dcf9cd1 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Mon, 16 Dec 2024 12:32:07 -0500 Subject: [PATCH 03/18] Clean up context access profiling experiment (#31806) We introduced the `unstable_useContextWithBailout` API to run compiler based experiments. This API was designed to be an experiment proxy for alternative approaches which would be heavier to implement. The experiment turned out to be inconclusive. Since most of our performance critical usage is already optimized, we weren't able to find a clear win with this approach. Since we don't have further plans for this API, let's clean it up. --- .../react-debug-tools/src/ReactDebugHooks.js | 30 +-- .../react-reconciler/src/ReactFiberHooks.js | 115 +--------- .../src/ReactFiberNewContext.js | 137 +---------- .../src/ReactInternalTypes.js | 27 +-- .../__tests__/ReactContextWithBailout-test.js | 217 ------------------ packages/react/index.fb.js | 1 - packages/react/src/ReactClient.js | 2 - packages/react/src/ReactHooks.js | 25 -- packages/shared/ReactFeatureFlags.js | 3 - .../forks/ReactFeatureFlags.native-fb.js | 1 - .../forks/ReactFeatureFlags.native-oss.js | 1 - .../forks/ReactFeatureFlags.test-renderer.js | 1 - .../ReactFeatureFlags.test-renderer.www.js | 1 - .../shared/forks/ReactFeatureFlags.www.js | 2 - scripts/error-codes/codes.json | 1 - 15 files changed, 16 insertions(+), 548 deletions(-) delete mode 100644 packages/react-reconciler/src/__tests__/ReactContextWithBailout-test.js diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index eccaa6a67b166..d2cea81f576c4 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 8affe4804b8ab..771b60cbb0894 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 190475519e1ca..6a779cfd4ab4c 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( - 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( +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/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index 2ef3a2da4f535..afe853202f141 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 c510206148b16..0000000000000 --- 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 8cded91b9854d..84128cf0ea5d9 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 90ce18b133a34..02625e3af91c5 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 40c8a23d3798b..32f49268880f0 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 d1dc6cb7f7907..8caf79fbf285d 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 498af214b0188..6c14a9ece37a0 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 fb996a43adb6c..6dce24e57fcd7 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 6646a126a0ceb..b332c485a0862 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 dae38936f8a01..20ecdb44b4717 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 060600286e6de..8054a14bbc8a6 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 65961e313b074..3ad0aa6f6006a 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." } - From 07facb52d3cad10ae9888bf3b645d5a0cb760016 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 16 Dec 2024 12:39:15 -0500 Subject: [PATCH 04/18] [Flight] Sort Server Components Track Group ahead of Client Scheduler/Components Tracks (#31736) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on #31735. This ensures that Server Components Track comes first. Since it's typically rendered first on the server for initial load and then flows into scheduler and client components work. Also puts it closer to the Network and further away from "Main" JS. Screenshot 2024-12-11 at 5 31 41 PM Same trick as in #31615. --- .../react-client/src/ReactFlightClient.js | 6 +++++- .../src/ReactFlightPerformanceTrack.js | 20 +++++++++++++++++++ .../src/ReactFiberPerformanceTrack.js | 8 ++++---- 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 0456136a7e6cf..ccb8632289f20 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -69,7 +69,10 @@ import {createBoundServerReference} from './ReactFlightReplyClient'; import {readTemporaryReference} from './ReactFlightTemporaryReferences'; -import {logComponentRender} from './ReactFlightPerformanceTrack'; +import { + markAllTracksInOrder, + logComponentRender, +} from './ReactFlightPerformanceTrack'; import { REACT_LAZY_TYPE, @@ -643,6 +646,7 @@ export function reportGlobalError(response: Response, error: Error): void { } }); if (enableProfilerTimer && enableComponentPerformanceTrack) { + markAllTracksInOrder(); flushComponentPerformance(getChunk(response, 0), 0, -Infinity); } } diff --git a/packages/react-client/src/ReactFlightPerformanceTrack.js b/packages/react-client/src/ReactFlightPerformanceTrack.js index b8bd9ca2be935..cf78f7d7de03c 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', diff --git a/packages/react-reconciler/src/ReactFiberPerformanceTrack.js b/packages/react-reconciler/src/ReactFiberPerformanceTrack.js index 7e9e246d8beb2..d94fd5ee480b3 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', From bdf187174d610eb6471bfe39f0d80ab33b68cb3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 16 Dec 2024 13:16:53 -0500 Subject: [PATCH 05/18] [Flight] Emit Deduped Server Components Marker (#31737) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on #31736. Screenshot 2024-12-11 at 8 21 12 PM This emits a placeholder when we're deduping a component. This starts when the parent's self time ends, where we would've started rendering this component if it wasn't already started. The end time is when the actual render ends since the parent is also blocked by it. --- fixtures/flight/src/App.js | 5 ++- .../react-client/src/ReactFlightClient.js | 45 +++++++++++++++++-- .../src/ReactFlightPerformanceTrack.js | 16 +++++++ 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/fixtures/flight/src/App.js b/fixtures/flight/src/App.js index 49bfc9e05135c..69fbb5e0af97d 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 ccb8632289f20..52c7ec73a4c41 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -72,6 +72,7 @@ import {readTemporaryReference} from './ReactFlightTemporaryReferences'; import { markAllTracksInOrder, logComponentRender, + logDedupedComponentRender, } from './ReactFlightPerformanceTrack'; import { @@ -130,6 +131,7 @@ export type JSONValue = type ProfilingResult = { track: number, endTime: number, + component: null | ReactComponentInfo, }; const ROW_ID = 0; @@ -647,7 +649,7 @@ export function reportGlobalError(response: Response, error: Error): void { }); if (enableProfilerTimer && enableComponentPerformanceTrack) { markAllTracksInOrder(); - flushComponentPerformance(getChunk(response, 0), 0, -Infinity); + flushComponentPerformance(getChunk(response, 0), 0, -Infinity, -Infinity); } } @@ -2748,7 +2750,8 @@ function resolveTypedArray( function flushComponentPerformance( 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 @@ -2765,6 +2768,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; @@ -2792,15 +2811,27 @@ 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; @@ -2810,7 +2841,11 @@ function flushComponentPerformance( children[i], childTrackIdx, childTrackTime, + parentEndTime, ); + if (childResult.component !== null) { + result.component = childResult.component; + } childTrackIdx = childResult.track; const childEndTime = childResult.endTime; childTrackTime = childEndTime; @@ -2842,6 +2877,8 @@ function flushComponentPerformance( endTime, childrenEndTime, ); + // 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 cf78f7d7de03c..cfd8ca96b3359 100644 --- a/packages/react-client/src/ReactFlightPerformanceTrack.js +++ b/packages/react-client/src/ReactFlightPerformanceTrack.js @@ -90,3 +90,19 @@ export function logComponentRender( performance.measure(name, 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; + performance.measure(name + ' [deduped]', reusableComponentOptions); + } +} From 54e86bd0d0eac76320c8810090810e7a858125d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 16 Dec 2024 13:39:19 -0500 Subject: [PATCH 06/18] [Flight] Color and badge non-primary environments (#31738) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on #31737. Screenshot 2024-12-11 at 8 41 15 PM When mixing environments (like "use cache" or third party RSC) it's useful to color and badge those components differently to differentiate. I'm not putting them in separate tracks because when they do actually execute, like cache misses or third party RSCs, they behave like they're part of the same tree. --- .../react-client/src/ReactFlightClient.js | 11 +++++++++- .../src/ReactFlightPerformanceTrack.js | 22 ++++++++++++++----- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 52c7ec73a4c41..8e00df7c1c2e9 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -649,7 +649,13 @@ export function reportGlobalError(response: Response, error: Error): void { }); if (enableProfilerTimer && enableComponentPerformanceTrack) { markAllTracksInOrder(); - flushComponentPerformance(getChunk(response, 0), 0, -Infinity, -Infinity); + flushComponentPerformance( + response, + getChunk(response, 0), + 0, + -Infinity, + -Infinity, + ); } } @@ -2748,6 +2754,7 @@ function resolveTypedArray( } function flushComponentPerformance( + response: Response, root: SomeChunk, trackIdx: number, // Next available track trackTime: number, // The time after which it is available, @@ -2838,6 +2845,7 @@ function flushComponentPerformance( let childTrackTime = trackTime; for (let i = 0; i < children.length; i++) { const childResult = flushComponentPerformance( + response, children[i], childTrackIdx, childTrackTime, @@ -2876,6 +2884,7 @@ 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 cfd8ca96b3359..d2860e407cc65 100644 --- a/packages/react-client/src/ReactFlightPerformanceTrack.js +++ b/packages/react-client/src/ReactFlightPerformanceTrack.js @@ -72,22 +72,33 @@ 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); } } @@ -103,6 +114,7 @@ export function logDedupedComponentRender( reusableComponentDevToolDetails.track = trackNames[trackIdx]; reusableComponentOptions.start = startTime < 0 ? 0 : startTime; reusableComponentOptions.end = endTime; - performance.measure(name + ' [deduped]', reusableComponentOptions); + const entryName = name + ' [deduped]'; + performance.measure(entryName, reusableComponentOptions); } } From e30872a4e01bdc0cf185a818156ae7741c815e21 Mon Sep 17 00:00:00 2001 From: mofeiZ <34200447+mofeiZ@users.noreply.github.com> Date: Mon, 16 Dec 2024 14:43:21 -0500 Subject: [PATCH 07/18] [compiler][be] Playground now compiles entire program (#31774) Compiler playground now runs the entire program through `babel-plugin-react-compiler` instead of a custom pipeline which previously duplicated function inference logic from `Program.ts`. In addition, the playground output reflects the tranformed file (instead of a "virtual file" of manually concatenated functions). This helps with the following: - Reduce potential discrepencies between playground and babel plugin behavior. See attached fixture output for an example where we previously diverged. - Let playground users see compiler-inserted imports (e.g. `_c` or `useFire`) This also helps us repurpose playground into a more general tool for compiler-users instead of just for compiler engineers. - imports and other functions are preserved. We differentiate between imports and globals in many cases (e.g. `inferEffectDeps`), so it may be misleading to omit imports in printed output - playground now shows other program-changing behavior like position of outlined functions and hoisted declarations - emitted compiled functions do not need synthetic names --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31774). * #31809 * __->__ #31774 --- .../page.spec.ts/01-user-output.txt | 3 +- .../page.spec.ts/02-default-output.txt | 3 +- .../module-scope-use-memo-output.txt | 4 +- .../module-scope-use-no-memo-output.txt | 3 +- .../page.spec.ts/parse-flow-output.txt | 14 + .../page.spec.ts/parse-typescript-output.txt | 20 ++ ...cope-does-not-beat-module-scope-output.txt | 5 + .../page.spec.ts/use-memo-output.txt | 5 +- .../page.spec.ts/use-no-memo-output.txt | 8 +- .../playground/__tests__/e2e/page.spec.ts | 41 ++- .../components/Editor/EditorImpl.tsx | 260 ++++-------------- .../playground/components/Editor/Output.tsx | 63 ++--- .../src/Babel/BabelPlugin.ts | 5 +- .../src/Entrypoint/Options.ts | 2 + .../src/Entrypoint/Pipeline.ts | 141 +++++----- .../src/HIR/Environment.ts | 29 +- ...ule-scope-usememo-function-scope.expect.md | 29 ++ ...emo-module-scope-usememo-function-scope.js | 7 + .../src/__tests__/parseConfigPragma-test.ts | 1 + .../babel-plugin-react-compiler/src/index.ts | 2 - 20 files changed, 302 insertions(+), 343 deletions(-) create mode 100644 compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/parse-flow-output.txt create mode 100644 compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/parse-typescript-output.txt create mode 100644 compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/todo-function-scope-does-not-beat-module-scope-output.txt create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-memo-module-scope-usememo-function-scope.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-memo-module-scope-usememo-function-scope.js diff --git a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/01-user-output.txt b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/01-user-output.txt index ba680bbb57232..1600f35107339 100644 --- a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/01-user-output.txt +++ b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/01-user-output.txt @@ -1,4 +1,5 @@ -function TestComponent(t0) { +import { c as _c } from "react/compiler-runtime"; +export default function TestComponent(t0) { const $ = _c(2); const { x } = t0; let t1; diff --git a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/02-default-output.txt b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/02-default-output.txt index 2cbd09bba6179..1d59a120f9849 100644 --- a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/02-default-output.txt +++ b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/02-default-output.txt @@ -1,4 +1,5 @@ -function MyApp() { +import { c as _c } from "react/compiler-runtime"; +export default function MyApp() { const $ = _c(1); let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { diff --git a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/module-scope-use-memo-output.txt b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/module-scope-use-memo-output.txt index ba680bbb57232..638a2bcd22841 100644 --- a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/module-scope-use-memo-output.txt +++ b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/module-scope-use-memo-output.txt @@ -1,4 +1,6 @@ -function TestComponent(t0) { +"use memo"; +import { c as _c } from "react/compiler-runtime"; +export default function TestComponent(t0) { const $ = _c(2); const { x } = t0; let t1; diff --git a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/module-scope-use-no-memo-output.txt b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/module-scope-use-no-memo-output.txt index 2c69ddc1d65b8..ebd2d2b04678c 100644 --- a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/module-scope-use-no-memo-output.txt +++ b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/module-scope-use-no-memo-output.txt @@ -1,3 +1,4 @@ -function TestComponent({ x }) { +"use no memo"; +export default function TestComponent({ x }) { return ; } diff --git a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/parse-flow-output.txt b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/parse-flow-output.txt new file mode 100644 index 0000000000000..c0907856babf7 --- /dev/null +++ b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/parse-flow-output.txt @@ -0,0 +1,14 @@ +import { c as _c } from "react/compiler-runtime"; +function useFoo(propVal) { +  const $ = _c(2); +  const t0 = (propVal.baz: number); +  let t1; +  if ($[0] !== t0) { +    t1 = 
{t0}
; +    $[0] = t0; +    $[1] = t1; +  } else { +    t1 = $[1]; +  } +  return t1; +} \ No newline at end of file diff --git a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/parse-typescript-output.txt b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/parse-typescript-output.txt new file mode 100644 index 0000000000000..c936c5ae2ed0a --- /dev/null +++ b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/parse-typescript-output.txt @@ -0,0 +1,20 @@ +import { c as _c } from "react/compiler-runtime"; +function Foo() { +  const $ = _c(2); +  let t0; +  if ($[0] === Symbol.for("react.memo_cache_sentinel")) { +    t0 = foo(); +    $[0] = t0; +  } else { +    t0 = $[0]; +  } +  const x = t0 as number; +  let t1; +  if ($[1] === Symbol.for("react.memo_cache_sentinel")) { +    t1 = 
{x}
; +    $[1] = t1; +  } else { +    t1 = $[1]; +  } +  return t1; +} \ No newline at end of file diff --git a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/todo-function-scope-does-not-beat-module-scope-output.txt b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/todo-function-scope-does-not-beat-module-scope-output.txt new file mode 100644 index 0000000000000..325e6972e1514 --- /dev/null +++ b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/todo-function-scope-does-not-beat-module-scope-output.txt @@ -0,0 +1,5 @@ +"use no memo"; +function TestComponent({ x }) { + "use memo"; + return ; +} diff --git a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/use-memo-output.txt b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/use-memo-output.txt index 804bacab97e05..de6dd52680077 100644 --- a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/use-memo-output.txt +++ b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/use-memo-output.txt @@ -1,3 +1,4 @@ +import { c as _c } from "react/compiler-runtime"; function TestComponent(t0) { "use memo"; const $ = _c(2); @@ -12,7 +13,7 @@ function TestComponent(t0) { } return t1; } -function anonymous_1(t0) { +const TestComponent2 = (t0) => { "use memo"; const $ = _c(2); const { x } = t0; @@ -25,4 +26,4 @@ function anonymous_1(t0) { t1 = $[1]; } return t1; -} +}; diff --git a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/use-no-memo-output.txt b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/use-no-memo-output.txt index 5fb66309fc70c..02c1367622185 100644 --- a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/use-no-memo-output.txt +++ b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/use-no-memo-output.txt @@ -1,8 +1,8 @@ -function anonymous_1() { +const TestComponent = function () { "use no memo"; return ; -} -function anonymous_3({ x }) { +}; +const TestComponent2 = ({ x }) => { "use no memo"; return ; -} +}; diff --git a/compiler/apps/playground/__tests__/e2e/page.spec.ts b/compiler/apps/playground/__tests__/e2e/page.spec.ts index 846e6227bd1a1..05fe96d4b9048 100644 --- a/compiler/apps/playground/__tests__/e2e/page.spec.ts +++ b/compiler/apps/playground/__tests__/e2e/page.spec.ts @@ -9,11 +9,11 @@ import {expect, test} from '@playwright/test'; import {encodeStore, type Store} from '../../lib/stores'; import {format} from 'prettier'; -function print(data: Array): Promise { +function formatPrint(data: Array): Promise { return format(data.join(''), {parser: 'babel'}); } -const DIRECTIVE_TEST_CASES = [ +const TEST_CASE_INPUTS = [ { name: 'module-scope-use-memo', input: ` @@ -55,7 +55,7 @@ const TestComponent2 = ({ x }) => { };`, }, { - name: 'function-scope-beats-module-scope', + name: 'todo-function-scope-does-not-beat-module-scope', input: ` 'use no memo'; function TestComponent({ x }) { @@ -63,6 +63,26 @@ function TestComponent({ x }) { return ; }`, }, + { + name: 'parse-typescript', + input: ` +function Foo() { + const x = foo() as number; + return
{x}
; +} +`, + noFormat: true, + }, + { + name: 'parse-flow', + input: ` +// @flow +function useFoo(propVal: {+baz: number}) { + return
{(propVal.baz as number)}
; +} + `, + noFormat: true, + }, ]; test('editor should open successfully', async ({page}) => { @@ -90,7 +110,7 @@ test('editor should compile from hash successfully', async ({page}) => { }); const text = (await page.locator('.monaco-editor').nth(1).allInnerTexts()) ?? []; - const output = await print(text); + const output = await formatPrint(text); expect(output).not.toEqual(''); expect(output).toMatchSnapshot('01-user-output.txt'); @@ -115,14 +135,14 @@ test('reset button works', async ({page}) => { }); const text = (await page.locator('.monaco-editor').nth(1).allInnerTexts()) ?? []; - const output = await print(text); + const output = await formatPrint(text); expect(output).not.toEqual(''); expect(output).toMatchSnapshot('02-default-output.txt'); }); -DIRECTIVE_TEST_CASES.forEach((t, idx) => - test(`directives work: ${t.name}`, async ({page}) => { +TEST_CASE_INPUTS.forEach((t, idx) => + test(`playground compiles: ${t.name}`, async ({page}) => { const store: Store = { source: t.input, }; @@ -135,7 +155,12 @@ DIRECTIVE_TEST_CASES.forEach((t, idx) => const text = (await page.locator('.monaco-editor').nth(1).allInnerTexts()) ?? []; - const output = await print(text); + let output: string; + if (t.noFormat) { + output = text.join(''); + } else { + output = await formatPrint(text); + } expect(output).not.toEqual(''); expect(output).toMatchSnapshot(`${t.name}-output.txt`); diff --git a/compiler/apps/playground/components/Editor/EditorImpl.tsx b/compiler/apps/playground/components/Editor/EditorImpl.tsx index 82a40272bd312..785b9fd075d12 100644 --- a/compiler/apps/playground/components/Editor/EditorImpl.tsx +++ b/compiler/apps/playground/components/Editor/EditorImpl.tsx @@ -5,23 +5,22 @@ * LICENSE file in the root directory of this source tree. */ -import {parse as babelParse} from '@babel/parser'; +import {parse as babelParse, ParseResult} from '@babel/parser'; import * as HermesParser from 'hermes-parser'; -import traverse, {NodePath} from '@babel/traverse'; import * as t from '@babel/types'; -import { +import BabelPluginReactCompiler, { CompilerError, CompilerErrorDetail, Effect, ErrorSeverity, parseConfigPragmaForTests, ValueKind, - runPlayground, type Hook, - findDirectiveDisablingMemoization, - findDirectiveEnablingMemoization, + PluginOptions, + CompilerPipelineValue, + parsePluginOptions, } from 'babel-plugin-react-compiler/src'; -import {type ReactFunctionType} from 'babel-plugin-react-compiler/src/HIR/Environment'; +import {type EnvironmentConfig} from 'babel-plugin-react-compiler/src/HIR/Environment'; import clsx from 'clsx'; import invariant from 'invariant'; import {useSnackbar} from 'notistack'; @@ -39,32 +38,18 @@ import {useStore, useStoreDispatch} from '../StoreContext'; import Input from './Input'; import { CompilerOutput, + CompilerTransformOutput, default as Output, PrintedCompilerPipelineValue, } from './Output'; import {printFunctionWithOutlined} from 'babel-plugin-react-compiler/src/HIR/PrintHIR'; import {printReactiveFunctionWithOutlined} from 'babel-plugin-react-compiler/src/ReactiveScopes/PrintReactiveFunction'; +import {transformFromAstSync} from '@babel/core'; -type FunctionLike = - | NodePath - | NodePath - | NodePath; -enum MemoizeDirectiveState { - Enabled = 'Enabled', - Disabled = 'Disabled', - Undefined = 'Undefined', -} - -const MEMOIZE_ENABLED_OR_UNDEFINED_STATES = new Set([ - MemoizeDirectiveState.Enabled, - MemoizeDirectiveState.Undefined, -]); - -const MEMOIZE_ENABLED_OR_DISABLED_STATES = new Set([ - MemoizeDirectiveState.Enabled, - MemoizeDirectiveState.Disabled, -]); -function parseInput(input: string, language: 'flow' | 'typescript'): any { +function parseInput( + input: string, + language: 'flow' | 'typescript', +): ParseResult { // Extract the first line to quickly check for custom test directives if (language === 'flow') { return HermesParser.parse(input, { @@ -77,95 +62,45 @@ function parseInput(input: string, language: 'flow' | 'typescript'): any { return babelParse(input, { plugins: ['typescript', 'jsx'], sourceType: 'module', - }); + }) as ParseResult; } } -function parseFunctions( +function invokeCompiler( source: string, language: 'flow' | 'typescript', -): Array<{ - compilationEnabled: boolean; - fn: FunctionLike; -}> { - const items: Array<{ - compilationEnabled: boolean; - fn: FunctionLike; - }> = []; - try { - const ast = parseInput(source, language); - traverse(ast, { - FunctionDeclaration(nodePath) { - items.push({ - compilationEnabled: shouldCompile(nodePath), - fn: nodePath, - }); - nodePath.skip(); - }, - ArrowFunctionExpression(nodePath) { - items.push({ - compilationEnabled: shouldCompile(nodePath), - fn: nodePath, - }); - nodePath.skip(); - }, - FunctionExpression(nodePath) { - items.push({ - compilationEnabled: shouldCompile(nodePath), - fn: nodePath, - }); - nodePath.skip(); - }, - }); - } catch (e) { - console.error(e); - CompilerError.throwInvalidJS({ - reason: String(e), - description: null, - loc: null, - suggestions: null, - }); - } - - return items; -} - -function shouldCompile(fn: FunctionLike): boolean { - const {body} = fn.node; - if (t.isBlockStatement(body)) { - const selfCheck = checkExplicitMemoizeDirectives(body.directives); - if (selfCheck === MemoizeDirectiveState.Enabled) return true; - if (selfCheck === MemoizeDirectiveState.Disabled) return false; - - const parentWithDirective = fn.findParent(parentPath => { - if (parentPath.isBlockStatement() || parentPath.isProgram()) { - const directiveCheck = checkExplicitMemoizeDirectives( - parentPath.node.directives, - ); - return MEMOIZE_ENABLED_OR_DISABLED_STATES.has(directiveCheck); - } - return false; - }); - - if (!parentWithDirective) return true; - const parentDirectiveCheck = checkExplicitMemoizeDirectives( - (parentWithDirective.node as t.Program | t.BlockStatement).directives, - ); - return MEMOIZE_ENABLED_OR_UNDEFINED_STATES.has(parentDirectiveCheck); - } - return false; -} - -function checkExplicitMemoizeDirectives( - directives: Array, -): MemoizeDirectiveState { - if (findDirectiveEnablingMemoization(directives).length) { - return MemoizeDirectiveState.Enabled; - } - if (findDirectiveDisablingMemoization(directives).length) { - return MemoizeDirectiveState.Disabled; + environment: EnvironmentConfig, + logIR: (pipelineValue: CompilerPipelineValue) => void, +): CompilerTransformOutput { + const opts: PluginOptions = parsePluginOptions({ + logger: { + debugLogIRs: logIR, + logEvent: () => {}, + }, + environment, + compilationMode: 'all', + panicThreshold: 'all_errors', + }); + const ast = parseInput(source, language); + let result = transformFromAstSync(ast, source, { + filename: '_playgroundFile.js', + highlightCode: false, + retainLines: true, + plugins: [[BabelPluginReactCompiler, opts]], + ast: true, + sourceType: 'module', + configFile: false, + sourceMaps: true, + babelrc: false, + }); + if (result?.ast == null || result?.code == null || result?.map == null) { + throw new Error('Expected successful compilation'); } - return MemoizeDirectiveState.Undefined; + return { + code: result.code, + sourceMaps: result.map, + language, + }; } const COMMON_HOOKS: Array<[string, Hook]> = [ @@ -216,37 +151,6 @@ const COMMON_HOOKS: Array<[string, Hook]> = [ ], ]; -function isHookName(s: string): boolean { - return /^use[A-Z0-9]/.test(s); -} - -function getReactFunctionType(id: t.Identifier | null): ReactFunctionType { - if (id != null) { - if (isHookName(id.name)) { - return 'Hook'; - } - - const isPascalCaseNameSpace = /^[A-Z].*/; - if (isPascalCaseNameSpace.test(id.name)) { - return 'Component'; - } - } - return 'Other'; -} - -function getFunctionIdentifier( - fn: - | NodePath - | NodePath - | NodePath, -): t.Identifier | null { - if (fn.isArrowFunctionExpression()) { - return null; - } - const id = fn.get('id'); - return Array.isArray(id) === false && id.isIdentifier() ? id.node : null; -} - function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] { const results = new Map>(); const error = new CompilerError(); @@ -264,71 +168,25 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] { } else { language = 'typescript'; } - let count = 0; - const withIdentifier = (id: t.Identifier | null): t.Identifier => { - if (id != null && id.name != null) { - return id; - } else { - return t.identifier(`anonymous_${count++}`); - } - }; + let transformOutput; try { // Extract the first line to quickly check for custom test directives const pragma = source.substring(0, source.indexOf('\n')); const config = parseConfigPragmaForTests(pragma); - const parsedFunctions = parseFunctions(source, language); - for (const func of parsedFunctions) { - const id = withIdentifier(getFunctionIdentifier(func.fn)); - const fnName = id.name; - if (!func.compilationEnabled) { - upsert({ - kind: 'ast', - fnName, - name: 'CodeGen', - value: { - type: 'FunctionDeclaration', - id: - func.fn.isArrowFunctionExpression() || - func.fn.isFunctionExpression() - ? withIdentifier(null) - : func.fn.node.id, - async: func.fn.node.async, - generator: !!func.fn.node.generator, - body: func.fn.node.body as t.BlockStatement, - params: func.fn.node.params, - }, - }); - continue; - } - for (const result of runPlayground( - func.fn, - { - ...config, - customHooks: new Map([...COMMON_HOOKS]), - }, - getReactFunctionType(id), - )) { + + transformOutput = invokeCompiler( + source, + language, + {...config, customHooks: new Map([...COMMON_HOOKS])}, + result => { switch (result.kind) { case 'ast': { - upsert({ - kind: 'ast', - fnName, - name: result.name, - value: { - type: 'FunctionDeclaration', - id: withIdentifier(result.value.id), - async: result.value.async, - generator: result.value.generator, - body: result.value.body, - params: result.value.params, - }, - }); break; } case 'hir': { upsert({ kind: 'hir', - fnName, + fnName: result.value.id, name: result.name, value: printFunctionWithOutlined(result.value), }); @@ -337,7 +195,7 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] { case 'reactive': { upsert({ kind: 'reactive', - fnName, + fnName: result.value.id, name: result.name, value: printReactiveFunctionWithOutlined(result.value), }); @@ -346,7 +204,7 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] { case 'debug': { upsert({ kind: 'debug', - fnName, + fnName: null, name: result.name, value: result.value, }); @@ -357,8 +215,8 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] { throw new Error(`Unhandled result ${result}`); } } - } - } + }, + ); } catch (err) { /** * error might be an invariant violation or other runtime error @@ -385,7 +243,7 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] { if (error.hasErrors()) { return [{kind: 'err', results, error: error}, language]; } - return [{kind: 'ok', results}, language]; + return [{kind: 'ok', results, transformOutput}, language]; } export default function Editor(): JSX.Element { @@ -405,7 +263,7 @@ export default function Editor(): JSX.Element { } catch (e) { invariant(e instanceof Error, 'Only Error may be caught.'); enqueueSnackbar(e.message, { - variant: 'message', + variant: 'warning', ...createMessage( 'Bad URL - fell back to the default Playground.', MessageLevel.Info, diff --git a/compiler/apps/playground/components/Editor/Output.tsx b/compiler/apps/playground/components/Editor/Output.tsx index 97a63400d2b42..d4127c63cff05 100644 --- a/compiler/apps/playground/components/Editor/Output.tsx +++ b/compiler/apps/playground/components/Editor/Output.tsx @@ -5,8 +5,6 @@ * LICENSE file in the root directory of this source tree. */ -import generate from '@babel/generator'; -import * as t from '@babel/types'; import { CodeIcon, DocumentAddIcon, @@ -21,17 +19,12 @@ import {memo, ReactNode, useEffect, useState} from 'react'; import {type Store} from '../../lib/stores'; import TabbedWindow from '../TabbedWindow'; import {monacoOptions} from './monacoOptions'; +import {BabelFileResult} from '@babel/core'; const MemoizedOutput = memo(Output); export default MemoizedOutput; export type PrintedCompilerPipelineValue = - | { - kind: 'ast'; - name: string; - fnName: string | null; - value: t.FunctionDeclaration; - } | { kind: 'hir'; name: string; @@ -41,8 +34,17 @@ export type PrintedCompilerPipelineValue = | {kind: 'reactive'; name: string; fnName: string | null; value: string} | {kind: 'debug'; name: string; fnName: string | null; value: string}; +export type CompilerTransformOutput = { + code: string; + sourceMaps: BabelFileResult['map']; + language: 'flow' | 'typescript'; +}; export type CompilerOutput = - | {kind: 'ok'; results: Map>} + | { + kind: 'ok'; + transformOutput: CompilerTransformOutput; + results: Map>; + } | { kind: 'err'; results: Map>; @@ -61,7 +63,6 @@ async function tabify( const tabs = new Map(); const reorderedTabs = new Map(); const concattedResults = new Map(); - let topLevelFnDecls: Array = []; // Concat all top level function declaration results into a single tab for each pass for (const [passName, results] of compilerOutput.results) { for (const result of results) { @@ -87,9 +88,6 @@ async function tabify( } break; } - case 'ast': - topLevelFnDecls.push(result.value); - break; case 'debug': { concattedResults.set(passName, result.value); break; @@ -114,13 +112,17 @@ async function tabify( lastPassOutput = text; } // Ensure that JS and the JS source map come first - if (topLevelFnDecls.length > 0) { - /** - * Make a synthetic Program so we can have a single AST with all the top level - * FunctionDeclarations - */ - const ast = t.program(topLevelFnDecls); - const {code, sourceMapUrl} = await codegen(ast, source); + if (compilerOutput.kind === 'ok') { + const {transformOutput} = compilerOutput; + const sourceMapUrl = getSourceMapUrl( + transformOutput.code, + JSON.stringify(transformOutput.sourceMaps), + ); + const code = await prettier.format(transformOutput.code, { + semi: true, + parser: transformOutput.language === 'flow' ? 'babel-flow' : 'babel-ts', + plugins: [parserBabel, prettierPluginEstree], + }); reorderedTabs.set( 'JS', { - const generated = generate( - ast, - {sourceMaps: true, sourceFileName: 'input.js'}, - source, - ); - const sourceMapUrl = getSourceMapUrl( - generated.code, - JSON.stringify(generated.map), - ); - const codegenOutput = await prettier.format(generated.code, { - semi: true, - parser: 'babel', - plugins: [parserBabel, prettierPluginEstree], - }); - return {code: codegenOutput, sourceMapUrl}; -} - function utf16ToUTF8(s: string): string { return unescape(encodeURIComponent(s)); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts b/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts index 401cbd4bdf50f..c648c66043980 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Babel/BabelPlugin.ts @@ -39,7 +39,10 @@ export default function BabelPluginReactCompiler( ) { opts = injectReanimatedFlag(opts); } - if (isDev) { + if ( + opts.environment.enableResetCacheOnSourceFileChanges !== false && + isDev + ) { opts = { ...opts, environment: { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Options.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Options.ts index 72ed9e7c866ce..fb951d25c5398 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Options.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Options.ts @@ -15,6 +15,7 @@ import { } from '../HIR/Environment'; import {hasOwnProperty} from '../Utils/utils'; import {fromZodError} from 'zod-validation-error'; +import {CompilerPipelineValue} from './Pipeline'; const PanicThresholdOptionsSchema = z.enum([ /* @@ -209,6 +210,7 @@ export type LoggerEvent = export type Logger = { logEvent: (filename: string | null, event: LoggerEvent) => void; + debugLogIRs?: (value: CompilerPipelineValue) => void; }; export const defaultOptions: PluginOptions = { 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 8a97eea217b33..6995aec7f1825 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -112,7 +112,7 @@ export type CompilerPipelineValue = | {kind: 'reactive'; name: string; value: ReactiveFunction} | {kind: 'debug'; name: string; value: string}; -export function* run( +function run( func: NodePath< t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression >, @@ -122,7 +122,7 @@ export function* run( logger: Logger | null, filename: string | null, code: string | null, -): Generator { +): CodegenFunction { const contextIdentifiers = findContextIdentifiers(func); const env = new Environment( func.scope, @@ -134,12 +134,17 @@ export function* run( code, useMemoCacheIdentifier, ); - yield log({ + env.logger?.debugLogIRs?.({ + kind: 'debug', + name: 'EnvironmentConfig', + value: prettyFormat(env.config), + }); + printLog({ kind: 'debug', name: 'EnvironmentConfig', value: prettyFormat(env.config), }); - const ast = yield* runWithEnvironment(func, env); + const ast = runWithEnvironment(func, env); return ast; } @@ -147,17 +152,22 @@ export function* run( * Note: this is split from run() to make `config` out of scope, so that all * access to feature flags has to be through the Environment for consistency. */ -function* runWithEnvironment( +function runWithEnvironment( func: NodePath< t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression >, env: Environment, -): Generator { +): CodegenFunction { + const log = (value: CompilerPipelineValue): CompilerPipelineValue => { + printLog(value); + env.logger?.debugLogIRs?.(value); + return value; + }; const hir = lower(func, env).unwrap(); - yield log({kind: 'hir', name: 'HIR', value: hir}); + log({kind: 'hir', name: 'HIR', value: hir}); pruneMaybeThrows(hir); - yield log({kind: 'hir', name: 'PruneMaybeThrows', value: hir}); + log({kind: 'hir', name: 'PruneMaybeThrows', value: hir}); validateContextVariableLValues(hir); validateUseMemo(hir); @@ -168,35 +178,35 @@ function* runWithEnvironment( !env.config.enableChangeDetectionForDebugging ) { dropManualMemoization(hir); - yield log({kind: 'hir', name: 'DropManualMemoization', value: hir}); + log({kind: 'hir', name: 'DropManualMemoization', value: hir}); } inlineImmediatelyInvokedFunctionExpressions(hir); - yield log({ + log({ kind: 'hir', name: 'InlineImmediatelyInvokedFunctionExpressions', value: hir, }); mergeConsecutiveBlocks(hir); - yield log({kind: 'hir', name: 'MergeConsecutiveBlocks', value: hir}); + log({kind: 'hir', name: 'MergeConsecutiveBlocks', value: hir}); assertConsistentIdentifiers(hir); assertTerminalSuccessorsExist(hir); enterSSA(hir); - yield log({kind: 'hir', name: 'SSA', value: hir}); + log({kind: 'hir', name: 'SSA', value: hir}); eliminateRedundantPhi(hir); - yield log({kind: 'hir', name: 'EliminateRedundantPhi', value: hir}); + log({kind: 'hir', name: 'EliminateRedundantPhi', value: hir}); assertConsistentIdentifiers(hir); constantPropagation(hir); - yield log({kind: 'hir', name: 'ConstantPropagation', value: hir}); + log({kind: 'hir', name: 'ConstantPropagation', value: hir}); inferTypes(hir); - yield log({kind: 'hir', name: 'InferTypes', value: hir}); + log({kind: 'hir', name: 'InferTypes', value: hir}); if (env.config.validateHooksUsage) { validateHooksUsage(hir); @@ -211,30 +221,30 @@ function* runWithEnvironment( } optimizePropsMethodCalls(hir); - yield log({kind: 'hir', name: 'OptimizePropsMethodCalls', value: hir}); + log({kind: 'hir', name: 'OptimizePropsMethodCalls', value: hir}); analyseFunctions(hir); - yield log({kind: 'hir', name: 'AnalyseFunctions', value: hir}); + log({kind: 'hir', name: 'AnalyseFunctions', value: hir}); inferReferenceEffects(hir); - yield log({kind: 'hir', name: 'InferReferenceEffects', value: hir}); + log({kind: 'hir', name: 'InferReferenceEffects', value: hir}); validateLocalsNotReassignedAfterRender(hir); // Note: Has to come after infer reference effects because "dead" code may still affect inference deadCodeElimination(hir); - yield log({kind: 'hir', name: 'DeadCodeElimination', value: hir}); + log({kind: 'hir', name: 'DeadCodeElimination', value: hir}); if (env.config.enableInstructionReordering) { instructionReordering(hir); - yield log({kind: 'hir', name: 'InstructionReordering', value: hir}); + log({kind: 'hir', name: 'InstructionReordering', value: hir}); } pruneMaybeThrows(hir); - yield log({kind: 'hir', name: 'PruneMaybeThrows', value: hir}); + log({kind: 'hir', name: 'PruneMaybeThrows', value: hir}); inferMutableRanges(hir); - yield log({kind: 'hir', name: 'InferMutableRanges', value: hir}); + log({kind: 'hir', name: 'InferMutableRanges', value: hir}); if (env.config.assertValidMutableRanges) { assertValidMutableRanges(hir); @@ -257,27 +267,27 @@ function* runWithEnvironment( } inferReactivePlaces(hir); - yield log({kind: 'hir', name: 'InferReactivePlaces', value: hir}); + log({kind: 'hir', name: 'InferReactivePlaces', value: hir}); rewriteInstructionKindsBasedOnReassignment(hir); - yield log({ + log({ kind: 'hir', name: 'RewriteInstructionKindsBasedOnReassignment', value: hir, }); propagatePhiTypes(hir); - yield log({ + log({ kind: 'hir', name: 'PropagatePhiTypes', value: hir, }); inferReactiveScopeVariables(hir); - yield log({kind: 'hir', name: 'InferReactiveScopeVariables', value: hir}); + log({kind: 'hir', name: 'InferReactiveScopeVariables', value: hir}); const fbtOperands = memoizeFbtAndMacroOperandsInSameScope(hir); - yield log({ + log({ kind: 'hir', name: 'MemoizeFbtAndMacroOperandsInSameScope', value: hir, @@ -289,39 +299,39 @@ function* runWithEnvironment( if (env.config.enableFunctionOutlining) { outlineFunctions(hir, fbtOperands); - yield log({kind: 'hir', name: 'OutlineFunctions', value: hir}); + log({kind: 'hir', name: 'OutlineFunctions', value: hir}); } alignMethodCallScopes(hir); - yield log({ + log({ kind: 'hir', name: 'AlignMethodCallScopes', value: hir, }); alignObjectMethodScopes(hir); - yield log({ + log({ kind: 'hir', name: 'AlignObjectMethodScopes', value: hir, }); pruneUnusedLabelsHIR(hir); - yield log({ + log({ kind: 'hir', name: 'PruneUnusedLabelsHIR', value: hir, }); alignReactiveScopesToBlockScopesHIR(hir); - yield log({ + log({ kind: 'hir', name: 'AlignReactiveScopesToBlockScopesHIR', value: hir, }); mergeOverlappingReactiveScopesHIR(hir); - yield log({ + log({ kind: 'hir', name: 'MergeOverlappingReactiveScopesHIR', value: hir, @@ -329,7 +339,7 @@ function* runWithEnvironment( assertValidBlockNesting(hir); buildReactiveScopeTerminalsHIR(hir); - yield log({ + log({ kind: 'hir', name: 'BuildReactiveScopeTerminalsHIR', value: hir, @@ -338,14 +348,14 @@ function* runWithEnvironment( assertValidBlockNesting(hir); flattenReactiveLoopsHIR(hir); - yield log({ + log({ kind: 'hir', name: 'FlattenReactiveLoopsHIR', value: hir, }); flattenScopesWithHooksOrUseHIR(hir); - yield log({ + log({ kind: 'hir', name: 'FlattenScopesWithHooksOrUseHIR', value: hir, @@ -353,7 +363,7 @@ function* runWithEnvironment( assertTerminalSuccessorsExist(hir); assertTerminalPredsExist(hir); propagateScopeDependenciesHIR(hir); - yield log({ + log({ kind: 'hir', name: 'PropagateScopeDependenciesHIR', value: hir, @@ -365,7 +375,7 @@ function* runWithEnvironment( if (env.config.inlineJsxTransform) { inlineJsxTransform(hir, env.config.inlineJsxTransform); - yield log({ + log({ kind: 'hir', name: 'inlineJsxTransform', value: hir, @@ -373,7 +383,7 @@ function* runWithEnvironment( } const reactiveFunction = buildReactiveFunction(hir); - yield log({ + log({ kind: 'reactive', name: 'BuildReactiveFunction', value: reactiveFunction, @@ -382,7 +392,7 @@ function* runWithEnvironment( assertWellFormedBreakTargets(reactiveFunction); pruneUnusedLabels(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'PruneUnusedLabels', value: reactiveFunction, @@ -390,35 +400,35 @@ function* runWithEnvironment( assertScopeInstructionsWithinScopes(reactiveFunction); pruneNonEscapingScopes(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'PruneNonEscapingScopes', value: reactiveFunction, }); pruneNonReactiveDependencies(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'PruneNonReactiveDependencies', value: reactiveFunction, }); pruneUnusedScopes(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'PruneUnusedScopes', value: reactiveFunction, }); mergeReactiveScopesThatInvalidateTogether(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'MergeReactiveScopesThatInvalidateTogether', value: reactiveFunction, }); pruneAlwaysInvalidatingScopes(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'PruneAlwaysInvalidatingScopes', value: reactiveFunction, @@ -426,7 +436,7 @@ function* runWithEnvironment( if (env.config.enableChangeDetectionForDebugging != null) { pruneInitializationDependencies(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'PruneInitializationDependencies', value: reactiveFunction, @@ -434,49 +444,49 @@ function* runWithEnvironment( } propagateEarlyReturns(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'PropagateEarlyReturns', value: reactiveFunction, }); pruneUnusedLValues(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'PruneUnusedLValues', value: reactiveFunction, }); promoteUsedTemporaries(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'PromoteUsedTemporaries', value: reactiveFunction, }); extractScopeDeclarationsFromDestructuring(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'ExtractScopeDeclarationsFromDestructuring', value: reactiveFunction, }); stabilizeBlockIds(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'StabilizeBlockIds', value: reactiveFunction, }); const uniqueIdentifiers = renameVariables(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'RenameVariables', value: reactiveFunction, }); pruneHoistedContexts(reactiveFunction); - yield log({ + log({ kind: 'reactive', name: 'PruneHoistedContexts', value: reactiveFunction, @@ -497,9 +507,9 @@ function* runWithEnvironment( uniqueIdentifiers, fbtOperands, }).unwrap(); - yield log({kind: 'ast', name: 'Codegen', value: ast}); + log({kind: 'ast', name: 'Codegen', value: ast}); for (const outlined of ast.outlined) { - yield log({kind: 'ast', name: 'Codegen (outlined)', value: outlined.fn}); + log({kind: 'ast', name: 'Codegen (outlined)', value: outlined.fn}); } /** @@ -525,7 +535,7 @@ export function compileFn( filename: string | null, code: string | null, ): CodegenFunction { - let generator = run( + return run( func, config, fnType, @@ -534,15 +544,9 @@ export function compileFn( filename, code, ); - while (true) { - const next = generator.next(); - if (next.done) { - return next.value; - } - } } -export function log(value: CompilerPipelineValue): CompilerPipelineValue { +function printLog(value: CompilerPipelineValue): CompilerPipelineValue { switch (value.kind) { case 'ast': { logCodegenFunction(value.name, value.value); @@ -566,14 +570,3 @@ export function log(value: CompilerPipelineValue): CompilerPipelineValue { } return value; } - -export function* runPlayground( - func: NodePath< - t.FunctionDeclaration | t.ArrowFunctionExpression | t.FunctionExpression - >, - config: EnvironmentConfig, - fnType: ReactFunctionType, -): Generator { - const ast = yield* run(func, config, fnType, '_c', null, null, null); - return ast; -} 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 fa581d8ed8d81..48589b8be9a77 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -168,11 +168,19 @@ const EnvironmentConfigSchema = z.object({ customMacros: z.nullable(z.array(MacroSchema)).default(null), /** - * Enable a check that resets the memoization cache when the source code of the file changes. - * This is intended to support hot module reloading (HMR), where the same runtime component - * instance will be reused across different versions of the component source. + * Enable a check that resets the memoization cache when the source code of + * the file changes. This is intended to support hot module reloading (HMR), + * where the same runtime component instance will be reused across different + * versions of the component source. + * + * When set to + * - true: code for HMR support is always generated, regardless of NODE_ENV + * or `globalThis.__DEV__` + * - false: code for HMR support is not generated + * - null: (default) code for HMR support is conditionally generated dependent + * on `NODE_ENV` and `globalThis.__DEV__` at the time of compilation. */ - enableResetCacheOnSourceFileChanges: z.boolean().default(false), + enableResetCacheOnSourceFileChanges: z.nullable(z.boolean()).default(null), /** * Enable using information from existing useMemo/useCallback to understand when a value is done @@ -708,7 +716,10 @@ export function parseConfigPragmaForTests(pragma: string): EnvironmentConfig { continue; } - if (typeof defaultConfig[key as keyof EnvironmentConfig] !== 'boolean') { + if ( + key !== 'enableResetCacheOnSourceFileChanges' && + typeof defaultConfig[key as keyof EnvironmentConfig] !== 'boolean' + ) { // skip parsing non-boolean properties continue; } @@ -718,9 +729,15 @@ export function parseConfigPragmaForTests(pragma: string): EnvironmentConfig { maybeConfig[key] = false; } } - const config = EnvironmentConfigSchema.safeParse(maybeConfig); if (config.success) { + /** + * Unless explicitly enabled, do not insert HMR handling code + * in test fixtures or playground to reduce visual noise. + */ + if (config.data.enableResetCacheOnSourceFileChanges == null) { + config.data.enableResetCacheOnSourceFileChanges = false; + } return config.data; } CompilerError.invariant(false, { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-memo-module-scope-usememo-function-scope.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-memo-module-scope-usememo-function-scope.expect.md new file mode 100644 index 0000000000000..69c1b9bbbbc73 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-memo-module-scope-usememo-function-scope.expect.md @@ -0,0 +1,29 @@ + +## Input + +```javascript +// @compilationMode(all) +'use no memo'; + +function TestComponent({x}) { + 'use memo'; + return ; +} + +``` + +## Code + +```javascript +// @compilationMode(all) +"use no memo"; + +function TestComponent({ x }) { + "use memo"; + return ; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-memo-module-scope-usememo-function-scope.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-memo-module-scope-usememo-function-scope.js new file mode 100644 index 0000000000000..9b314e1f99d53 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-no-memo-module-scope-usememo-function-scope.js @@ -0,0 +1,7 @@ +// @compilationMode(all) +'use no memo'; + +function TestComponent({x}) { + 'use memo'; + return ; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/parseConfigPragma-test.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/parseConfigPragma-test.ts index d634bd235f190..dc4d5d25a47e2 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/parseConfigPragma-test.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/parseConfigPragma-test.ts @@ -25,6 +25,7 @@ describe('parseConfigPragmaForTests()', () => { enableUseTypeAnnotations: true, validateNoSetStateInPassiveEffects: true, validateNoSetStateInRender: false, + enableResetCacheOnSourceFileChanges: false, }); }); }); diff --git a/compiler/packages/babel-plugin-react-compiler/src/index.ts b/compiler/packages/babel-plugin-react-compiler/src/index.ts index 150df26e45818..188c244d9ef2c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/index.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/index.ts @@ -17,8 +17,6 @@ export { compileFn as compile, compileProgram, parsePluginOptions, - run, - runPlayground, OPT_OUT_DIRECTIVES, OPT_IN_DIRECTIVES, findDirectiveEnablingMemoization, From 80b81fe56353f7419bd07ecfc3534d274a413fca Mon Sep 17 00:00:00 2001 From: mofeiZ <34200447+mofeiZ@users.noreply.github.com> Date: Mon, 16 Dec 2024 14:43:34 -0500 Subject: [PATCH 08/18] [compiler] Repro for aliased captures within inner function expressions (#31770) see fixture --- ...-func-maybealias-captured-mutate.expect.md | 129 ++++++++++++++++++ ...pturing-func-maybealias-captured-mutate.ts | 48 +++++++ .../packages/snap/src/SproutTodoFilter.ts | 1 + 3 files changed, 178 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-capturing-func-maybealias-captured-mutate.ts 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 0000000000000..b8c7f8d4225f7 --- /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 0000000000000..ca7076fda4019 --- /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 36b3e92f9636b..361070739630c 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', From ac172706526a840100302f92ae90dfa4ad804c56 Mon Sep 17 00:00:00 2001 From: mofeiZ <34200447+mofeiZ@users.noreply.github.com> Date: Mon, 16 Dec 2024 15:11:52 -0500 Subject: [PATCH 09/18] [compiler][ez] Clean up duplicate code in propagateScopeDeps (#31581) Clean up duplicate checks for when to skip processing values as dependencies / hoistable temporaries. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31581). * #31583 * #31582 * __->__ #31581 --- .../src/HIR/PropagateScopeDependenciesHIR.ts | 82 +++++++++++++------ 1 file changed, 58 insertions(+), 24 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts index 8aed17f8ee847..4a85a4ef5cfad 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts @@ -358,6 +358,7 @@ class Context { #temporaries: ReadonlyMap; #temporariesUsedOutsideScope: ReadonlySet; + #processedInstrsInOptional: ReadonlySet; /** * Tracks the traversal state. See Context.declare for explanation of why this @@ -368,9 +369,11 @@ class Context { constructor( temporariesUsedOutsideScope: ReadonlySet, temporaries: ReadonlyMap, + processedInstrsInOptional: ReadonlySet, ) { this.#temporariesUsedOutsideScope = temporariesUsedOutsideScope; this.#temporaries = temporaries; + this.#processedInstrsInOptional = processedInstrsInOptional; } enterScope(scope: ReactiveScope): void { @@ -574,22 +577,49 @@ class Context { currentScope.reassignments.add(place.identifier); } } + enterInnerFn(cb: () => T): T { + const wasInInnerFn = this.inInnerFn; + this.inInnerFn = true; + const result = cb(); + this.inInnerFn = wasInInnerFn; + return result; + } + + /** + * Skip dependencies that are subexpressions of other dependencies. e.g. if a + * dependency is tracked in the temporaries sidemap, it can be added at + * site-of-use + */ + isDeferredDependency( + instr: + | {kind: HIRValue.Instruction; value: Instruction} + | {kind: HIRValue.Terminal; value: Terminal}, + ): boolean { + return ( + this.#processedInstrsInOptional.has(instr.value) || + (instr.kind === HIRValue.Instruction && + this.#temporaries.has(instr.value.lvalue.identifier.id)) + ); + } +} +enum HIRValue { + Instruction = 1, + Terminal, } function handleInstruction(instr: Instruction, context: Context): void { const {id, value, lvalue} = instr; - if (value.kind === 'LoadLocal') { - if ( - value.place.identifier.name === null || - lvalue.identifier.name !== null || - context.isUsedOutsideDeclaringScope(lvalue) - ) { - context.visitOperand(value.place); - } - } else if (value.kind === 'PropertyLoad') { - if (context.isUsedOutsideDeclaringScope(lvalue)) { - context.visitProperty(value.object, value.property, false); - } + context.declare(lvalue.identifier, { + id, + scope: context.currentScope, + }); + if ( + context.isDeferredDependency({kind: HIRValue.Instruction, value: instr}) + ) { + return; + } + if (value.kind === 'PropertyLoad') { + context.visitProperty(value.object, value.property, false); } else if (value.kind === 'StoreLocal') { context.visitOperand(value.value); if (value.lvalue.kind === InstructionKind.Reassign) { @@ -632,11 +662,6 @@ function handleInstruction(instr: Instruction, context: Context): void { context.visitOperand(operand); } } - - context.declare(lvalue.identifier, { - id, - scope: context.currentScope, - }); } function collectDependencies( @@ -645,7 +670,11 @@ function collectDependencies( temporaries: ReadonlyMap, processedInstrsInOptional: ReadonlySet, ): Map> { - const context = new Context(usedOutsideDeclaringScope, temporaries); + const context = new Context( + usedOutsideDeclaringScope, + temporaries, + processedInstrsInOptional, + ); for (const param of fn.params) { if (param.kind === 'Identifier') { @@ -694,16 +723,21 @@ function collectDependencies( /** * Recursively visit the inner function to extract dependencies there */ - const wasInInnerFn = context.inInnerFn; - context.inInnerFn = true; - handleFunction(instr.value.loweredFunc.func); - context.inInnerFn = wasInInnerFn; - } else if (!processedInstrsInOptional.has(instr)) { + const innerFn = instr.value.loweredFunc.func; + context.enterInnerFn(() => { + handleFunction(innerFn); + }); + } else { handleInstruction(instr, context); } } - if (!processedInstrsInOptional.has(block.terminal)) { + if ( + !context.isDeferredDependency({ + kind: HIRValue.Terminal, + value: block.terminal, + }) + ) { for (const place of eachTerminalOperand(block.terminal)) { context.visitOperand(place); } From d325f872de658fc26127a91c965c135d8ad4e877 Mon Sep 17 00:00:00 2001 From: mofeiZ <34200447+mofeiZ@users.noreply.github.com> Date: Mon, 16 Dec 2024 15:15:13 -0500 Subject: [PATCH 10/18] [compiler][be] Logger based debug printing in test runner (#31809) Avoid mutable logging enabled state and writing to `process.stdout` within our babel transform. --- .../babel-plugin-react-compiler/package.json | 2 - .../src/Entrypoint/Pipeline.ts | 44 +------ .../src/Inference/AnalyseFunctions.ts | 7 +- .../InferReactiveScopeVariables.ts | 7 +- .../src/Utils/logger.ts | 110 ------------------ compiler/packages/snap/src/compiler.ts | 22 ++-- compiler/packages/snap/src/constants.ts | 12 +- compiler/packages/snap/src/runner-worker.ts | 47 +++++++- 8 files changed, 78 insertions(+), 173 deletions(-) delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/Utils/logger.ts diff --git a/compiler/packages/babel-plugin-react-compiler/package.json b/compiler/packages/babel-plugin-react-compiler/package.json index 7d55e7b27fe9b..158b800dba900 100644 --- a/compiler/packages/babel-plugin-react-compiler/package.json +++ b/compiler/packages/babel-plugin-react-compiler/package.json @@ -42,9 +42,7 @@ "babel-jest": "^29.0.3", "babel-plugin-fbt": "^1.0.0", "babel-plugin-fbt-runtime": "^1.0.0", - "chalk": "4", "eslint": "^8.57.1", - "glob": "^7.1.6", "invariant": "^2.2.4", "jest": "^29.0.3", "jest-environment-jsdom": "^29.0.3", 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 6995aec7f1825..c48cba32b2642 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -79,13 +79,6 @@ import { rewriteInstructionKindsBasedOnReassignment, } from '../SSA'; import {inferTypes} from '../TypeInference'; -import { - logCodegenFunction, - logDebug, - logHIRFunction, - logReactiveFunction, -} from '../Utils/logger'; -import {assertExhaustive} from '../Utils/utils'; import { validateContextVariableLValues, validateHooksUsage, @@ -139,13 +132,7 @@ function run( name: 'EnvironmentConfig', value: prettyFormat(env.config), }); - printLog({ - kind: 'debug', - name: 'EnvironmentConfig', - value: prettyFormat(env.config), - }); - const ast = runWithEnvironment(func, env); - return ast; + return runWithEnvironment(func, env); } /* @@ -158,10 +145,8 @@ function runWithEnvironment( >, env: Environment, ): CodegenFunction { - const log = (value: CompilerPipelineValue): CompilerPipelineValue => { - printLog(value); + const log = (value: CompilerPipelineValue): void => { env.logger?.debugLogIRs?.(value); - return value; }; const hir = lower(func, env).unwrap(); log({kind: 'hir', name: 'HIR', value: hir}); @@ -545,28 +530,3 @@ export function compileFn( code, ); } - -function printLog(value: CompilerPipelineValue): CompilerPipelineValue { - switch (value.kind) { - case 'ast': { - logCodegenFunction(value.name, value.value); - break; - } - case 'hir': { - logHIRFunction(value.name, value.value); - break; - } - case 'reactive': { - logReactiveFunction(value.name, value.value); - break; - } - case 'debug': { - logDebug(value.name, value.value); - break; - } - default: { - assertExhaustive(value, 'Unexpected compilation kind'); - } - } - return value; -} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts index 684acaf298388..75bd8f6811ffb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts @@ -19,7 +19,6 @@ import { import {deadCodeElimination} from '../Optimization'; import {inferReactiveScopeVariables} from '../ReactiveScopes'; import {rewriteInstructionKindsBasedOnReassignment} from '../SSA'; -import {logHIRFunction} from '../Utils/logger'; import {inferMutableContextVariables} from './InferMutableContextVariables'; import {inferMutableRanges} from './InferMutableRanges'; import inferReferenceEffects from './InferReferenceEffects'; @@ -112,7 +111,11 @@ function lower(func: HIRFunction): void { rewriteInstructionKindsBasedOnReassignment(func); inferReactiveScopeVariables(func); inferMutableContextVariables(func); - logHIRFunction('AnalyseFunction (inner)', func); + func.env.logger?.debugLogIRs?.({ + kind: 'hir', + name: 'AnalyseFunction (inner)', + value: func, + }); } function infer( diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts index 098139b150d5a..1108422f070d7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts @@ -25,7 +25,6 @@ import { eachPatternOperand, } from '../HIR/visitors'; import DisjointSet from '../Utils/DisjointSet'; -import {logHIRFunction} from '../Utils/logger'; import {assertExhaustive} from '../Utils/utils'; /* @@ -156,7 +155,11 @@ export function inferReactiveScopeVariables(fn: HIRFunction): void { scope.range.end > maxInstruction + 1 ) { // Make it easier to debug why the error occurred - logHIRFunction('InferReactiveScopeVariables (invalid scope)', fn); + fn.env.logger?.debugLogIRs?.({ + kind: 'hir', + name: 'InferReactiveScopeVariables (invalid scope)', + value: fn, + }); CompilerError.invariant(false, { reason: `Invalid mutable range for scope`, loc: GeneratedSource, diff --git a/compiler/packages/babel-plugin-react-compiler/src/Utils/logger.ts b/compiler/packages/babel-plugin-react-compiler/src/Utils/logger.ts deleted file mode 100644 index fa43a8befeb83..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/Utils/logger.ts +++ /dev/null @@ -1,110 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import generate from '@babel/generator'; -import * as t from '@babel/types'; -import chalk from 'chalk'; -import {HIR, HIRFunction, ReactiveFunction} from '../HIR/HIR'; -import {printFunctionWithOutlined, printHIR} from '../HIR/PrintHIR'; -import {CodegenFunction} from '../ReactiveScopes'; -import {printReactiveFunctionWithOutlined} from '../ReactiveScopes/PrintReactiveFunction'; - -let ENABLED: boolean = false; - -let lastLogged: string; - -export function toggleLogging(enabled: boolean): void { - ENABLED = enabled; -} - -export function logDebug(step: string, value: string): void { - if (ENABLED) { - process.stdout.write(`${chalk.green(step)}:\n${value}\n\n`); - } -} - -export function logHIR(step: string, ir: HIR): void { - if (ENABLED) { - const printed = printHIR(ir); - if (printed !== lastLogged) { - lastLogged = printed; - process.stdout.write(`${chalk.green(step)}:\n${printed}\n\n`); - } else { - process.stdout.write(`${chalk.blue(step)}: (no change)\n\n`); - } - } -} - -export function logCodegenFunction(step: string, fn: CodegenFunction): void { - if (ENABLED) { - let printed: string | null = null; - try { - const node = t.functionDeclaration( - fn.id, - fn.params, - fn.body, - fn.generator, - fn.async, - ); - const ast = generate(node); - printed = ast.code; - } catch (e) { - let errMsg: string; - if ( - typeof e === 'object' && - e != null && - 'message' in e && - typeof e.message === 'string' - ) { - errMsg = e.message.toString(); - } else { - errMsg = '[empty]'; - } - console.log('Error formatting AST: ' + errMsg); - } - if (printed === null) { - return; - } - if (printed !== lastLogged) { - lastLogged = printed; - process.stdout.write(`${chalk.green(step)}:\n${printed}\n\n`); - } else { - process.stdout.write(`${chalk.blue(step)}: (no change)\n\n`); - } - } -} - -export function logHIRFunction(step: string, fn: HIRFunction): void { - if (ENABLED) { - const printed = printFunctionWithOutlined(fn); - if (printed !== lastLogged) { - lastLogged = printed; - process.stdout.write(`${chalk.green(step)}:\n${printed}\n\n`); - } else { - process.stdout.write(`${chalk.blue(step)}: (no change)\n\n`); - } - } -} - -export function logReactiveFunction(step: string, fn: ReactiveFunction): void { - if (ENABLED) { - const printed = printReactiveFunctionWithOutlined(fn); - if (printed !== lastLogged) { - lastLogged = printed; - process.stdout.write(`${chalk.green(step)}:\n${printed}\n\n`); - } else { - process.stdout.write(`${chalk.blue(step)}: (no change)\n\n`); - } - } -} - -export function log(fn: () => string): void { - if (ENABLED) { - const message = fn(); - process.stdout.write(message.trim() + '\n\n'); - } -} diff --git a/compiler/packages/snap/src/compiler.ts b/compiler/packages/snap/src/compiler.ts index 1cb8fe48b9451..5866445d2e45b 100644 --- a/compiler/packages/snap/src/compiler.ts +++ b/compiler/packages/snap/src/compiler.ts @@ -19,6 +19,7 @@ import type { PanicThresholdOptions, PluginOptions, CompilerReactTarget, + CompilerPipelineValue, } from 'babel-plugin-react-compiler/src/Entrypoint'; import type {Effect, ValueKind} from 'babel-plugin-react-compiler/src/HIR'; import type { @@ -45,6 +46,7 @@ export function parseLanguage(source: string): 'flow' | 'typescript' { function makePluginOptions( firstLine: string, parseConfigPragmaFn: typeof ParseConfigPragma, + debugIRLogger: (value: CompilerPipelineValue) => void, EffectEnum: typeof Effect, ValueKindEnum: typeof ValueKind, ): [PluginOptions, Array<{filename: string | null; event: LoggerEvent}>] { @@ -182,15 +184,15 @@ function makePluginOptions( .filter(s => s.length > 0); } - let logs: Array<{filename: string | null; event: LoggerEvent}> = []; - let logger: Logger | null = null; - if (firstLine.includes('@logger')) { - logger = { - logEvent(filename: string | null, event: LoggerEvent): void { - logs.push({filename, event}); - }, - }; - } + const logs: Array<{filename: string | null; event: LoggerEvent}> = []; + const logger: Logger = { + logEvent: firstLine.includes('@logger') + ? (filename, event) => { + logs.push({filename, event}); + } + : () => {}, + debugLogIRs: debugIRLogger, + }; const config = parseConfigPragmaFn(firstLine); const options = { @@ -338,6 +340,7 @@ export async function transformFixtureInput( parseConfigPragmaFn: typeof ParseConfigPragma, plugin: BabelCore.PluginObj, includeEvaluator: boolean, + debugIRLogger: (value: CompilerPipelineValue) => void, EffectEnum: typeof Effect, ValueKindEnum: typeof ValueKind, ): Promise<{kind: 'ok'; value: TransformResult} | {kind: 'err'; msg: string}> { @@ -365,6 +368,7 @@ export async function transformFixtureInput( const [options, logs] = makePluginOptions( firstLine, parseConfigPragmaFn, + debugIRLogger, EffectEnum, ValueKindEnum, ); diff --git a/compiler/packages/snap/src/constants.ts b/compiler/packages/snap/src/constants.ts index abee06c55be8a..ad77441b532df 100644 --- a/compiler/packages/snap/src/constants.ts +++ b/compiler/packages/snap/src/constants.ts @@ -18,11 +18,17 @@ export const COMPILER_PATH = path.join( 'BabelPlugin.js', ); export const COMPILER_INDEX_PATH = path.join(process.cwd(), 'dist', 'index'); -export const LOGGER_PATH = path.join( +export const PRINT_HIR_PATH = path.join( process.cwd(), 'dist', - 'Utils', - 'logger.js', + 'HIR', + 'PrintHIR.js', +); +export const PRINT_REACTIVE_IR_PATH = path.join( + process.cwd(), + 'dist', + 'ReactiveScopes', + 'PrintReactiveFunction.js', ); export const PARSE_CONFIG_PRAGMA_PATH = path.join( process.cwd(), diff --git a/compiler/packages/snap/src/runner-worker.ts b/compiler/packages/snap/src/runner-worker.ts index f05757d3df68d..ea87cd1e91d16 100644 --- a/compiler/packages/snap/src/runner-worker.ts +++ b/compiler/packages/snap/src/runner-worker.ts @@ -8,16 +8,21 @@ import {codeFrameColumns} from '@babel/code-frame'; import type {PluginObj} from '@babel/core'; import type {parseConfigPragmaForTests as ParseConfigPragma} from 'babel-plugin-react-compiler/src/HIR/Environment'; +import type {printFunctionWithOutlined as PrintFunctionWithOutlined} from 'babel-plugin-react-compiler/src/HIR/PrintHIR'; +import type {printReactiveFunctionWithOutlined as PrintReactiveFunctionWithOutlined} from 'babel-plugin-react-compiler/src/ReactiveScopes/PrintReactiveFunction'; import {TransformResult, transformFixtureInput} from './compiler'; import { COMPILER_PATH, COMPILER_INDEX_PATH, - LOGGER_PATH, PARSE_CONFIG_PRAGMA_PATH, + PRINT_HIR_PATH, + PRINT_REACTIVE_IR_PATH, } from './constants'; import {TestFixture, getBasename, isExpectError} from './fixture-utils'; import {TestResult, writeOutputToString} from './reporter'; import {runSprout} from './sprout'; +import {CompilerPipelineValue} from 'babel-plugin-react-compiler/src'; +import chalk from 'chalk'; const originalConsoleError = console.error; @@ -64,20 +69,56 @@ async function compile( const {Effect: EffectEnum, ValueKind: ValueKindEnum} = require( COMPILER_INDEX_PATH, ); - const {toggleLogging} = require(LOGGER_PATH); + const {printFunctionWithOutlined} = require(PRINT_HIR_PATH) as { + printFunctionWithOutlined: typeof PrintFunctionWithOutlined; + }; + const {printReactiveFunctionWithOutlined} = require( + PRINT_REACTIVE_IR_PATH, + ) as { + printReactiveFunctionWithOutlined: typeof PrintReactiveFunctionWithOutlined; + }; + + let lastLogged: string | null = null; + const debugIRLogger = shouldLog + ? (value: CompilerPipelineValue) => { + let printed: string; + switch (value.kind) { + case 'hir': + printed = printFunctionWithOutlined(value.value); + break; + case 'reactive': + printed = printReactiveFunctionWithOutlined(value.value); + break; + case 'debug': + printed = value.value; + break; + case 'ast': + // skip printing ast as we already write fixture output JS + printed = '(ast)'; + break; + } + + if (printed !== lastLogged) { + lastLogged = printed; + console.log(`${chalk.green(value.name)}:\n ${printed}\n`); + } else { + console.log(`${chalk.blue(value.name)}: (no change)\n`); + } + } + : () => {}; const {parseConfigPragmaForTests} = require(PARSE_CONFIG_PRAGMA_PATH) as { parseConfigPragmaForTests: typeof ParseConfigPragma; }; // only try logging if we filtered out all but one fixture, // since console log order is non-deterministic - toggleLogging(shouldLog); const result = await transformFixtureInput( input, fixturePath, parseConfigPragmaForTests, BabelPluginReactCompiler, includeEvaluator, + debugIRLogger, EffectEnum, ValueKindEnum, ); From 308be6e8dc0310c67c0b7341e459ebd8ad1ee1d0 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Mon, 16 Dec 2024 15:48:19 -0500 Subject: [PATCH 11/18] [compiler] Add option for firing effect functions (#31794) Config flag for `fire` -- --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31794). * #31811 * #31798 * #31797 * #31796 * #31795 * __->__ #31794 --- .../babel-plugin-react-compiler/src/HIR/Environment.ts | 2 ++ compiler/packages/snap/src/compiler.ts | 6 ++++++ 2 files changed, 8 insertions(+) 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 48589b8be9a77..e2932296ca739 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -249,6 +249,8 @@ const EnvironmentConfigSchema = z.object({ */ enableOptionalDependencies: z.boolean().default(true), + enableFire: z.boolean().default(false), + /** * Enables inference and auto-insertion of effect dependencies. Takes in an array of * configurable module and import pairs to allow for user-land experimentation. For example, diff --git a/compiler/packages/snap/src/compiler.ts b/compiler/packages/snap/src/compiler.ts index 5866445d2e45b..a95c61450d840 100644 --- a/compiler/packages/snap/src/compiler.ts +++ b/compiler/packages/snap/src/compiler.ts @@ -58,6 +58,7 @@ function makePluginOptions( let validatePreserveExistingMemoizationGuarantees = false; let customMacros: null | Array = null; let validateBlocklistedImports = null; + let enableFire = false; let target: CompilerReactTarget = '19'; if (firstLine.indexOf('@compilationMode(annotation)') !== -1) { @@ -129,6 +130,10 @@ function makePluginOptions( validatePreserveExistingMemoizationGuarantees = true; } + if (firstLine.includes('@enableFire')) { + enableFire = true; + } + const hookPatternMatch = /@hookPattern:"([^"]+)"/.exec(firstLine); if ( hookPatternMatch && @@ -207,6 +212,7 @@ function makePluginOptions( hookPattern, validatePreserveExistingMemoizationGuarantees, validateBlocklistedImports, + enableFire, }, compilationMode, logger, From c869063f0dd92f09244bb43e44fe175a5f97b7e6 Mon Sep 17 00:00:00 2001 From: Jordan Brown Date: Mon, 16 Dec 2024 15:48:32 -0500 Subject: [PATCH 12/18] [compiler] Add fire to known React APIs (#31795) Makes `fire` a known export for type-based analysis -- --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31795). * #31811 * #31798 * #31797 * #31796 * __->__ #31795 * #31794 --- .../src/HIR/Globals.ts | 16 ++++++++++++++++ .../src/HIR/ObjectShape.ts | 1 + 2 files changed, 17 insertions(+) diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts index 2525b87bd86bc..2249e33491fa8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts @@ -9,6 +9,7 @@ import {Effect, ValueKind, ValueReason} from './HIR'; import { BUILTIN_SHAPES, BuiltInArrayId, + BuiltInFireId, BuiltInMixedReadonlyId, BuiltInUseActionStateId, BuiltInUseContextHookId, @@ -468,6 +469,21 @@ const REACT_APIS: Array<[string, BuiltInType]> = [ BuiltInUseOperatorId, ), ], + [ + 'fire', + addFunction( + DEFAULT_SHAPES, + [], + { + positionalParams: [], + restParam: null, + returnType: {kind: 'Primitive'}, + calleeEffect: Effect.Read, + returnValueKind: ValueKind.Frozen, + }, + BuiltInFireId, + ), + ], ]; TYPED_GLOBALS.push( diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts index 14f809f2c4082..4482d17890196 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts @@ -213,6 +213,7 @@ export const BuiltInDispatchId = 'BuiltInDispatch'; export const BuiltInUseContextHookId = 'BuiltInUseContextHook'; export const BuiltInUseTransitionId = 'BuiltInUseTransition'; export const BuiltInStartTransitionId = 'BuiltInStartTransition'; +export const BuiltInFireId = 'BuiltInFire'; // ShapeRegistry with default definitions for built-ins. export const BUILTIN_SHAPES: ShapeRegistry = new Map(); From a78bbf9dbcf92434c902f9265ddfee34eae51a54 Mon Sep 17 00:00:00 2001 From: mofeiZ <34200447+mofeiZ@users.noreply.github.com> Date: Mon, 16 Dec 2024 16:45:05 -0500 Subject: [PATCH 13/18] [compiler] Context variables as dependencies (#31582) We previously didn't track context variables in the hoistable values sidemap of `propagateScopeDependencies`. This was overly conservative as we *do* track the mutable range of context variables, and it is safe to hoist accesses to context variables after their last direct / aliased maybe-assignment. ```js function Component({value}) { // start of mutable range for `x` let x = DEFAULT; const setX = () => x = value; const aliasedSet = maybeAlias(setX); maybeCall(aliasedSet); // end of mutable range for `x` // here, we should be able to take x (and property reads // off of x) as dependencies return } ``` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31582). * #31583 * __->__ #31582 --- .../src/HIR/HIR.ts | 11 +- .../src/HIR/PropagateScopeDependenciesHIR.ts | 65 ++++++--- .../bug-functiondecl-hoisting.expect.md | 18 ++- ...-array-assignment-to-context-var.expect.md | 15 +- ...array-declaration-to-context-var.expect.md | 15 +- ...object-assignment-to-context-var.expect.md | 15 +- ...bject-declaration-to-context-var.expect.md | 15 +- ...mutated-non-reactive-to-reactive.expect.md | 16 +-- ...ures-reassigned-context-property.expect.md | 53 ------- ...ures-reassigned-context-property.expect.md | 101 ++++++++++++++ ...-captures-reassigned-context-property.tsx} | 0 ...o-reordering-depslist-assignment.expect.md | 15 +- ...epro-scope-missing-mutable-range.expect.md | 16 +-- ...l-dependency-on-context-variable.expect.md | 16 +-- .../context-var-granular-dep.expect.md | 130 ++++++++++++++++++ .../context-var-granular-dep.js | 43 ++++++ ...epro-scope-missing-mutable-range.expect.md | 16 +-- .../use-operator-conditional.expect.md | 42 +++--- compiler/packages/snap/src/sprout/index.ts | 10 +- .../snap/src/sprout/shared-runtime.ts | 29 ++-- 20 files changed, 447 insertions(+), 194 deletions(-) delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.expect.md rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/{error.todo-useCallback-captures-reassigned-context-property.tsx => useCallback-captures-reassigned-context-property.tsx} (100%) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/context-var-granular-dep.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/context-var-granular-dep.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts index 475d8e8bec574..a6a9cbcd48ec5 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -840,6 +840,11 @@ export type LoadLocal = { place: Place; loc: SourceLocation; }; +export type LoadContext = { + kind: 'LoadContext'; + place: Place; + loc: SourceLocation; +}; /* * The value of a given instruction. Note that values are not recursive: complex @@ -852,11 +857,7 @@ export type LoadLocal = { export type InstructionValue = | LoadLocal - | { - kind: 'LoadContext'; - place: Place; - loc: SourceLocation; - } + | LoadContext | { kind: 'DeclareLocal'; lvalue: LValue; diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts index 4a85a4ef5cfad..08856e9143139 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts @@ -17,6 +17,11 @@ import { areEqualPaths, IdentifierId, Terminal, + InstructionValue, + LoadContext, + TInstruction, + FunctionExpression, + ObjectMethod, } from './HIR'; import { collectHoistablePropertyLoads, @@ -223,11 +228,25 @@ export function collectTemporariesSidemap( fn, usedOutsideDeclaringScope, temporaries, - false, + null, ); return temporaries; } +function isLoadContextMutable( + instrValue: InstructionValue, + id: InstructionId, +): instrValue is LoadContext { + if (instrValue.kind === 'LoadContext') { + CompilerError.invariant(instrValue.place.identifier.scope != null, { + reason: + '[PropagateScopeDependencies] Expected all context variables to be assigned a scope', + loc: instrValue.loc, + }); + return id >= instrValue.place.identifier.scope.range.end; + } + return false; +} /** * Recursive collect a sidemap of all `LoadLocal` and `PropertyLoads` with a * function and all nested functions. @@ -239,17 +258,21 @@ function collectTemporariesSidemapImpl( fn: HIRFunction, usedOutsideDeclaringScope: ReadonlySet, temporaries: Map, - isInnerFn: boolean, + innerFnContext: {instrId: InstructionId} | null, ): void { for (const [_, block] of fn.body.blocks) { - for (const instr of block.instructions) { - const {value, lvalue} = instr; + for (const {value, lvalue, id: origInstrId} of block.instructions) { + const instrId = + innerFnContext != null ? innerFnContext.instrId : origInstrId; const usedOutside = usedOutsideDeclaringScope.has( lvalue.identifier.declarationId, ); if (value.kind === 'PropertyLoad' && !usedOutside) { - if (!isInnerFn || temporaries.has(value.object.identifier.id)) { + if ( + innerFnContext == null || + temporaries.has(value.object.identifier.id) + ) { /** * All dependencies of a inner / nested function must have a base * identifier from the outermost component / hook. This is because the @@ -265,13 +288,13 @@ function collectTemporariesSidemapImpl( temporaries.set(lvalue.identifier.id, property); } } else if ( - value.kind === 'LoadLocal' && + (value.kind === 'LoadLocal' || isLoadContextMutable(value, instrId)) && lvalue.identifier.name == null && value.place.identifier.name !== null && !usedOutside ) { if ( - !isInnerFn || + innerFnContext == null || fn.context.some( context => context.identifier.id === value.place.identifier.id, ) @@ -289,7 +312,7 @@ function collectTemporariesSidemapImpl( value.loweredFunc.func, usedOutsideDeclaringScope, temporaries, - true, + innerFnContext ?? {instrId}, ); } } @@ -364,7 +387,7 @@ class Context { * Tracks the traversal state. See Context.declare for explanation of why this * is needed. */ - inInnerFn: boolean = false; + #innerFnContext: {outerInstrId: InstructionId} | null = null; constructor( temporariesUsedOutsideScope: ReadonlySet, @@ -434,7 +457,7 @@ class Context { * by root identifier mutable ranges). */ declare(identifier: Identifier, decl: Decl): void { - if (this.inInnerFn) return; + if (this.#innerFnContext != null) return; if (!this.#declarations.has(identifier.declarationId)) { this.#declarations.set(identifier.declarationId, decl); } @@ -577,11 +600,14 @@ class Context { currentScope.reassignments.add(place.identifier); } } - enterInnerFn(cb: () => T): T { - const wasInInnerFn = this.inInnerFn; - this.inInnerFn = true; + enterInnerFn( + innerFn: TInstruction | TInstruction, + cb: () => T, + ): T { + const prevContext = this.#innerFnContext; + this.#innerFnContext = this.#innerFnContext ?? {outerInstrId: innerFn.id}; const result = cb(); - this.inInnerFn = wasInInnerFn; + this.#innerFnContext = prevContext; return result; } @@ -724,9 +750,14 @@ function collectDependencies( * Recursively visit the inner function to extract dependencies there */ const innerFn = instr.value.loweredFunc.func; - context.enterInnerFn(() => { - handleFunction(innerFn); - }); + context.enterInnerFn( + instr as + | TInstruction + | TInstruction, + () => { + handleFunction(innerFn); + }, + ); } else { handleInstruction(instr, context); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-functiondecl-hoisting.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-functiondecl-hoisting.expect.md index 2b0031b117be2..f8712ed7289a9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-functiondecl-hoisting.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-functiondecl-hoisting.expect.md @@ -58,18 +58,16 @@ function Foo(t0) { bar = $[1]; result = $[2]; } - - const t1 = bar; - let t2; - if ($[3] !== result || $[4] !== t1) { - t2 = ; - $[3] = result; - $[4] = t1; - $[5] = t2; + let t1; + if ($[3] !== bar || $[4] !== result) { + t1 = ; + $[3] = bar; + $[4] = result; + $[5] = t1; } else { - t2 = $[5]; + t1 = $[5]; } - return t2; + return t1; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-assignment-to-context-var.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-assignment-to-context-var.expect.md index 7febb3fecb6e7..1268cbcfdc3d3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-assignment-to-context-var.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-assignment-to-context-var.expect.md @@ -43,16 +43,15 @@ function Component(props) { } else { x = $[1]; } - const t0 = x; - let t1; - if ($[2] !== t0) { - t1 = { x: t0 }; - $[2] = t0; - $[3] = t1; + let t0; + if ($[2] !== x) { + t0 = { x }; + $[2] = x; + $[3] = t0; } else { - t1 = $[3]; + t0 = $[3]; } - return t1; + return t0; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-declaration-to-context-var.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-declaration-to-context-var.expect.md index 26b56ea2a4f4d..769e4871f4ad8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-declaration-to-context-var.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-array-declaration-to-context-var.expect.md @@ -42,16 +42,15 @@ function Component(props) { } else { x = $[1]; } - const t0 = x; - let t1; - if ($[2] !== t0) { - t1 =
{t0}
; - $[2] = t0; - $[3] = t1; + let t0; + if ($[2] !== x) { + t0 =
{x}
; + $[2] = x; + $[3] = t0; } else { - t1 = $[3]; + t0 = $[3]; } - return t1; + return t0; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-object-assignment-to-context-var.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-object-assignment-to-context-var.expect.md index 5ffa73389ffd0..e66ef2df13d5f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-object-assignment-to-context-var.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-object-assignment-to-context-var.expect.md @@ -43,16 +43,15 @@ function Component(props) { } else { x = $[1]; } - const t0 = x; - let t1; - if ($[2] !== t0) { - t1 = { x: t0 }; - $[2] = t0; - $[3] = t1; + let t0; + if ($[2] !== x) { + t0 = { x }; + $[2] = x; + $[3] = t0; } else { - t1 = $[3]; + t0 = $[3]; } - return t1; + return t0; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-object-declaration-to-context-var.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-object-declaration-to-context-var.expect.md index 2c495d8223d0b..66799c5c4720b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-object-declaration-to-context-var.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/destructure-object-declaration-to-context-var.expect.md @@ -42,16 +42,15 @@ function Component(props) { } else { x = $[1]; } - const t0 = x; - let t1; - if ($[2] !== t0) { - t1 = { x: t0 }; - $[2] = t0; - $[3] = t1; + let t0; + if ($[2] !== x) { + t0 = { x }; + $[2] = x; + $[3] = t0; } else { - t1 = $[3]; + t0 = $[3]; } - return t1; + return t0; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/lambda-mutated-non-reactive-to-reactive.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/lambda-mutated-non-reactive-to-reactive.expect.md index dfe941282e2a3..d34db46d6aa28 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/lambda-mutated-non-reactive-to-reactive.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/lambda-mutated-non-reactive-to-reactive.expect.md @@ -33,17 +33,15 @@ function f(a) { } else { x = $[1]; } - - const t0 = x; - let t1; - if ($[2] !== t0) { - t1 =
; - $[2] = t0; - $[3] = t1; + let t0; + if ($[2] !== x) { + t0 =
; + $[2] = x; + $[3] = t0; } else { - t1 = $[3]; + t0 = $[3]; } - return t1; + return t0; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.expect.md deleted file mode 100644 index ae44f27912293..0000000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.expect.md +++ /dev/null @@ -1,53 +0,0 @@ - -## Input - -```javascript -// @validatePreserveExistingMemoizationGuarantees -import {useCallback} from 'react'; -import {Stringify} from 'shared-runtime'; - -/** - * TODO: we're currently bailing out because `contextVar` is a context variable - * and not recorded into the PropagateScopeDeps LoadLocal / PropertyLoad - * sidemap. Previously, we were able to avoid this as `BuildHIR` hoisted - * `LoadContext` and `PropertyLoad` instructions into the outer function, which - * we took as eligible dependencies. - * - * One solution is to simply record `LoadContext` identifiers into the - * temporaries sidemap when the instruction occurs *after* the context - * variable's mutable range. - */ -function Foo(props) { - let contextVar; - if (props.cond) { - contextVar = {val: 2}; - } else { - contextVar = {}; - } - - const cb = useCallback(() => [contextVar.val], [contextVar.val]); - - return ; -} - -export const FIXTURE_ENTRYPOINT = { - fn: Foo, - params: [{cond: true}], -}; - -``` - - -## Error - -``` - 22 | } - 23 | -> 24 | const cb = useCallback(() => [contextVar.val], [contextVar.val]); - | ^^^^^^^^^^^^^^^^^^^^^^ CannotPreserveMemoization: React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. The inferred dependencies did not match the manually specified dependencies, which could cause the value to change more or less frequently than expected (24:24) - 25 | - 26 | return ; - 27 | } -``` - - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.expect.md new file mode 100644 index 0000000000000..a1cbe89a88340 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.expect.md @@ -0,0 +1,101 @@ + +## Input + +```javascript +// @validatePreserveExistingMemoizationGuarantees +import {useCallback} from 'react'; +import {Stringify} from 'shared-runtime'; + +/** + * TODO: we're currently bailing out because `contextVar` is a context variable + * and not recorded into the PropagateScopeDeps LoadLocal / PropertyLoad + * sidemap. Previously, we were able to avoid this as `BuildHIR` hoisted + * `LoadContext` and `PropertyLoad` instructions into the outer function, which + * we took as eligible dependencies. + * + * One solution is to simply record `LoadContext` identifiers into the + * temporaries sidemap when the instruction occurs *after* the context + * variable's mutable range. + */ +function Foo(props) { + let contextVar; + if (props.cond) { + contextVar = {val: 2}; + } else { + contextVar = {}; + } + + const cb = useCallback(() => [contextVar.val], [contextVar.val]); + + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{cond: true}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees +import { useCallback } from "react"; +import { Stringify } from "shared-runtime"; + +/** + * TODO: we're currently bailing out because `contextVar` is a context variable + * and not recorded into the PropagateScopeDeps LoadLocal / PropertyLoad + * sidemap. Previously, we were able to avoid this as `BuildHIR` hoisted + * `LoadContext` and `PropertyLoad` instructions into the outer function, which + * we took as eligible dependencies. + * + * One solution is to simply record `LoadContext` identifiers into the + * temporaries sidemap when the instruction occurs *after* the context + * variable's mutable range. + */ +function Foo(props) { + const $ = _c(6); + let contextVar; + if ($[0] !== props.cond) { + if (props.cond) { + contextVar = { val: 2 }; + } else { + contextVar = {}; + } + $[0] = props.cond; + $[1] = contextVar; + } else { + contextVar = $[1]; + } + let t0; + if ($[2] !== contextVar.val) { + t0 = () => [contextVar.val]; + $[2] = contextVar.val; + $[3] = t0; + } else { + t0 = $[3]; + } + contextVar; + const cb = t0; + let t1; + if ($[4] !== cb) { + t1 = ; + $[4] = cb; + $[5] = t1; + } else { + t1 = $[5]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{ cond: true }], +}; + +``` + +### Eval output +(kind: ok)
{"cb":{"kind":"Function","result":[2]},"shouldInvokeFns":true}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.tsx similarity index 100% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.todo-useCallback-captures-reassigned-context-property.tsx rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useCallback-captures-reassigned-context-property.tsx diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useMemo-reordering-depslist-assignment.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useMemo-reordering-depslist-assignment.expect.md index dc1a87fe5113c..e8a3e2d627c59 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useMemo-reordering-depslist-assignment.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/useMemo-reordering-depslist-assignment.expect.md @@ -44,16 +44,15 @@ function useFoo(arr1, arr2) { y = $[2]; } let t0; - const t1 = y; - let t2; - if ($[3] !== t1) { - t2 = { y: t1 }; - $[3] = t1; - $[4] = t2; + let t1; + if ($[3] !== y) { + t1 = { y }; + $[3] = y; + $[4] = t1; } else { - t2 = $[4]; + t1 = $[4]; } - t0 = t2; + t0 = t1; return t0; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/repro-scope-missing-mutable-range.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/repro-scope-missing-mutable-range.expect.md index 39f301432e51f..9d232d8e78169 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/repro-scope-missing-mutable-range.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/propagate-scope-deps-hir-fork/repro-scope-missing-mutable-range.expect.md @@ -36,17 +36,15 @@ function HomeDiscoStoreItemTileRating(props) { } else { count = $[1]; } - - const t0 = count; - let t1; - if ($[2] !== t0) { - t1 = {t0}; - $[2] = t0; - $[3] = t1; + let t0; + if ($[2] !== count) { + t0 = {count}; + $[2] = count; + $[3] = t0; } else { - t1 = $[3]; + t0 = $[3]; } - return t1; + return t0; } ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-on-context-variable.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-on-context-variable.expect.md index 23cc7ee84607b..ceaa350012258 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-on-context-variable.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-on-context-variable.expect.md @@ -67,17 +67,15 @@ function Component(props) { } else { x = $[1]; } - - const t0 = x; - let t1; - if ($[2] !== t0) { - t1 = [t0]; - $[2] = t0; - $[3] = t1; + let t0; + if ($[2] !== x) { + t0 = [x]; + $[2] = x; + $[3] = t0; } else { - t1 = $[3]; + t0 = $[3]; } - return t1; + return t0; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/context-var-granular-dep.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/context-var-granular-dep.expect.md new file mode 100644 index 0000000000000..d72f34b4fd8a9 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/context-var-granular-dep.expect.md @@ -0,0 +1,130 @@ + +## Input + +```javascript +import {throwErrorWithMessage, ValidateMemoization} from 'shared-runtime'; + +/** + * Context variables are local variables that (1) have at least one reassignment + * and (2) are captured into a function expression. These have a known mutable + * range: from first declaration / assignment to the last direct or aliased, + * mutable reference. + * + * This fixture validates that forget can take granular dependencies on context + * variables when the reference to a context var happens *after* the end of its + * mutable range. + */ +function Component({cond, a}) { + let contextVar; + if (cond) { + contextVar = {val: a}; + } else { + contextVar = {}; + throwErrorWithMessage(''); + } + const cb = {cb: () => contextVar.val * 4}; + + /** + * manually specify input to avoid adding a `PropertyLoad` from contextVar, + * which might affect hoistable-objects analysis. + */ + return ( + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{cond: false, a: undefined}], + sequentialRenders: [ + {cond: true, a: 2}, + {cond: true, a: 2}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { throwErrorWithMessage, ValidateMemoization } from "shared-runtime"; + +/** + * Context variables are local variables that (1) have at least one reassignment + * and (2) are captured into a function expression. These have a known mutable + * range: from first declaration / assignment to the last direct or aliased, + * mutable reference. + * + * This fixture validates that forget can take granular dependencies on context + * variables when the reference to a context var happens *after* the end of its + * mutable range. + */ +function Component(t0) { + const $ = _c(10); + const { cond, a } = t0; + let contextVar; + if ($[0] !== a || $[1] !== cond) { + if (cond) { + contextVar = { val: a }; + } else { + contextVar = {}; + throwErrorWithMessage(""); + } + $[0] = a; + $[1] = cond; + $[2] = contextVar; + } else { + contextVar = $[2]; + } + let t1; + if ($[3] !== contextVar.val) { + t1 = { cb: () => contextVar.val * 4 }; + $[3] = contextVar.val; + $[4] = t1; + } else { + t1 = $[4]; + } + const cb = t1; + + const t2 = cond ? a : undefined; + let t3; + if ($[5] !== t2) { + t3 = [t2]; + $[5] = t2; + $[6] = t3; + } else { + t3 = $[6]; + } + let t4; + if ($[7] !== cb || $[8] !== t3) { + t4 = ( + + ); + $[7] = cb; + $[8] = t3; + $[9] = t4; + } else { + t4 = $[9]; + } + return t4; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ cond: false, a: undefined }], + sequentialRenders: [ + { cond: true, a: 2 }, + { cond: true, a: 2 }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"inputs":[2],"output":{"cb":"[[ function params=0 ]]"}}
+
{"inputs":[2],"output":{"cb":"[[ function params=0 ]]"}}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/context-var-granular-dep.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/context-var-granular-dep.js new file mode 100644 index 0000000000000..b9bdd67e2f504 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reduce-reactive-deps/context-var-granular-dep.js @@ -0,0 +1,43 @@ +import {throwErrorWithMessage, ValidateMemoization} from 'shared-runtime'; + +/** + * Context variables are local variables that (1) have at least one reassignment + * and (2) are captured into a function expression. These have a known mutable + * range: from first declaration / assignment to the last direct or aliased, + * mutable reference. + * + * This fixture validates that forget can take granular dependencies on context + * variables when the reference to a context var happens *after* the end of its + * mutable range. + */ +function Component({cond, a}) { + let contextVar; + if (cond) { + contextVar = {val: a}; + } else { + contextVar = {}; + throwErrorWithMessage(''); + } + const cb = {cb: () => contextVar.val * 4}; + + /** + * manually specify input to avoid adding a `PropertyLoad` from contextVar, + * which might affect hoistable-objects analysis. + */ + return ( + + ); +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{cond: false, a: undefined}], + sequentialRenders: [ + {cond: true, a: 2}, + {cond: true, a: 2}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md index d8e59c486a55b..b7c425ba5c027 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md @@ -35,17 +35,15 @@ function HomeDiscoStoreItemTileRating(props) { } else { count = $[1]; } - - const t0 = count; - let t1; - if ($[2] !== t0) { - t1 = {t0}; - $[2] = t0; - $[3] = t1; + let t0; + if ($[2] !== count) { + t0 = {count}; + $[2] = count; + $[3] = t0; } else { - t1 = $[3]; + t0 = $[3]; } - return t1; + return t0; } ``` diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-operator-conditional.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-operator-conditional.expect.md index d94a5e7e375b3..e335273026791 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-operator-conditional.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/use-operator-conditional.expect.md @@ -88,36 +88,34 @@ function Inner(props) { input; input; let t0; - const t1 = input; - let t2; - if ($[0] !== t1) { - t2 = [t1]; - $[0] = t1; - $[1] = t2; + let t1; + if ($[0] !== input) { + t1 = [input]; + $[0] = input; + $[1] = t1; } else { - t2 = $[1]; + t1 = $[1]; } - t0 = t2; + t0 = t1; const output = t0; - const t3 = input; - let t4; - if ($[2] !== t3) { - t4 = [t3]; - $[2] = t3; - $[3] = t4; + let t2; + if ($[2] !== input) { + t2 = [input]; + $[2] = input; + $[3] = t2; } else { - t4 = $[3]; + t2 = $[3]; } - let t5; - if ($[4] !== output || $[5] !== t4) { - t5 = ; + let t3; + if ($[4] !== output || $[5] !== t2) { + t3 = ; $[4] = output; - $[5] = t4; - $[6] = t5; + $[5] = t2; + $[6] = t3; } else { - t5 = $[6]; + t3 = $[6]; } - return t5; + return t3; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/snap/src/sprout/index.ts b/compiler/packages/snap/src/sprout/index.ts index 733be561c08a8..04748bed28f4f 100644 --- a/compiler/packages/snap/src/sprout/index.ts +++ b/compiler/packages/snap/src/sprout/index.ts @@ -32,7 +32,15 @@ export function runSprout( originalCode: string, forgetCode: string, ): SproutResult { - const forgetResult = doEval(forgetCode); + let forgetResult; + try { + (globalThis as any).__SNAP_EVALUATOR_MODE = 'forget'; + forgetResult = doEval(forgetCode); + } catch (e) { + throw e; + } finally { + (globalThis as any).__SNAP_EVALUATOR_MODE = undefined; + } if (forgetResult.kind === 'UnexpectedError') { return makeError('Unexpected error in Forget runner', forgetResult.value); } diff --git a/compiler/packages/snap/src/sprout/shared-runtime.ts b/compiler/packages/snap/src/sprout/shared-runtime.ts index 58815842cb03c..1b8648f4ff031 100644 --- a/compiler/packages/snap/src/sprout/shared-runtime.ts +++ b/compiler/packages/snap/src/sprout/shared-runtime.ts @@ -259,26 +259,35 @@ export function Throw() { export function ValidateMemoization({ inputs, - output, + output: rawOutput, + onlyCheckCompiled = false, }: { inputs: Array; output: any; + onlyCheckCompiled: boolean; }): React.ReactElement { 'use no forget'; + // Wrap rawOutput as it might be a function, which useState would invoke. + const output = {value: rawOutput}; const [previousInputs, setPreviousInputs] = React.useState(inputs); const [previousOutput, setPreviousOutput] = React.useState(output); if ( - inputs.length !== previousInputs.length || - inputs.some((item, i) => item !== previousInputs[i]) + onlyCheckCompiled && + (globalThis as any).__SNAP_EVALUATOR_MODE === 'forget' ) { - // Some input changed, we expect the output to change - setPreviousInputs(inputs); - setPreviousOutput(output); - } else if (output !== previousOutput) { - // Else output should be stable - throw new Error('Output identity changed but inputs did not'); + if ( + inputs.length !== previousInputs.length || + inputs.some((item, i) => item !== previousInputs[i]) + ) { + // Some input changed, we expect the output to change + setPreviousInputs(inputs); + setPreviousOutput(output); + } else if (output.value !== previousOutput.value) { + // Else output should be stable + throw new Error('Output identity changed but inputs did not'); + } } - return React.createElement(Stringify, {inputs, output}); + return React.createElement(Stringify, {inputs, output: rawOutput}); } export function createHookWrapper( From 8a7b30669aaa6f020494ba143d40fb396521e7a7 Mon Sep 17 00:00:00 2001 From: mofeiZ <34200447+mofeiZ@users.noreply.github.com> Date: Mon, 16 Dec 2024 16:45:17 -0500 Subject: [PATCH 14/18] [compiler][ez] Add shape for global Object.keys (#31583) Add shape / type for global Object.keys. This is useful because - it has an Effect.Read (not an Effect.Capture) as it cannot alias its argument. - Object.keys return an array --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31583). * __->__ #31583 * #31582 --- .../src/HIR/Globals.ts | 15 ++++++ .../compiler/shapes-object-key.expect.md | 49 +++++++++++++++++++ .../fixtures/compiler/shapes-object-key.ts | 11 +++++ 3 files changed, 75 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/shapes-object-key.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/shapes-object-key.ts diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts index 2249e33491fa8..ac5d44bab0875 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Globals.ts @@ -88,6 +88,21 @@ const UNTYPED_GLOBALS: Set = new Set([ ]); const TYPED_GLOBALS: Array<[string, BuiltInType]> = [ + [ + 'Object', + addObject(DEFAULT_SHAPES, 'Object', [ + [ + 'keys', + addFunction(DEFAULT_SHAPES, [], { + positionalParams: [Effect.Read], + restParam: null, + returnType: {kind: 'Object', shapeId: BuiltInArrayId}, + calleeEffect: Effect.Read, + returnValueKind: ValueKind.Mutable, + }), + ], + ]), + ], [ 'Array', addObject(DEFAULT_SHAPES, 'Array', [ diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/shapes-object-key.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/shapes-object-key.expect.md new file mode 100644 index 0000000000000..e491eb6c69d3c --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/shapes-object-key.expect.md @@ -0,0 +1,49 @@ + +## Input + +```javascript +import {arrayPush} from 'shared-runtime'; + +function useFoo({a, b}) { + const obj = {a}; + arrayPush(Object.keys(obj), b); + return obj; +} +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{a: 2, b: 3}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { arrayPush } from "shared-runtime"; + +function useFoo(t0) { + const $ = _c(2); + const { a, b } = t0; + let t1; + if ($[0] !== a) { + t1 = { a }; + $[0] = a; + $[1] = t1; + } else { + t1 = $[1]; + } + const obj = t1; + arrayPush(Object.keys(obj), b); + return obj; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ a: 2, b: 3 }], +}; + +``` + +### Eval output +(kind: ok) {"a":2} \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/shapes-object-key.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/shapes-object-key.ts new file mode 100644 index 0000000000000..9dbaac79c6b5e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/shapes-object-key.ts @@ -0,0 +1,11 @@ +import {arrayPush} from 'shared-runtime'; + +function useFoo({a, b}) { + const obj = {a}; + arrayPush(Object.keys(obj), b); + return obj; +} +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{a: 2, b: 3}], +}; From 8dab5920e019950874bcc9061480dd78c849e1d7 Mon Sep 17 00:00:00 2001 From: Ricky Date: Mon, 16 Dec 2024 22:43:51 -0500 Subject: [PATCH 15/18] Turn on useModernStrictMode in test renderers (#31769) It's on everywhere else, let's turn this on so we can remove it. Probably should have been turned on in the test renderer for 19. --- packages/shared/forks/ReactFeatureFlags.test-renderer.js | 2 +- packages/shared/forks/ReactFeatureFlags.test-renderer.www.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index b332c485a0862..8281009889b21 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -53,7 +53,7 @@ export const enableLegacyHidden = false; export const enableTransitionTracing = false; -export const useModernStrictMode = false; +export const useModernStrictMode = true; export const enableDO_NOT_USE_disableStrictPassiveEffect = false; export const enableFizzExternalRuntime = true; export const enableDeferRootSchedulingToMicrotask = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 20ecdb44b4717..d1fe7629901ec 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -55,7 +55,7 @@ export const enableLegacyHidden = false; export const enableTransitionTracing = false; -export const useModernStrictMode = false; +export const useModernStrictMode = true; export const enableDO_NOT_USE_disableStrictPassiveEffect = false; export const enableFizzExternalRuntime = false; export const enableDeferRootSchedulingToMicrotask = true; From 49b1a956a915da972e60221e9610d383fac08bd7 Mon Sep 17 00:00:00 2001 From: Ricky Date: Mon, 16 Dec 2024 22:51:15 -0500 Subject: [PATCH 16/18] Enable disableDefaultPropsExceptForClasses (#31804) TODO: test this PR to see what internal tests fail --- packages/shared/forks/ReactFeatureFlags.test-renderer.www.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index d1fe7629901ec..233a847ec37f7 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -72,7 +72,7 @@ export const enableInfiniteRenderLoopDetection = false; export const enableReactTestRendererWarning = false; export const disableLegacyMode = true; -export const disableDefaultPropsExceptForClasses = false; +export const disableDefaultPropsExceptForClasses = true; export const renameElementSymbol = false; From 975cea2d3ddb95ad31f10ae112bdde5101725c85 Mon Sep 17 00:00:00 2001 From: Ricky Date: Mon, 16 Dec 2024 22:52:18 -0500 Subject: [PATCH 17/18] Enable debugRenderPhaseSideEffectsForStrictMode in test renderers (#31761) This flag controls the strict mode double invoke render/lifecycles/etc behavior in Strict Mode. The only place this flag is off is the test renderers, which it should be on for. If we can land this, we can follow up to remove the flag. --- packages/shared/forks/ReactFeatureFlags.test-renderer.js | 2 +- .../shared/forks/ReactFeatureFlags.test-renderer.native-fb.js | 2 +- packages/shared/forks/ReactFeatureFlags.test-renderer.www.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 8281009889b21..53057d78d7390 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -10,7 +10,7 @@ import typeof * as FeatureFlagsType from 'shared/ReactFeatureFlags'; import typeof * as ExportsType from './ReactFeatureFlags.test-renderer'; -export const debugRenderPhaseSideEffectsForStrictMode = false; +export const debugRenderPhaseSideEffectsForStrictMode = __DEV__; export const enableAsyncDebugInfo = false; export const enableSchedulingProfiler = false; export const enableProfilerTimer = __PROFILE__; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index 8f71306c51a62..66e4329cb829d 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -11,7 +11,7 @@ import typeof * as FeatureFlagsType from 'shared/ReactFeatureFlags'; import typeof * as ExportsType from './ReactFeatureFlags.test-renderer'; export const alwaysThrottleRetries = false; -export const debugRenderPhaseSideEffectsForStrictMode = false; +export const debugRenderPhaseSideEffectsForStrictMode = __DEV__; export const disableClientCache = true; export const disableCommentsAsDOMContainers = true; export const disableDefaultPropsExceptForClasses = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 233a847ec37f7..b71ff7a266b4a 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -10,7 +10,7 @@ import typeof * as FeatureFlagsType from 'shared/ReactFeatureFlags'; import typeof * as ExportsType from './ReactFeatureFlags.test-renderer.www'; -export const debugRenderPhaseSideEffectsForStrictMode = false; +export const debugRenderPhaseSideEffectsForStrictMode = __DEV__; export const enableAsyncDebugInfo = false; export const enableSchedulingProfiler = false; export const enableProfilerTimer = __PROFILE__; From d42872588282b9eef56b8fa02441b33d596fd197 Mon Sep 17 00:00:00 2001 From: Ricky Date: Tue, 17 Dec 2024 10:27:46 -0500 Subject: [PATCH 18/18] [flags] Clean up scheduler flags (#31814) These flags are hardcoded now, we can make them static. --- .../forks/SchedulerFeatureFlags.www-dynamic.js | 3 --- .../src/forks/SchedulerFeatureFlags.www.js | 15 +++++++-------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/packages/scheduler/src/forks/SchedulerFeatureFlags.www-dynamic.js b/packages/scheduler/src/forks/SchedulerFeatureFlags.www-dynamic.js index 9346663083bc6..e20a8e7a3a5de 100644 --- a/packages/scheduler/src/forks/SchedulerFeatureFlags.www-dynamic.js +++ b/packages/scheduler/src/forks/SchedulerFeatureFlags.www-dynamic.js @@ -11,7 +11,4 @@ // Use __VARIANT__ to simulate a GK. The tests will be run twice: once // with the __VARIANT__ set to `true`, and once set to `false`. -export const userBlockingPriorityTimeout = 250; -export const normalPriorityTimeout = 5000; -export const lowPriorityTimeout = 10000; export const enableRequestPaint = __VARIANT__; diff --git a/packages/scheduler/src/forks/SchedulerFeatureFlags.www.js b/packages/scheduler/src/forks/SchedulerFeatureFlags.www.js index ec8498ec6adb5..c65e2aea241f9 100644 --- a/packages/scheduler/src/forks/SchedulerFeatureFlags.www.js +++ b/packages/scheduler/src/forks/SchedulerFeatureFlags.www.js @@ -10,13 +10,12 @@ // $FlowFixMe[cannot-resolve-module] const dynamicFeatureFlags = require('SchedulerFeatureFlags'); -export const { - userBlockingPriorityTimeout, - normalPriorityTimeout, - lowPriorityTimeout, - enableRequestPaint, -} = dynamicFeatureFlags; +export const {enableRequestPaint} = dynamicFeatureFlags; -export const frameYieldMs = 10; -export const enableSchedulerDebugging = true; +export const enableSchedulerDebugging = false; export const enableProfiling = __DEV__; +export const frameYieldMs = 10; + +export const userBlockingPriorityTimeout = 250; +export const normalPriorityTimeout = 5000; +export const lowPriorityTimeout = 10000;