Skip to content

Commit 404011c

Browse files
hoxyqhoxyq
committed
refactor: data source for errors and warnings tracking is now in Store
1 parent a5b2ad4 commit 404011c

File tree

8 files changed

+109
-83
lines changed

8 files changed

+109
-83
lines changed

‎packages/react-devtools-shared/src/__tests__/store-test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2148,8 +2148,8 @@ describe('Store', () => {
21482148
act(() => render(<React.Fragment />));
21492149
});
21502150
expect(store).toMatchInlineSnapshot(`[root]`);
2151-
expect(store.errorCount).toBe(0);
2152-
expect(store.warningCount).toBe(0);
2151+
expect(store.uniqueErrorCount).toBe(0);
2152+
expect(store.uniqueWarningCount).toBe(0);
21532153
});
21542154

21552155
// Regression test for https://github.com/facebook/react/issues/23202

‎packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -420,8 +420,8 @@ describe('Store component filters', () => {
420420
});
421421

422422
expect(store).toMatchInlineSnapshot(``);
423-
expect(store.errorCount).toBe(0);
424-
expect(store.warningCount).toBe(0);
423+
expect(store.uniqueErrorCount).toBe(0);
424+
expect(store.uniqueWarningCount).toBe(0);
425425

426426
await actAsync(async () => (store.componentFilters = []));
427427
expect(store).toMatchInlineSnapshot(`
@@ -460,8 +460,8 @@ describe('Store component filters', () => {
460460
]),
461461
);
462462
expect(store).toMatchInlineSnapshot(`[root]`);
463-
expect(store.errorCount).toBe(0);
464-
expect(store.warningCount).toBe(0);
463+
expect(store.uniqueErrorCount).toBe(0);
464+
expect(store.uniqueWarningCount).toBe(0);
465465

466466
await actAsync(async () => (store.componentFilters = []));
467467
expect(store).toMatchInlineSnapshot(`
@@ -510,8 +510,8 @@ describe('Store component filters', () => {
510510
});
511511

512512
expect(store).toMatchInlineSnapshot(``);
513-
expect(store.errorCount).toBe(0);
514-
expect(store.warningCount).toBe(0);
513+
expect(store.uniqueErrorCount).toBe(0);
514+
expect(store.uniqueWarningCount).toBe(0);
515515

516516
await actAsync(async () => (store.componentFilters = []));
517517
expect(store).toMatchInlineSnapshot(`
@@ -550,8 +550,8 @@ describe('Store component filters', () => {
550550
]),
551551
);
552552
expect(store).toMatchInlineSnapshot(`[root]`);
553-
expect(store.errorCount).toBe(0);
554-
expect(store.warningCount).toBe(0);
553+
expect(store.uniqueErrorCount).toBe(0);
554+
expect(store.uniqueWarningCount).toBe(0);
555555

556556
await actAsync(async () => (store.componentFilters = []));
557557
expect(store).toMatchInlineSnapshot(`

‎packages/react-devtools-shared/src/devtools/store.js

Lines changed: 53 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,8 @@ export default class Store extends EventEmitter<{
114114
_bridge: FrontendBridge;
115115

116116
// Computed whenever _errorsAndWarnings Map changes.
117-
_cachedErrorCount: number = 0;
118-
_cachedWarningCount: number = 0;
117+
_cachedUniqueErrorCount: number = 0;
118+
_cachedUniqueWarningCount: number = 0;
119119
_cachedErrorAndWarningTuples: ErrorAndWarningTuples | null = null;
120120

121121
// Should new nodes be collapsed by default when added to the tree?
@@ -196,6 +196,7 @@ export default class Store extends EventEmitter<{
196196

197197
_shouldCheckBridgeProtocolCompatibility: boolean = false;
198198
_hookSettings: $ReadOnly<DevToolsHookSettings> | null = null;
199+
_shouldShowWarningsAndErrors: boolean = false;
199200

200201
constructor(bridge: FrontendBridge, config?: Config) {
201202
super();
@@ -383,8 +384,24 @@ export default class Store extends EventEmitter<{
383384
return this._bridgeProtocol;
384385
}
385386

386-
get errorCount(): number {
387-
return this._cachedErrorCount;
387+
get uniqueErrorCount(): number {
388+
if (!this._shouldShowWarningsAndErrors) {
389+
return 0;
390+
}
391+
392+
return this._cachedUniqueErrorCount;
393+
}
394+
395+
get uniqueWarningCount(): number {
396+
if (!this._shouldShowWarningsAndErrors) {
397+
return 0;
398+
}
399+
400+
return this._cachedUniqueWarningCount;
401+
}
402+
403+
get displayingErrorsAndWarningsEnabled(): boolean {
404+
return this._shouldShowWarningsAndErrors;
388405
}
389406

390407
get hasOwnerMetadata(): boolean {
@@ -480,10 +497,6 @@ export default class Store extends EventEmitter<{
480497
return this._unsupportedRendererVersionDetected;
481498
}
482499

483-
get warningCount(): number {
484-
return this._cachedWarningCount;
485-
}
486-
487500
containsElement(id: number): boolean {
488501
return this._idToElement.has(id);
489502
}
@@ -581,7 +594,11 @@ export default class Store extends EventEmitter<{
581594
}
582595

583596
// Returns a tuple of [id, index]
584-
getElementsWithErrorsAndWarnings(): Array<{id: number, index: number}> {
597+
getElementsWithErrorsAndWarnings(): ErrorAndWarningTuples {
598+
if (!this._shouldShowWarningsAndErrors) {
599+
return [];
600+
}
601+
585602
if (this._cachedErrorAndWarningTuples !== null) {
586603
return this._cachedErrorAndWarningTuples;
587604
}
@@ -615,6 +632,10 @@ export default class Store extends EventEmitter<{
615632
errorCount: number,
616633
warningCount: number,
617634
} {
635+
if (!this._shouldShowWarningsAndErrors) {
636+
return {errorCount: 0, warningCount: 0};
637+
}
638+
618639
return this._errorsAndWarnings.get(id) || {errorCount: 0, warningCount: 0};
619640
}
620641

@@ -1325,16 +1346,21 @@ export default class Store extends EventEmitter<{
13251346
this._cachedErrorAndWarningTuples = null;
13261347

13271348
if (haveErrorsOrWarningsChanged) {
1328-
let errorCount = 0;
1329-
let warningCount = 0;
1349+
let uniqueErrorCount = 0;
1350+
let uniqueWarningCount = 0;
13301351

13311352
this._errorsAndWarnings.forEach(entry => {
1332-
errorCount += entry.errorCount;
1333-
warningCount += entry.warningCount;
1353+
if (entry.errorCount > 0) {
1354+
uniqueErrorCount++;
1355+
}
1356+
1357+
if (entry.warningCount > 0) {
1358+
uniqueWarningCount++;
1359+
}
13341360
});
13351361

1336-
this._cachedErrorCount = errorCount;
1337-
this._cachedWarningCount = warningCount;
1362+
this._cachedUniqueErrorCount = uniqueErrorCount;
1363+
this._cachedUniqueWarningCount = uniqueWarningCount;
13381364
}
13391365

13401366
if (haveRootsChanged) {
@@ -1528,9 +1554,21 @@ export default class Store extends EventEmitter<{
15281554
onHookSettings: (settings: $ReadOnly<DevToolsHookSettings>) => void =
15291555
settings => {
15301556
this._hookSettings = settings;
1557+
1558+
this.setShouldShowWarningsAndErrors(settings.showInlineWarningsAndErrors);
15311559
this.emit('hookSettings', settings);
15321560
};
15331561

1562+
setShouldShowWarningsAndErrors(status: boolean): void {
1563+
const previousStatus = this._shouldShowWarningsAndErrors;
1564+
this._shouldShowWarningsAndErrors = status;
1565+
1566+
if (previousStatus !== status) {
1567+
// Propagate to subscribers, although tree state has not changed
1568+
this.emit('mutated', [[], new Map()]);
1569+
}
1570+
}
1571+
15341572
// The Store should never throw an Error without also emitting an event.
15351573
// Otherwise Store errors will be invisible to users,
15361574
// but the downstream errors they cause will be reported as bugs.

‎packages/react-devtools-shared/src/devtools/views/Components/Element.js

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import {Fragment, useContext, useMemo, useState} from 'react';
1212
import Store from 'react-devtools-shared/src/devtools/store';
1313
import ButtonIcon from '../ButtonIcon';
1414
import {TreeDispatcherContext, TreeStateContext} from './TreeContext';
15-
import {SettingsContext} from '../Settings/SettingsContext';
1615
import {StoreContext} from '../context';
1716
import {useSubscription} from '../hooks';
1817
import {logEvent} from 'react-devtools-shared/src/Logger';
@@ -37,7 +36,6 @@ export default function Element({data, index, style}: Props): React.Node {
3736
const {ownerFlatTree, ownerID, selectedElementID} =
3837
useContext(TreeStateContext);
3938
const dispatch = useContext(TreeDispatcherContext);
40-
const {showInlineWarningsAndErrors} = React.useContext(SettingsContext);
4139

4240
const element =
4341
ownerFlatTree !== null
@@ -181,7 +179,7 @@ export default function Element({data, index, style}: Props): React.Node {
181179
className={styles.BadgesBlock}
182180
/>
183181

184-
{showInlineWarningsAndErrors && errorCount > 0 && (
182+
{errorCount > 0 && (
185183
<Icon
186184
type="error"
187185
className={
@@ -191,7 +189,7 @@ export default function Element({data, index, style}: Props): React.Node {
191189
}
192190
/>
193191
)}
194-
{showInlineWarningsAndErrors && warningCount > 0 && (
192+
{warningCount > 0 && (
195193
<Icon
196194
type="warning"
197195
className={

‎packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorsAndWarningsTree.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
import * as React from 'react';
1111
import {
12-
useContext,
1312
unstable_useCacheRefresh as useCacheRefresh,
1413
useTransition,
1514
} from 'react';
@@ -18,7 +17,6 @@ import ButtonIcon from '../ButtonIcon';
1817
import Store from '../../store';
1918
import sharedStyles from './InspectedElementSharedStyles.css';
2019
import styles from './InspectedElementErrorsAndWarningsTree.css';
21-
import {SettingsContext} from '../Settings/SettingsContext';
2220
import {
2321
clearErrorsForElement as clearErrorsForElementAPI,
2422
clearWarningsForElement as clearWarningsForElementAPI,
@@ -74,12 +72,14 @@ export default function InspectedElementErrorsAndWarningsTree({
7472
}
7573
};
7674

77-
const {showInlineWarningsAndErrors} = useContext(SettingsContext);
78-
if (!showInlineWarningsAndErrors) {
75+
if (!store.displayingErrorsAndWarningsEnabled) {
7976
return null;
8077
}
8178

8279
const {errors, warnings} = inspectedElement;
80+
if (errors.length === 0 && warnings.length === 0) {
81+
return null;
82+
}
8383

8484
return (
8585
<React.Fragment>

‎packages/react-devtools-shared/src/devtools/views/Components/Tree.js

Lines changed: 35 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ export default function Tree(props: Props): React.Node {
7373

7474
const [treeFocused, setTreeFocused] = useState<boolean>(false);
7575

76-
const {lineHeight, showInlineWarningsAndErrors} = useContext(SettingsContext);
76+
const {lineHeight} = useContext(SettingsContext);
7777

7878
// Make sure a newly selected element is visible in the list.
7979
// This is helpful for things like the owners list and search.
@@ -325,8 +325,8 @@ export default function Tree(props: Props): React.Node {
325325
const errorsOrWarningsSubscription = useMemo(
326326
() => ({
327327
getCurrentValue: () => ({
328-
errors: store.errorCount,
329-
warnings: store.warningCount,
328+
errors: store.uniqueErrorCount,
329+
warnings: store.uniqueWarningCount,
330330
}),
331331
subscribe: (callback: Function) => {
332332
store.addListener('mutated', callback);
@@ -370,40 +370,38 @@ export default function Tree(props: Props): React.Node {
370370
<Suspense fallback={<Loading />}>
371371
{ownerID !== null ? <OwnersStack /> : <ComponentSearchInput />}
372372
</Suspense>
373-
{showInlineWarningsAndErrors &&
374-
ownerID === null &&
375-
(errors > 0 || warnings > 0) && (
376-
<React.Fragment>
377-
<div className={styles.VRule} />
378-
{errors > 0 && (
379-
<div className={styles.IconAndCount}>
380-
<Icon className={styles.ErrorIcon} type="error" />
381-
{errors}
382-
</div>
383-
)}
384-
{warnings > 0 && (
385-
<div className={styles.IconAndCount}>
386-
<Icon className={styles.WarningIcon} type="warning" />
387-
{warnings}
388-
</div>
389-
)}
390-
<Button
391-
onClick={handlePreviousErrorOrWarningClick}
392-
title="Scroll to previous error or warning">
393-
<ButtonIcon type="up" />
394-
</Button>
395-
<Button
396-
onClick={handleNextErrorOrWarningClick}
397-
title="Scroll to next error or warning">
398-
<ButtonIcon type="down" />
399-
</Button>
400-
<Button
401-
onClick={clearErrorsAndWarnings}
402-
title="Clear all errors and warnings">
403-
<ButtonIcon type="clear" />
404-
</Button>
405-
</React.Fragment>
406-
)}
373+
{ownerID === null && (errors > 0 || warnings > 0) && (
374+
<React.Fragment>
375+
<div className={styles.VRule} />
376+
{errors > 0 && (
377+
<div className={styles.IconAndCount}>
378+
<Icon className={styles.ErrorIcon} type="error" />
379+
{errors}
380+
</div>
381+
)}
382+
{warnings > 0 && (
383+
<div className={styles.IconAndCount}>
384+
<Icon className={styles.WarningIcon} type="warning" />
385+
{warnings}
386+
</div>
387+
)}
388+
<Button
389+
onClick={handlePreviousErrorOrWarningClick}
390+
title="Scroll to previous error or warning">
391+
<ButtonIcon type="up" />
392+
</Button>
393+
<Button
394+
onClick={handleNextErrorOrWarningClick}
395+
title="Scroll to next error or warning">
396+
<ButtonIcon type="down" />
397+
</Button>
398+
<Button
399+
onClick={clearErrorsAndWarnings}
400+
title="Clear all errors and warnings">
401+
<ButtonIcon type="clear" />
402+
</Button>
403+
</React.Fragment>
404+
)}
407405
{!hideSettings && (
408406
<Fragment>
409407
<div className={styles.VRule} />

‎packages/react-devtools-shared/src/devtools/views/Settings/DebuggingSettings.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ export default function DebuggingSettings({
3737
const [showInlineWarningsAndErrors, setShowInlineWarningsAndErrors] =
3838
useState(usedHookSettings.showInlineWarningsAndErrors);
3939

40+
useEffect(() => {
41+
store.setShouldShowWarningsAndErrors(showInlineWarningsAndErrors);
42+
}, [showInlineWarningsAndErrors]);
43+
4044
useEffect(() => {
4145
store.updateHookSettings({
4246
appendComponentStack,

‎packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -43,21 +43,9 @@ type Context = {
4343
// Specified as a separate prop so it can trigger a re-render of FixedSizeList.
4444
lineHeight: number,
4545

46-
appendComponentStack: boolean,
47-
setAppendComponentStack: (value: boolean) => void,
48-
49-
breakOnConsoleErrors: boolean,
50-
setBreakOnConsoleErrors: (value: boolean) => void,
51-
5246
parseHookNames: boolean,
5347
setParseHookNames: (value: boolean) => void,
5448

55-
hideConsoleLogsInStrictMode: boolean,
56-
setHideConsoleLogsInStrictMode: (value: boolean) => void,
57-
58-
showInlineWarningsAndErrors: boolean,
59-
setShowInlineWarningsAndErrors: (value: boolean) => void,
60-
6149
theme: Theme,
6250
setTheme(value: Theme): void,
6351

@@ -176,7 +164,7 @@ function SettingsContextController({
176164
bridge.send('setTraceUpdatesEnabled', traceUpdatesEnabled);
177165
}, [bridge, traceUpdatesEnabled]);
178166

179-
const value = useMemo(
167+
const value: Context = useMemo(
180168
() => ({
181169
displayDensity,
182170
lineHeight:

0 commit comments

Comments
 (0)