From fe84397e81c94a7bccdf0479994a7d0363a12115 Mon Sep 17 00:00:00 2001 From: Eugene Choi <4eugenechoi@gmail.com> Date: Thu, 11 Sep 2025 11:51:32 -0400 Subject: [PATCH 1/5] [compiler][playground] (4/N) Config override panel (#34436) ## Summary Removed the old `OVERRIDE` pragma to make the source of truth for config overrides in the left-hand pane. Now, it will automatically update the output pane each time there is an edit to the config. The old pragma format is still supported, but it will be overwritten by the config pane if they are modifying the same flags. Removed the gating on the config panel so now all users will automatically be able to view it, but it will be initially collapsed. ## How did you test this change? https://github.com/user-attachments/assets/9d4512b9-e203-4ce0-ae95-dd96ff03bbc1 --- .../components/Editor/ConfigEditor.tsx | 129 +++++------------- .../components/Editor/EditorImpl.tsx | 40 ++++-- .../playground/components/Editor/Input.tsx | 7 +- .../playground/components/StoreContext.tsx | 18 ++- compiler/apps/playground/lib/configUtils.ts | 120 ---------------- compiler/apps/playground/lib/defaultStore.ts | 19 +-- .../src/Utils/TestUtils.ts | 102 -------------- 7 files changed, 80 insertions(+), 355 deletions(-) delete mode 100644 compiler/apps/playground/lib/configUtils.ts diff --git a/compiler/apps/playground/components/Editor/ConfigEditor.tsx b/compiler/apps/playground/components/Editor/ConfigEditor.tsx index 43b3f1e8a9164..63522987db052 100644 --- a/compiler/apps/playground/components/Editor/ConfigEditor.tsx +++ b/compiler/apps/playground/components/Editor/ConfigEditor.tsx @@ -10,14 +10,8 @@ import type {editor} from 'monaco-editor'; import * as monaco from 'monaco-editor'; import React, {useState, useCallback} from 'react'; import {Resizable} from 're-resizable'; -import {useSnackbar} from 'notistack'; import {useStore, useStoreDispatch} from '../StoreContext'; import {monacoOptions} from './monacoOptions'; -import { - ConfigError, - generateOverridePragmaFromConfig, - updateSourceWithOverridePragma, -} from '../../lib/configUtils'; // @ts-expect-error - webpack asset/source loader handles .d.ts files as strings import compilerTypeDefs from 'babel-plugin-react-compiler/dist/index.d.ts'; @@ -28,61 +22,17 @@ export default function ConfigEditor(): React.ReactElement { const [isExpanded, setIsExpanded] = useState(false); const store = useStore(); const dispatchStore = useStoreDispatch(); - const {enqueueSnackbar} = useSnackbar(); const toggleExpanded = useCallback(() => { setIsExpanded(prev => !prev); }, []); - const handleApplyConfig: () => Promise = async () => { - try { - const config = store.config || ''; - - if (!config.trim()) { - enqueueSnackbar( - 'Config is empty. Please add configuration options first.', - { - variant: 'warning', - }, - ); - return; - } - const newPragma = await generateOverridePragmaFromConfig(config); - const updatedSource = updateSourceWithOverridePragma( - store.source, - newPragma, - ); - - dispatchStore({ - type: 'updateFile', - payload: { - source: updatedSource, - config: config, - }, - }); - } catch (error) { - console.error('Failed to apply config:', error); - - if (error instanceof ConfigError && error.message.trim()) { - enqueueSnackbar(error.message, { - variant: 'error', - }); - } else { - enqueueSnackbar('Unexpected error: failed to apply config.', { - variant: 'error', - }); - } - } - }; - const handleChange: (value: string | undefined) => void = value => { if (value === undefined) return; - // Only update the config dispatchStore({ - type: 'updateFile', + type: 'updateConfig', payload: { - source: store.source, config: value, }, }); @@ -120,49 +70,40 @@ export default function ConfigEditor(): React.ReactElement { return (
{isExpanded ? ( - <> - -

- - Config Overrides -

-
- -
- - - + +

+ - Config Overrides +

+
+ +
+ ) : (
diff --git a/packages/react-devtools-shared/src/devtools/views/Components/ForgetBadge.css b/packages/react-devtools-shared/src/devtools/views/Components/ForgetBadge.css index e80f37083345e..415f8e96fd759 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/ForgetBadge.css +++ b/packages/react-devtools-shared/src/devtools/views/Components/ForgetBadge.css @@ -11,11 +11,3 @@ position: absolute; right: 0.25em; } - -.ForgetToggle { - display: flex; -} - -.ForgetToggle > span { /* targets .ToggleContent */ - padding: 0; -} diff --git a/packages/react-devtools-shared/src/devtools/views/Components/ForgetBadge.js b/packages/react-devtools-shared/src/devtools/views/Components/ForgetBadge.js index 00b4f0db13f9b..5048f3085f710 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/ForgetBadge.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/ForgetBadge.js @@ -11,7 +11,7 @@ import * as React from 'react'; import Badge from './Badge'; import IndexableDisplayName from './IndexableDisplayName'; -import Toggle from '../Toggle'; +import Tooltip from './reach-ui/tooltip'; import styles from './ForgetBadge.css'; @@ -40,12 +40,11 @@ export default function ForgetBadge(props: Props): React.Node { 'Memo' ); - const onChange = () => {}; const title = '✨ This component has been auto-memoized by the React Compiler.'; return ( - + {innerView} - + ); } diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js index 200f78586c51a..5596257fa5df5 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js @@ -29,6 +29,7 @@ import InspectedElementViewSourceButton from './InspectedElementViewSourceButton import useEditorURL from '../useEditorURL'; import styles from './InspectedElement.css'; +import Tooltip from './reach-ui/tooltip'; export type Props = {}; @@ -192,14 +193,15 @@ export default function InspectedElementWrapper(_: Props): React.Node { let strictModeBadge = null; if (element.isStrictModeNonCompliant) { strictModeBadge = ( - - - + + + + + ); } diff --git a/packages/react-devtools-shared/src/devtools/views/Components/NativeTagBadge.css b/packages/react-devtools-shared/src/devtools/views/Components/NativeTagBadge.css index 7188a53743432..cebc617e69cfc 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/NativeTagBadge.css +++ b/packages/react-devtools-shared/src/devtools/views/Components/NativeTagBadge.css @@ -1,11 +1,3 @@ -.Toggle { - display: flex; -} - -.Toggle > span { /* targets .ToggleContent */ - padding: 0; -} - .Badge { cursor: help; } diff --git a/packages/react-devtools-shared/src/devtools/views/Components/NativeTagBadge.js b/packages/react-devtools-shared/src/devtools/views/Components/NativeTagBadge.js index 118255b536a09..c092891ec9dd3 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/NativeTagBadge.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/NativeTagBadge.js @@ -10,7 +10,7 @@ import * as React from 'react'; import Badge from './Badge'; -import Toggle from '../Toggle'; +import Tooltip from './reach-ui/tooltip'; import styles from './NativeTagBadge.css'; @@ -18,14 +18,13 @@ type Props = { nativeTag: number, }; -const noop = () => {}; const title = 'Unique identifier for the corresponding native component. React Native only.'; export default function NativeTagBadge({nativeTag}: Props): React.Node { return ( - + Tag {nativeTag} - + ); } diff --git a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ReportNewIssue.js b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ReportNewIssue.js index 360a6c232b072..f8dc401365dce 100644 --- a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ReportNewIssue.js +++ b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ReportNewIssue.js @@ -63,8 +63,7 @@ export default function ReportNewIssue({ className={styles.ReportLink} href={bugURL} rel="noopener noreferrer" - target="_blank" - title="Report bug"> + target="_blank"> Report this issue
diff --git a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/WorkplaceGroup.js b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/WorkplaceGroup.js index 1e04c304a743d..c72edc06f1c9a 100644 --- a/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/WorkplaceGroup.js +++ b/packages/react-devtools-shared/src/devtools/views/ErrorBoundary/WorkplaceGroup.js @@ -25,8 +25,7 @@ export default function WorkplaceGroup(): React.Node { className={styles.ReportLink} href={REACT_DEVTOOLS_WORKPLACE_URL} rel="noopener noreferrer" - target="_blank" - title="Report bug"> + target="_blank"> Report this on Workplace
(Facebook employees only.)
diff --git a/packages/react-devtools-shared/src/devtools/views/Icon.js b/packages/react-devtools-shared/src/devtools/views/Icon.js index f65f331a12d44..f49bb4fd0380c 100644 --- a/packages/react-devtools-shared/src/devtools/views/Icon.js +++ b/packages/react-devtools-shared/src/devtools/views/Icon.js @@ -33,12 +33,14 @@ type Props = { className?: string, title?: string, type: IconType, + ... }; export default function Icon({ className = '', title = '', type, + ...props }: Props): React.Node { let pathData = null; let viewBox = '0 0 24 24'; @@ -102,6 +104,7 @@ export default function Icon({ return ( Date: Thu, 11 Sep 2025 19:13:14 +0200 Subject: [PATCH 4/5] [DevTools] Stop recording reorders in disconnected subtrees (#34464) --- .../react-devtools-shared/src/backend/fiber/renderer.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 12e2ce31fb16f..3fe177e0797f1 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -4310,7 +4310,9 @@ export function attach( virtualLevel + 1, ); if ((updateFlags & ShouldResetChildren) !== NoUpdate) { - recordResetChildren(virtualInstance); + if (!isInDisconnectedSubtree) { + recordResetChildren(virtualInstance); + } updateFlags &= ~ShouldResetChildren; } removePreviousSuspendedBy( @@ -5097,7 +5099,9 @@ export function attach( // We need to crawl the subtree for closest non-filtered Fibers // so that we can display them in a flat children set. if (fiberInstance !== null && fiberInstance.kind === FIBER_INSTANCE) { - recordResetChildren(fiberInstance); + if (!nextIsHidden && !isInDisconnectedSubtree) { + recordResetChildren(fiberInstance); + } // We've handled the child order change for this Fiber. // Since it's included, there's no need to invalidate parent child order. From a9ad64c8524eb7a9af6753baa715a41909552fa6 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Thu, 11 Sep 2025 20:00:53 +0200 Subject: [PATCH 5/5] [DevTools] Stop mounting empty roots (#34467) --- .../src/__tests__/store-test.js | 5 ++- .../src/backend/fiber/renderer.js | 45 +++++++++---------- 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index fad4622acc15b..3b60d5ae093e4 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -2489,7 +2489,7 @@ describe('Store', () => { withErrorsOrWarningsIgnored(['test-only:'], async () => { await act(() => render()); }); - expect(store).toMatchInlineSnapshot(`[root]`); + expect(store).toMatchInlineSnapshot(``); expect(store.componentWithErrorCount).toBe(0); expect(store.componentWithWarningCount).toBe(0); }); @@ -3083,6 +3083,9 @@ describe('Store', () => { it('should handle an empty root', async () => { await actAsync(() => render(null)); + expect(store).toMatchInlineSnapshot(``); + + await actAsync(() => render()); expect(store).toMatchInlineSnapshot(`[root]`); }); }); diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 3fe177e0797f1..aee89e8ca2c54 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -5322,12 +5322,12 @@ export function attach( root: FiberRoot, priorityLevel: void | number, ) { - const current = root.current; + const nextFiber = root.current; let prevFiber: null | Fiber = null; let rootInstance = rootToFiberInstanceMap.get(root); if (!rootInstance) { - rootInstance = createFiberInstance(current); + rootInstance = createFiberInstance(nextFiber); rootToFiberInstanceMap.set(root, rootInstance); idToDevToolsInstanceMap.set(rootInstance.id, rootInstance); } else { @@ -5366,30 +5366,25 @@ export function attach( }; } - if (prevFiber !== null) { - // TODO: relying on this seems a bit fishy. - const wasMounted = - prevFiber.memoizedState != null && - prevFiber.memoizedState.element != null; - const isMounted = - current.memoizedState != null && current.memoizedState.element != null; - if (!wasMounted && isMounted) { - // Mount a new root. - setRootPseudoKey(currentRoot.id, current); - mountFiberRecursively(current, false); - } else if (wasMounted && isMounted) { - // Update an existing root. - updateFiberRecursively(rootInstance, current, prevFiber, false); - } else if (wasMounted && !isMounted) { - // Unmount an existing root. - unmountInstanceRecursively(rootInstance); - removeRootPseudoKey(currentRoot.id); - rootToFiberInstanceMap.delete(root); - } - } else { + const nextIsMounted = nextFiber.child !== null; + const prevWasMounted = prevFiber !== null && prevFiber.child !== null; + if (!prevWasMounted && nextIsMounted) { // Mount a new root. - setRootPseudoKey(currentRoot.id, current); - mountFiberRecursively(current, false); + setRootPseudoKey(currentRoot.id, nextFiber); + mountFiberRecursively(nextFiber, false); + } else if (prevWasMounted && nextIsMounted) { + if (prevFiber === null) { + throw new Error( + 'Expected a previous Fiber when updating an existing root.', + ); + } + // Update an existing root. + updateFiberRecursively(rootInstance, nextFiber, prevFiber, false); + } else if (prevWasMounted && !nextIsMounted) { + // Unmount an existing root. + unmountInstanceRecursively(rootInstance); + removeRootPseudoKey(currentRoot.id); + rootToFiberInstanceMap.delete(root); } if (isProfiling && isProfilingSupported) {