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 43b3f1e8a91..63522987db0 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 e80f3708334..415f8e96fd7 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 00b4f0db13f..5048f3085f7 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 200f78586c5..5596257fa5d 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 7188a537434..cebc617e69c 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 118255b536a..c092891ec9d 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 360a6c232b0..f8dc401365d 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 1e04c304a74..c72edc06f1c 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 f65f331a12d..f49bb4fd038 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 12e2ce31fb1..3fe177e0797 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 fad4622acc1..3b60d5ae093 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 3fe177e0797..aee89e8ca2c 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) {