diff --git a/packages/react-devtools-inline/__tests__/__e2e__/components.test.js b/packages/react-devtools-inline/__tests__/__e2e__/components.test.js index 3d187c3ddaba..9c4dd6af5bac 100644 --- a/packages/react-devtools-inline/__tests__/__e2e__/components.test.js +++ b/packages/react-devtools-inline/__tests__/__e2e__/components.test.js @@ -220,12 +220,20 @@ test.describe('Components', () => { window.REACT_DOM_DEVTOOLS; const container = document.getElementById('devtools'); - const element = findAllNodes(container, [ + // The current result index is an editable input, so its value is not + // part of the wrapper's innerText. Combine the input value with the + // total result count to reconstruct the "X | Y" label. + const indexInput = findAllNodes(container, [ + createTestNameSelector('ComponentSearchInput-ResultIndexInput'), + ])[0]; + const resultsCount = findAllNodes(container, [ createTestNameSelector('ComponentSearchInput-ResultsCount'), ])[0]; - return element !== undefined - ? element.innerText === expectedElementText - : false; + if (indexInput === undefined || resultsCount === undefined) { + return false; + } + const totalCount = resultsCount.innerText.replace(/[^0-9]/g, ''); + return `${indexInput.value} | ${totalCount}` === expectedElementText; }, text); } diff --git a/packages/react-devtools-shared/src/__tests__/treeContext-test.js b/packages/react-devtools-shared/src/__tests__/treeContext-test.js index c48aec8f10ac..5c5f6dff924c 100644 --- a/packages/react-devtools-shared/src/__tests__/treeContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/treeContext-test.js @@ -1078,6 +1078,172 @@ describe('TreeListContext', () => { `); }); + it('should jump directly to a specific search result by index', () => { + const Foo = () => null; + const Bar = () => null; + const Baz = () => null; + + utils.act(() => + render( + + + + + + , + ), + ); + + let renderer; + utils.act(() => (renderer = TestRenderer.create())); + + // search for "ba" (matches both elements and ) + utils.act(() => dispatch({type: 'SET_SEARCH_TEXT', payload: 'ba'})); + utils.act(() => renderer.update()); + expect(state).toMatchInlineSnapshot(` + [root] + + → + + + `); + + // jump directly to the third result + utils.act(() => dispatch({type: 'GO_TO_SEARCH_RESULT', payload: 2})); + utils.act(() => renderer.update()); + expect(state).toMatchInlineSnapshot(` + [root] + + + + → + `); + + // jump directly back to the first result + utils.act(() => dispatch({type: 'GO_TO_SEARCH_RESULT', payload: 0})); + utils.act(() => renderer.update()); + expect(state).toMatchInlineSnapshot(` + [root] + + → + + + `); + + // out-of-range indices are clamped to the valid range + utils.act(() => dispatch({type: 'GO_TO_SEARCH_RESULT', payload: 99})); + utils.act(() => renderer.update()); + expect(state).toMatchInlineSnapshot(` + [root] + + + + → + `); + + utils.act(() => dispatch({type: 'GO_TO_SEARCH_RESULT', payload: -5})); + utils.act(() => renderer.update()); + expect(state).toMatchInlineSnapshot(` + [root] + + → + + + `); + }); + + it('should do nothing when jumping to a result with no search matches', () => { + const Foo = () => null; + const Bar = () => null; + const Baz = () => null; + + utils.act(() => + render( + + + + + + , + ), + ); + + let renderer; + utils.act(() => (renderer = TestRenderer.create())); + + utils.act(() => dispatch({type: 'SET_SEARCH_TEXT', payload: 'nomatch'})); + utils.act(() => renderer.update()); + expect(state.searchResults).toHaveLength(0); + expect(state.searchIndex).toBe(null); + + utils.act(() => dispatch({type: 'GO_TO_SEARCH_RESULT', payload: 0})); + utils.act(() => renderer.update()); + expect(state.searchIndex).toBe(null); + expect(state.inspectedElementID).toBe(null); + expect(state).toMatchInlineSnapshot(` + [root] + + + + + `); + }); + + it('should advance past the selected result when retyping the same search', () => { + const Foo = () => null; + const Bar = () => null; + const Baz = () => null; + + utils.act(() => + render( + + + + + + , + ), + ); + + let renderer; + utils.act(() => (renderer = TestRenderer.create())); + + // search for "ba" and step to the second result () + utils.act(() => dispatch({type: 'SET_SEARCH_TEXT', payload: 'ba'})); + utils.act(() => dispatch({type: 'GO_TO_NEXT_SEARCH_RESULT'})); + utils.act(() => renderer.update()); + expect(state).toMatchInlineSnapshot(` + [root] + + + → + + `); + + // clear the search; the matched element stays selected + utils.act(() => dispatch({type: 'SET_SEARCH_TEXT', payload: ''})); + utils.act(() => renderer.update()); + expect(state).toMatchInlineSnapshot(` + [root] + + + → + + `); + + // retype the same query: instead of snapping back to the still-selected + // , the search advances to the next match (find-next semantics) + utils.act(() => dispatch({type: 'SET_SEARCH_TEXT', payload: 'ba'})); + utils.act(() => renderer.update()); + expect(state).toMatchInlineSnapshot(` + [root] + + + + → + `); + }); + it('should add newly mounted elements to the search results set if they match the current text', async () => { const Foo = () => null; const Bar = () => null; diff --git a/packages/react-devtools-shared/src/__tests__/utils-test.js b/packages/react-devtools-shared/src/__tests__/utils-test.js index 3d42aa28741c..03946618766f 100644 --- a/packages/react-devtools-shared/src/__tests__/utils-test.js +++ b/packages/react-devtools-shared/src/__tests__/utils-test.js @@ -11,7 +11,9 @@ import { getDisplayName, getDisplayNameForReactElement, isPlainObject, + printOperationsArray, } from 'react-devtools-shared/src/utils'; +import {TREE_OPERATION_APPLIED_ACTIVITY_SLICE_CHANGE} from 'react-devtools-shared/src/constants'; import {stackToComponentLocations} from 'react-devtools-shared/src/devtools/utils'; import { formatConsoleArguments, @@ -147,6 +149,20 @@ describe('utils', () => { ).toEqual('a 123 b true c'); }); + it('should support integer substitutions', () => { + expect(formatConsoleArgumentsToSingleString('%i', 3.14)).toEqual('3'); + }); + + it('should support float substitutions', () => { + expect(formatConsoleArgumentsToSingleString('%f', 3.5)).toEqual('3.5'); + }); + + it('should keep argument alignment across mixed substitutions', () => { + expect(formatConsoleArgumentsToSingleString('a %i b %s', 7, 'x')).toEqual( + 'a 7 b x', + ); + }); + it('should gracefully handle Symbol types', () => { expect( formatConsoleArgumentsToSingleString(Symbol('a'), 'b', Symbol('c')), @@ -509,5 +525,48 @@ function f() { } ]); expect(formatConsoleArguments('%s 100%', 'done')).toEqual(['done 100%']); }); + + it('keeps specifiers literal when no argument is supplied', () => { + expect(formatConsoleArguments('%s %s', 'the')).toEqual(['the %s']); + expect(formatConsoleArguments('%s %d', 'value')).toEqual(['value %d']); + expect(formatConsoleArguments('%s %i', 'value')).toEqual(['value %i']); + expect(formatConsoleArguments('%s %f', 'value')).toEqual(['value %f']); + }); + }); + + describe('printOperationsArray', () => { + let log; + beforeEach(() => { + log = jest.spyOn(console, 'log').mockImplementation(() => {}); + }); + afterEach(() => { + log.mockRestore(); + }); + + // The operation is [opcode, activitySliceID] (2 slots). A trailing operation + // after it verifies that the reader advances past the value slot instead of + // re-reading it as the next opcode. + it('should log an applied activity slice change and advance past its value', () => { + const rendererID = 1; + const rootID = 1; + const stringTableSize = 0; + const operations = [ + rendererID, + rootID, + stringTableSize, + TREE_OPERATION_APPLIED_ACTIVITY_SLICE_CHANGE, + 42, + TREE_OPERATION_APPLIED_ACTIVITY_SLICE_CHANGE, + 0, + ]; + + expect(() => printOperationsArray(operations)).not.toThrow(); + + expect(log).toHaveBeenCalledTimes(1); + expect(log.mock.calls[0][0]).toContain( + 'Applied activity slice change to 42', + ); + expect(log.mock.calls[0][0]).toContain('Reset applied activity slice'); + }); }); }); diff --git a/packages/react-devtools-shared/src/backend/utils/formatConsoleArguments.js b/packages/react-devtools-shared/src/backend/utils/formatConsoleArguments.js index 551f4cf674b8..8b699f5e9b37 100644 --- a/packages/react-devtools-shared/src/backend/utils/formatConsoleArguments.js +++ b/packages/react-devtools-shared/src/backend/utils/formatConsoleArguments.js @@ -45,18 +45,36 @@ export default function formatConsoleArguments( } case 'd': case 'i': { + if (argumentsPointer >= args.length) { + // No argument left for this specifier. Keep it as a literal, like + // the browser console does, rather than emitting 'NaN'. + template += `%${nextChar}`; + break; + } const [arg] = args.splice(argumentsPointer, 1); template += parseInt(arg, 10).toString(); break; } case 'f': { + if (argumentsPointer >= args.length) { + // No argument left for this specifier. Keep it as a literal, like + // the browser console does, rather than emitting 'NaN'. + template += `%${nextChar}`; + break; + } const [arg] = args.splice(argumentsPointer, 1); template += parseFloat(arg).toString(); break; } case 's': { + if (argumentsPointer >= args.length) { + // No argument left for this specifier. Keep it as a literal, like + // the browser console does, rather than emitting 'undefined'. + template += `%${nextChar}`; + break; + } const [arg] = args.splice(argumentsPointer, 1); template += String(arg); diff --git a/packages/react-devtools-shared/src/backend/utils/index.js b/packages/react-devtools-shared/src/backend/utils/index.js index 816145fdb19d..198364ec304c 100644 --- a/packages/react-devtools-shared/src/backend/utils/index.js +++ b/packages/react-devtools-shared/src/backend/utils/index.js @@ -193,7 +193,7 @@ export function formatConsoleArgumentsToSingleString( // If the first argument is a string, check for substitutions. if (typeof maybeMessage === 'string') { if (args.length) { - const REGEXP = /(%?)(%([jds]))/g; + const REGEXP = /(%?)(%([jdisf]))/g; // $FlowFixMe[incompatible-type] formatted = formatted.replace(REGEXP, (match, escaped, ptn, flag) => { diff --git a/packages/react-devtools-shared/src/devtools/views/Components/ComponentSearchInput.js b/packages/react-devtools-shared/src/devtools/views/Components/ComponentSearchInput.js index 654fe7691837..81506b673d7c 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/ComponentSearchInput.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/ComponentSearchInput.js @@ -36,11 +36,17 @@ export default function ComponentSearchInput(): React.Node { () => transitionDispatch({type: 'GO_TO_PREVIOUS_SEARCH_RESULT'}), [transitionDispatch], ); + const goToResult = useCallback( + (index: number) => + transitionDispatch({type: 'GO_TO_SEARCH_RESULT', payload: index}), + [transitionDispatch], + ); return ( , Map, null | Element['id']], @@ -129,6 +133,7 @@ type ACTION_SET_SEARCH_TEXT = { type Action = | ACTION_GO_TO_NEXT_SEARCH_RESULT | ACTION_GO_TO_PREVIOUS_SEARCH_RESULT + | ACTION_GO_TO_SEARCH_RESULT | ACTION_HANDLE_STORE_MUTATION | ACTION_RESET_OWNER_STACK | ACTION_SELECT_CHILD_ELEMENT_IN_TREE @@ -525,6 +530,19 @@ function reduceSearchState(store: Store, state: State, action: Action): State { : numPrevSearchResults - 1; } break; + case 'GO_TO_SEARCH_RESULT': + if (numPrevSearchResults > 0) { + didRequestSearch = true; + // Jump directly to a specific result (0-based), clamped to range. + // This lets users skip past large virtualized lists instead of + // stepping through results one at a time. + const targetIndex = (action as ACTION_GO_TO_SEARCH_RESULT).payload; + searchIndex = Math.max( + 0, + Math.min(targetIndex, numPrevSearchResults - 1), + ); + } + break; case 'HANDLE_STORE_MUTATION': if (searchText !== '') { const [addedElementIDs, removedElementIDs] = ( @@ -630,13 +648,21 @@ function reduceSearchState(store: Store, state: State, action: Action): State { if (searchText !== prevSearchText) { // $FlowFixMe[incompatible-type] const newSearchIndex = searchResults.indexOf(inspectedElementID); - if (newSearchIndex === -1) { - // Only move the selection if the new query - // doesn't match the current selection anymore. + if (prevSearchText === '') { + // Starting a fresh search (e.g. after clearing the box). Honor the index + // computed above, which uses "find next" semantics so that retyping the + // same query advances past the still-selected result instead of snapping + // back to it. + if (searchIndex !== null) { + didRequestSearch = true; + } + } else if (newSearchIndex === -1) { + // Refining an existing query and the current selection no longer matches, + // so move the selection to the nearest result. didRequestSearch = true; } else { - // Selected item still matches the new search query. - // Adjust the index to reflect its position in new results. + // Refining an existing query and the current selection still matches. + // Keep it selected and adjust the index to its position in new results. searchIndex = newSearchIndex; } } @@ -910,6 +936,7 @@ function TreeContextController({ switch (type) { case 'GO_TO_NEXT_SEARCH_RESULT': case 'GO_TO_PREVIOUS_SEARCH_RESULT': + case 'GO_TO_SEARCH_RESULT': case 'HANDLE_STORE_MUTATION': case 'RESET_OWNER_STACK': case 'SELECT_ELEMENT_AT_INDEX': @@ -1079,9 +1106,23 @@ function getNearestResultIndex( searchResults: Array, inspectedElementIndex: number, ): number { + // When the currently selected element is itself a match for the new query + // (e.g. you cleared the search and retyped the same text while a result was + // still selected), advance to the *next* match instead of snapping back to + // the same component. This mirrors "find next" semantics in browsers/editors + // and avoids the search feeling stuck on the same result. + const selectedIsResult = searchResults.some( + id => store.getIndexOfElementID(id) === inspectedElementIndex, + ); + const index = searchResults.findIndex(id => { const innerIndex = store.getIndexOfElementID(id); - return innerIndex !== null && innerIndex >= inspectedElementIndex; + if (innerIndex === null) { + return false; + } + return selectedIsResult + ? innerIndex > inspectedElementIndex + : innerIndex >= inspectedElementIndex; }); return index === -1 ? 0 : index; diff --git a/packages/react-devtools-shared/src/devtools/views/SearchInput.css b/packages/react-devtools-shared/src/devtools/views/SearchInput.css index 960bb152cb22..a61855eea09a 100644 --- a/packages/react-devtools-shared/src/devtools/views/SearchInput.css +++ b/packages/react-devtools-shared/src/devtools/views/SearchInput.css @@ -28,6 +28,35 @@ white-space: pre; } +.IndexInput { + color: var(--color-text); + font-size: var(--font-size-sans-normal); + font-family: inherit; + text-align: center; + background: none; + /* A visible border so it's clear this number can be edited. */ + border: 1px solid var(--color-border); + border-radius: 0.125rem; + outline: none; + padding: 0 0.25rem; + margin: 0; + /* Keep a floor so the box doesn't shrink/jitter as the digit count changes. */ + min-width: 1.5ch; +} + +/* AutoSizeInput sizes the element width to fit the text exactly (assuming no + padding/border). content-box makes our padding + border add around that + width rather than eating into it and clipping digits. The descendant + selector raises specificity above the global `.DevTools *` border-box rule, + which otherwise wins by source order at equal specificity. */ +.IndexLabel .IndexInput { + box-sizing: content-box; +} + +.IndexInput:focus { + background-color: var(--color-button-background-focus); +} + .LeftVRule{ height: 20px; width: 1px; diff --git a/packages/react-devtools-shared/src/devtools/views/SearchInput.js b/packages/react-devtools-shared/src/devtools/views/SearchInput.js index 3d94128bd67b..745d9cdc51bf 100644 --- a/packages/react-devtools-shared/src/devtools/views/SearchInput.js +++ b/packages/react-devtools-shared/src/devtools/views/SearchInput.js @@ -7,17 +7,24 @@ * @flow */ +import typeof { + SyntheticEvent, + SyntheticKeyboardEvent, +} from 'react-dom-bindings/src/events/SyntheticEvent'; + import * as React from 'react'; -import {useEffect, useRef} from 'react'; +import {useEffect, useRef, useState} from 'react'; import Button from './Button'; import ButtonIcon from './ButtonIcon'; import Icon from './Icon'; +import AutoSizeInput from './Components/NativeStyleEditor/AutoSizeInput'; import styles from './SearchInput.css'; type Props = { goToNextResult: () => void, goToPreviousResult: () => void, + goToResult: (index: number) => void, placeholder: string, search: (text: string) => void, searchIndex: number, @@ -29,6 +36,7 @@ type Props = { export default function SearchInput({ goToNextResult, goToPreviousResult, + goToResult, placeholder, search, searchIndex, @@ -38,6 +46,37 @@ export default function SearchInput({ }: Props): React.Node { const inputRef = useRef(null); + const [indexDraft, setIndexDraft] = useState(null); + const currentResultNumber = Math.min(searchIndex + 1, searchResultsCount); + const indexValue = + indexDraft !== null ? indexDraft : String(currentResultNumber); + + const handleIndexChange = (event: SyntheticEvent) => { + // Only digits are meaningful here; strip anything else as it's typed. + const raw = event.currentTarget.value.replace(/[^0-9]/g, ''); + + if (raw === '' || searchResultsCount === 0) { + setIndexDraft(raw); + return; + } + + // Clamp into [1, searchResultsCount] so the field never displays an + // out-of-range value, then live-preview by scrolling to that result. + const clamped = Math.max( + 1, + Math.min(parseInt(raw, 10), searchResultsCount), + ); + setIndexDraft(String(clamped)); + goToResult(clamped - 1); + }; + const handleIndexBlur = () => setIndexDraft(null); + const handleIndexKeyDown = (event: SyntheticKeyboardEvent) => { + if (event.key === 'Enter' || event.key === 'Escape') { + event.preventDefault(); + event.currentTarget.blur(); + } + }; + const resetSearch = () => search(''); // $FlowFixMe[missing-local-annot] @@ -103,7 +142,21 @@ export default function SearchInput({ - {Math.min(searchIndex + 1, searchResultsCount)} |{' '} + + {' | '} {searchResultsCount}
diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js index 22e4c444ddfa..65609ab25321 100644 --- a/packages/react-devtools-shared/src/utils.js +++ b/packages/react-devtools-shared/src/utils.js @@ -440,7 +440,7 @@ export function printOperationsArray(operations: Array) { } case TREE_OPERATION_APPLIED_ACTIVITY_SLICE_CHANGE: { i++; - const activitySliceIDChange = operations[i + 1]; + const activitySliceIDChange = operations[i++]; logs.push( activitySliceIDChange === 0 ? 'Reset applied activity slice' diff --git a/packages/react-devtools-shell/src/app/SearchableTable/index.js b/packages/react-devtools-shell/src/app/SearchableTable/index.js new file mode 100644 index 000000000000..f9069c6dfe88 --- /dev/null +++ b/packages/react-devtools-shell/src/app/SearchableTable/index.js @@ -0,0 +1,81 @@ +/** + * 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. + * + * @flow + */ + +import * as React from 'react'; +import {Fragment} from 'react'; + +// A large tree of similarly-named components (Table, TableRow, TableCell, ...). +// This mirrors a real virtualized table and is meant for exercising the +// component-tree search box in DevTools: +// 1. Open DevTools and search "Table" — there are 100+ matches. +// 2. Type a number into the result-index field (left of "| N") to jump +// directly to a specific match instead of scrolling/pressing Enter. +// 3. Select a match, clear the search, then retype the same text — the +// search advances to the *next* match instead of snapping back. + +const ROWS = 25; +const COLS = 4; + +function TableCell({row, col}: {row: number, col: number}): React.Node { + return {`r${row}c${col}`}; +} + +function TableColumnHeader({col}: {col: number}): React.Node { + return {`Column ${col}`}; +} + +function TableRow({row}: {row: number}): React.Node { + return ( + + {Array.from({length: COLS}, (_, col) => ( + + ))} + + ); +} + +function TableHeaderRow(): React.Node { + return ( + + {Array.from({length: COLS}, (_, col) => ( + + ))} + + ); +} + +function TableBody(): React.Node { + return ( + + {Array.from({length: ROWS}, (_, row) => ( + + ))} + + ); +} + +function Table(): React.Node { + return ( + + + + + +
+ ); +} + +export default function SearchableTable(): React.Node { + return ( + +

Searchable Table

+ + + ); +} diff --git a/packages/react-devtools-shell/src/app/index.js b/packages/react-devtools-shell/src/app/index.js index 0207a8934c2f..16c140707057 100644 --- a/packages/react-devtools-shell/src/app/index.js +++ b/packages/react-devtools-shell/src/app/index.js @@ -20,6 +20,7 @@ import ErrorBoundaries from './ErrorBoundaries'; import PartiallyStrictApp from './PartiallyStrictApp'; import Segments from './Segments'; import SuspenseTree from './SuspenseTree'; +import SearchableTable from './SearchableTable'; import ActivityTree from './ActivityTree'; import TraceUpdatesTest from './TraceUpdatesTest'; import {ignoreErrors, ignoreLogs, ignoreWarnings} from './console'; @@ -113,6 +114,7 @@ function mountTestApp() { mountApp(Toggle); mountApp(ErrorBoundaries); mountApp(SuspenseTree); + mountApp(SearchableTable); mountApp(DeeplyNestedComponents); mountApp(Iframe); mountApp(ActivityTree); diff --git a/packages/react/src/jsx/ReactJSXElement.js b/packages/react/src/jsx/ReactJSXElement.js index 3a5a5d51a9b8..0d1b46f75722 100644 --- a/packages/react/src/jsx/ReactJSXElement.js +++ b/packages/react/src/jsx/ReactJSXElement.js @@ -860,7 +860,6 @@ export function cloneElement(element, config, children) { * * @internal * @param {ReactNode} node Statically passed child of any type. - * @param {*} parentType node's parent's type. */ function validateChildKeys(node) { if (__DEV__) {