From 18b34b49268ab9256e5408b1746da4a45eec5ca1 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 26 Apr 2019 13:53:22 -0700 Subject: [PATCH 01/16] Rearranged Settings UI in prep for custom filter controls --- src/devtools/views/Settings/Settings.css | 47 +++++++++++++----- src/devtools/views/Settings/Settings.js | 63 ++++++++++++------------ 2 files changed, 67 insertions(+), 43 deletions(-) diff --git a/src/devtools/views/Settings/Settings.css b/src/devtools/views/Settings/Settings.css index 581facc0e9..9e255d1233 100644 --- a/src/devtools/views/Settings/Settings.css +++ b/src/devtools/views/Settings/Settings.css @@ -13,48 +13,73 @@ } .Section { - display: flex; - flex-direction: row; - align-items: center; - margin-right: 0.5rem; - margin-bottom: 0.5rem; + width: 100%; + padding: 0.5rem 0; + border-top: 1px solid var(--color-border); +} +.Section:first-of-type { + padding-top: 0; + border-top: none; } .Header { - margin-right: 0.5rem; + margin-bottom: 0.5rem; font-size: var(--font-size-sans-large); } .OptionGroup { - display: flex; + display: inline-flex; flex-direction: row; + align-items: center; user-select: none; + margin: 0 1rem 0.5rem 0; +} +.OptionGroup:last-of-type { + margin-right: 0; } -.Option { +.OptionLabel { + margin-right: 0.5rem; + font-size: var(--font-size-sans-normal); +} + +.RadioOption { cursor: pointer; padding: 0.5rem; border: 1px solid var(--color-border); border-right: none; } -.Option:hover { +.RadioOption:hover { background-color: var(--color-background-hover); } -.Option:first-of-type { +.RadioOption:first-of-type { border-top-left-radius: 0.25rem; border-bottom-left-radius: 0.25rem; } -.Option:last-of-type { +.RadioOption:last-of-type { border-top-right-radius: 0.25rem; border-bottom-right-radius: 0.25rem; border-right: 1px solid var(--color-border); } +.CheckboxOption { + display: block; + padding: 0.5rem 0; +} + .ScreenshotThrottling { background-color: var(--color-background-hover); padding: 0.25rem 0.5rem; border-radius: 0.25rem; } + +.HRule { + height: 1px; + background-color: var(--color-border); + width: 100%; + border: none; + margin: 0.5rem 0; +} diff --git a/src/devtools/views/Settings/Settings.js b/src/devtools/views/Settings/Settings.js index 1c1ed43bcf..b578fe5b38 100644 --- a/src/devtools/views/Settings/Settings.js +++ b/src/devtools/views/Settings/Settings.js @@ -73,9 +73,10 @@ function Settings(_: {||}) { return (
-
Theme
+
Display preferences
-
-
-
-
Components tree
- -
-
-
Display density
-
+ +
+
Components tree
+ +
+ {store.supportsCaptureScreenshots && ( -
-
-
Profiler
- -
+
+
Profiler
+ {captureScreenshots && (
Screenshots will be throttled in order to reduce the negative From 17c5feb66a625be8c6a3b223dedca29333de77db Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 26 Apr 2019 14:02:32 -0700 Subject: [PATCH 02/16] Reorganized types shared between backend and frontend slightly. --- src/backend/renderer.js | 19 +++++++++++++------ src/backend/types.js | 2 +- src/devtools/store.js | 5 ++--- src/devtools/types.js | 18 ------------------ src/devtools/views/Components/Element.js | 2 +- .../views/Components/SelectedElement.js | 2 +- src/devtools/views/Components/types.js | 2 +- .../views/Profiler/CommitTreeBuilder.js | 4 ++-- src/types.js | 18 ++++++++++++++++++ 9 files changed, 39 insertions(+), 33 deletions(-) delete mode 100644 src/devtools/types.js diff --git a/src/backend/renderer.js b/src/backend/renderer.js index 3b029be297..9c216a9033 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -8,12 +8,13 @@ import { ElementTypeEventComponent, ElementTypeEventTarget, ElementTypeForwardRef, + ElementTypeHost, ElementTypeMemo, ElementTypeOtherOrUnknown, ElementTypeProfiler, ElementTypeRoot, ElementTypeSuspense, -} from 'src/devtools/types'; +} from 'src/types'; import { getDisplayName, utfEncodeString } from '../utils'; import { cleanForBridge, copyWithSet, setInObject } from './utils'; import { @@ -259,19 +260,21 @@ export function attach( } }; - // Keep this function in sync with getDataForFiber() + // NOTICE Keep in sync with getDataForFiber() function shouldFilterFiber(fiber: Fiber): boolean { const { tag } = fiber; switch (tag) { case ClassComponent: case FunctionComponent: + case HostComponent: case IncompleteClassComponent: case IndeterminateComponent: case ForwardRef: case HostRoot: case MemoComponent: case SimpleMemoComponent: + // TODO (filtering) Check custom filters return false; case DehydratedSuspenseComponent: // TODO: ideally we would show dehydrated Suspense immediately. @@ -282,7 +285,6 @@ export function attach( return true; case EventComponent: case HostPortal: - case HostComponent: case HostText: case Fragment: return true; @@ -305,6 +307,7 @@ export function attach( case DEPRECATED_PLACEHOLDER_SYMBOL_STRING: case PROFILER_NUMBER: case PROFILER_SYMBOL_STRING: + // TODO (filtering) Check custom filters return false; default: return false; @@ -321,8 +324,7 @@ export function attach( : symbolOrNumber; } - // TODO: we might want to change the data structure once we no longer suppport Stack versions of `getData`. - // TODO: Keep in sync with getElementType() + // NOTICE Keep in sync with shouldFilterFiber() function getDataForFiber(fiber: Fiber): FiberData { const { elementType, type, key, tag } = fiber; @@ -397,8 +399,13 @@ export function attach( key: null, type: ElementTypeRoot, }; - case HostPortal: case HostComponent: + return { + displayName: type, + key, + type: ElementTypeHost, + }; + case HostPortal: case HostText: case Fragment: return { diff --git a/src/backend/types.js b/src/backend/types.js index d1946d164a..ae5f395ba2 100644 --- a/src/backend/types.js +++ b/src/backend/types.js @@ -1,6 +1,6 @@ // @flow -import type { ElementType } from 'src/devtools/types'; +import type { ElementType } from 'src/types'; import type { InspectedElement } from 'src/devtools/views/Components/types'; type BundleType = diff --git a/src/devtools/store.js b/src/devtools/store.js index ecdf1683df..242bb15757 100644 --- a/src/devtools/store.js +++ b/src/devtools/store.js @@ -9,19 +9,18 @@ import { TREE_OPERATION_REORDER_CHILDREN, TREE_OPERATION_UPDATE_TREE_BASE_DURATION, } from '../constants'; -import { ElementTypeRoot } from './types'; +import { ElementTypeRoot } from '../types'; import { utfDecodeString } from '../utils'; import { __DEBUG__ } from '../constants'; import ProfilingCache from './ProfilingCache'; import { printStore } from 'src/__tests__/storeSerializer'; -import type { ElementType } from './types'; import type { Element } from './views/Components/types'; import type { ImportedProfilingData, ProfilingSnapshotNode, } from './views/Profiler/types'; -import type { Bridge } from '../types'; +import type { ElementType, Bridge } from '../types'; const debug = (methodName, ...args) => { if (__DEBUG__) { diff --git a/src/devtools/types.js b/src/devtools/types.js deleted file mode 100644 index 40425b7275..0000000000 --- a/src/devtools/types.js +++ /dev/null @@ -1,18 +0,0 @@ -// @flow - -export const ElementTypeClass = 1; -export const ElementTypeEventComponent = 2; -export const ElementTypeEventTarget = 3; -export const ElementTypeFunction = 4; -export const ElementTypeContext = 5; -export const ElementTypeForwardRef = 6; -export const ElementTypeMemo = 7; -export const ElementTypeOtherOrUnknown = 8; -export const ElementTypeProfiler = 9; -export const ElementTypeRoot = 10; -export const ElementTypeSuspense = 11; - -// Different types of elements displayed in the Elements tree. -// These types may be used to visually distinguish types, -// or to enable/disable certain functionality. -export type ElementType = 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11; diff --git a/src/devtools/views/Components/Element.js b/src/devtools/views/Components/Element.js index bfe0ce65ce..322726784d 100644 --- a/src/devtools/views/Components/Element.js +++ b/src/devtools/views/Components/Element.js @@ -9,7 +9,7 @@ import React, { useRef, useState, } from 'react'; -import { ElementTypeClass, ElementTypeFunction } from 'src/devtools/types'; +import { ElementTypeClass, ElementTypeFunction } from 'src/types'; import Store from 'src/devtools/store'; import ButtonIcon from '../ButtonIcon'; import { createRegExp } from '../utils'; diff --git a/src/devtools/views/Components/SelectedElement.js b/src/devtools/views/Components/SelectedElement.js index c2b62a6389..8740480a98 100644 --- a/src/devtools/views/Components/SelectedElement.js +++ b/src/devtools/views/Components/SelectedElement.js @@ -16,7 +16,7 @@ import { ElementTypeFunction, ElementTypeMemo, ElementTypeSuspense, -} from '../../types'; +} from 'src/types'; import type { Element, InspectedElement } from './types'; diff --git a/src/devtools/views/Components/types.js b/src/devtools/views/Components/types.js index ca3c7e3df0..0849ec019a 100644 --- a/src/devtools/views/Components/types.js +++ b/src/devtools/views/Components/types.js @@ -1,6 +1,6 @@ // @flow -import type { ElementType } from '../../types'; +import type { ElementType } from 'src/types'; // Each element on the frontend corresponds to a Fiber on the backend. // Some of its information (e.g. id, type, displayName) come from the backend. diff --git a/src/devtools/views/Profiler/CommitTreeBuilder.js b/src/devtools/views/Profiler/CommitTreeBuilder.js index fdf98533c2..b9bf40650a 100644 --- a/src/devtools/views/Profiler/CommitTreeBuilder.js +++ b/src/devtools/views/Profiler/CommitTreeBuilder.js @@ -8,10 +8,10 @@ import { TREE_OPERATION_UPDATE_TREE_BASE_DURATION, } from 'src/constants'; import { utfDecodeString } from 'src/utils'; -import { ElementTypeRoot } from 'src/devtools/types'; +import { ElementTypeRoot } from 'src/types'; import Store from 'src/devtools/store'; -import type { ElementType } from 'src/devtools/types'; +import type { ElementType } from 'src/types'; import type { CommitTreeFrontend, CommitTreeNodeFrontend, diff --git a/src/types.js b/src/types.js index 9e5a9de04e..5abee01c6c 100644 --- a/src/types.js +++ b/src/types.js @@ -11,3 +11,21 @@ export type Wall = {| listen: (fn: Function) => Function, send: (event: string, payload: any, transferable?: Array) => void, |}; + +export const ElementTypeClass = 1; +export const ElementTypeContext = 2; +export const ElementTypeEventComponent = 3; +export const ElementTypeEventTarget = 4; +export const ElementTypeFunction = 5; +export const ElementTypeForwardRef = 6; +export const ElementTypeHost = 7; +export const ElementTypeMemo = 8; +export const ElementTypeOtherOrUnknown = 9; +export const ElementTypeProfiler = 10; +export const ElementTypeRoot = 11; +export const ElementTypeSuspense = 12; + +// Different types of elements displayed in the Elements tree. +// These types may be used to visually distinguish types, +// or to enable/disable certain functionality. +export type ElementType = 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12; From 9db209ee64b6eb12cbfb9cae8c74027fb2317d1f Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 26 Apr 2019 14:40:02 -0700 Subject: [PATCH 03/16] Initial pass at adding filter-by-type Flow types and support to renderer --- OVERVIEW.md | 2 +- src/backend/renderer.js | 73 ++++++++++++++++++++++++++++++++++------- src/constants.js | 2 ++ src/types.js | 16 ++++++++- src/utils.js | 15 ++++++++- 5 files changed, 93 insertions(+), 15 deletions(-) diff --git a/OVERVIEW.md b/OVERVIEW.md index 05069783d8..a2d8a67fa9 100644 --- a/OVERVIEW.md +++ b/OVERVIEW.md @@ -83,7 +83,7 @@ For example, adding a function component `` with an id 2: [ 1, // add operation 2, // fiber id - 2, // ElementTypeFunction + 1, // ElementTypeClass 1, // parent id 0, // owner id 3, // encoded display name size diff --git a/src/backend/renderer.js b/src/backend/renderer.js index 9c216a9033..718b148d21 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -3,19 +3,27 @@ import { gte } from 'semver'; import { ElementTypeClass, - ElementTypeFunction, ElementTypeContext, ElementTypeEventComponent, ElementTypeEventTarget, + ElementTypeFunction, ElementTypeForwardRef, - ElementTypeHost, + ElementTypeHostComponent, ElementTypeMemo, ElementTypeOtherOrUnknown, ElementTypeProfiler, ElementTypeRoot, ElementTypeSuspense, + FilterByElementType, + FilterByName, + FilterByPath, } from 'src/types'; -import { getDisplayName, utfEncodeString } from '../utils'; +import { + getDisplayName, + getSavedFilters, + getUID, + utfEncodeString, +} from 'src/utils'; import { cleanForBridge, copyWithSet, setInObject } from './utils'; import { __DEBUG__, @@ -25,7 +33,6 @@ import { TREE_OPERATION_REORDER_CHILDREN, TREE_OPERATION_UPDATE_TREE_BASE_DURATION, } from '../constants'; -import { getUID } from '../utils'; import { inspectHooksOfFiber } from './ReactDebugHooks'; import type { @@ -43,6 +50,7 @@ import type { ReactRenderer, RendererInterface, } from './types'; +import type { ElementType, Filter } from 'src/types'; import type { InspectedElement } from 'src/devtools/views/Components/types'; function getInternalReactConstants(version) { @@ -260,22 +268,62 @@ export function attach( } }; + const filterByElementTypeMap: Map = new Map(); + const filterByNames: Set = new Set(); + const filterByPaths: Set = new Set(); + + function updateFilters(filters: Array): void { + filterByElementTypeMap.clear(); + filterByNames.clear(); + filterByPaths.clear(); + + filters.forEach(({ type, value }) => { + switch (type) { + case FilterByElementType: + filterByElementTypeMap.set(((value: any): ElementType), true); + break; + case FilterByName: + filterByNames.add(((value: any): RegExp)); + break; + case FilterByPath: + filterByPaths.add(((value: any): RegExp)); + break; + default: + console.error(`Unsupported filter type "${type}"`); + break; + } + }); + } + + // Initialize to the persisted values + updateFilters(getSavedFilters()); + // NOTICE Keep in sync with getDataForFiber() function shouldFilterFiber(fiber: Fiber): boolean { const { tag } = fiber; + // TODO (filter) This does not yet support display name or path based filtering. + switch (tag) { case ClassComponent: - case FunctionComponent: - case HostComponent: case IncompleteClassComponent: + return filterByElementTypeMap.get(ElementTypeClass) === true; + case FunctionComponent: + return filterByElementTypeMap.get(ElementTypeFunction) === true; case IndeterminateComponent: + return ( + filterByElementTypeMap.get(ElementTypeClass) === true || + filterByElementTypeMap.get(ElementTypeFunction) === true + ); case ForwardRef: - case HostRoot: + return filterByElementTypeMap.get(ElementTypeForwardRef) === true; case MemoComponent: case SimpleMemoComponent: - // TODO (filtering) Check custom filters - return false; + return filterByElementTypeMap.get(ElementTypeMemo) === true; + case HostComponent: + return filterByElementTypeMap.get(ElementTypeHostComponent) === true; + case HostRoot: + return false; // We never support filtering roots case DehydratedSuspenseComponent: // TODO: ideally we would show dehydrated Suspense immediately. // However, it has some special behavior (like disconnecting @@ -302,13 +350,14 @@ export function attach( case CONTEXT_PROVIDER_SYMBOL_STRING: case CONTEXT_CONSUMER_NUMBER: case CONTEXT_CONSUMER_SYMBOL_STRING: + return filterByElementTypeMap.get(ElementTypeContext) === true; case SUSPENSE_NUMBER: case SUSPENSE_SYMBOL_STRING: case DEPRECATED_PLACEHOLDER_SYMBOL_STRING: + return filterByElementTypeMap.get(ElementTypeSuspense) === true; case PROFILER_NUMBER: case PROFILER_SYMBOL_STRING: - // TODO (filtering) Check custom filters - return false; + return filterByElementTypeMap.get(ElementTypeProfiler) === true; default: return false; } @@ -403,7 +452,7 @@ export function attach( return { displayName: type, key, - type: ElementTypeHost, + type: ElementTypeHostComponent, }; case HostPortal: case HostText: diff --git a/src/constants.js b/src/constants.js index 588c8d4219..ea4e7642db 100644 --- a/src/constants.js +++ b/src/constants.js @@ -5,6 +5,8 @@ export const TREE_OPERATION_REMOVE = 2; export const TREE_OPERATION_REORDER_CHILDREN = 3; export const TREE_OPERATION_UPDATE_TREE_BASE_DURATION = 4; +export const LOCAL_STORAGE_FILTERS_KEY = 'React::DevTools::filters'; + export const LOCAL_STORAGE_RELOAD_AND_PROFILE_KEY = 'React::DevTools::reloadAndProfile'; diff --git a/src/types.js b/src/types.js index 5abee01c6c..d831fcef37 100644 --- a/src/types.js +++ b/src/types.js @@ -18,7 +18,7 @@ export const ElementTypeEventComponent = 3; export const ElementTypeEventTarget = 4; export const ElementTypeFunction = 5; export const ElementTypeForwardRef = 6; -export const ElementTypeHost = 7; +export const ElementTypeHostComponent = 7; export const ElementTypeMemo = 8; export const ElementTypeOtherOrUnknown = 9; export const ElementTypeProfiler = 10; @@ -29,3 +29,17 @@ export const ElementTypeSuspense = 12; // These types may be used to visually distinguish types, // or to enable/disable certain functionality. export type ElementType = 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12; + +export const FilterByElementType = 1; +export const FilterByName = 2; +export const FilterByPath = 3; + +export type Filter = + | {| + type: 1, + value: ElementType, + |} + | {| + type: 2 | 3, + value: RegExp, + |}; diff --git a/src/utils.js b/src/utils.js index f085f550d5..9d07194fc7 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1,6 +1,10 @@ // @flow -const LRU = require('lru-cache'); +import LRU from 'lru-cache'; +import { LOCAL_STORAGE_FILTERS_KEY } from './constants'; +import { ElementTypeHostComponent } from './types'; + +import type { Filter } from './types'; const FB_MODULE_RE = /^(.*) \[from (.*)\]$/; const cachedDisplayNames: WeakMap = new WeakMap(); @@ -76,3 +80,12 @@ export function utfEncodeString(string: string): Uint32Array { function toCodePoint(string: string) { return string.codePointAt(0); } + +export function getSavedFilters(): Array { + const filters = localStorage.getItem(LOCAL_STORAGE_FILTERS_KEY); + if (filters != null) { + return ((JSON.parse(filters): any): Array); + } else { + return [{ type: 1, value: ElementTypeHostComponent }]; + } +} From ee1b38e4dbfdf674ecdb17d3a07a1375ed01979a Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sat, 27 Apr 2019 10:31:40 -0700 Subject: [PATCH 04/16] Added filter preference types and plugged into renderer partly. Lots of work to do still. --- src/backend/renderer.js | 60 ++++++---------------- src/constants.js | 3 +- src/devtools/views/ButtonIcon.js | 20 +++++++- src/devtools/views/Settings/FilterList.css | 4 ++ src/devtools/views/Settings/FilterList.js | 40 +++++++++++++++ src/devtools/views/Settings/Settings.css | 1 + src/devtools/views/Settings/Settings.js | 4 ++ src/devtools/views/hooks.js | 10 +++- src/types.js | 27 +++++----- src/utils.js | 46 ++++++++++++++--- 10 files changed, 147 insertions(+), 68 deletions(-) create mode 100644 src/devtools/views/Settings/FilterList.css create mode 100644 src/devtools/views/Settings/FilterList.js diff --git a/src/backend/renderer.js b/src/backend/renderer.js index 718b148d21..f1ee1bc7da 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -14,13 +14,10 @@ import { ElementTypeProfiler, ElementTypeRoot, ElementTypeSuspense, - FilterByElementType, - FilterByName, - FilterByPath, } from 'src/types'; import { getDisplayName, - getSavedFilters, + getSavedFilterPreferences, getUID, utfEncodeString, } from 'src/utils'; @@ -50,7 +47,6 @@ import type { ReactRenderer, RendererInterface, } from './types'; -import type { ElementType, Filter } from 'src/types'; import type { InspectedElement } from 'src/devtools/views/Components/types'; function getInternalReactConstants(version) { @@ -268,35 +264,11 @@ export function attach( } }; - const filterByElementTypeMap: Map = new Map(); - const filterByNames: Set = new Set(); - const filterByPaths: Set = new Set(); - - function updateFilters(filters: Array): void { - filterByElementTypeMap.clear(); - filterByNames.clear(); - filterByPaths.clear(); - - filters.forEach(({ type, value }) => { - switch (type) { - case FilterByElementType: - filterByElementTypeMap.set(((value: any): ElementType), true); - break; - case FilterByName: - filterByNames.add(((value: any): RegExp)); - break; - case FilterByPath: - filterByPaths.add(((value: any): RegExp)); - break; - default: - console.error(`Unsupported filter type "${type}"`); - break; - } - }); - } - - // Initialize to the persisted values - updateFilters(getSavedFilters()); + const { + hideElementsWithTypes, + // TOOD (filter) hideElementsWithDisplayNames, + // TOOD (filter) hideElementsWithPaths, + } = getSavedFilterPreferences(); // NOTICE Keep in sync with getDataForFiber() function shouldFilterFiber(fiber: Fiber): boolean { @@ -307,21 +279,21 @@ export function attach( switch (tag) { case ClassComponent: case IncompleteClassComponent: - return filterByElementTypeMap.get(ElementTypeClass) === true; + return hideElementsWithTypes.has(ElementTypeClass); case FunctionComponent: - return filterByElementTypeMap.get(ElementTypeFunction) === true; + return hideElementsWithTypes.has(ElementTypeFunction); case IndeterminateComponent: return ( - filterByElementTypeMap.get(ElementTypeClass) === true || - filterByElementTypeMap.get(ElementTypeFunction) === true + hideElementsWithTypes.has(ElementTypeClass) || + hideElementsWithTypes.has(ElementTypeFunction) ); case ForwardRef: - return filterByElementTypeMap.get(ElementTypeForwardRef) === true; + return hideElementsWithTypes.has(ElementTypeForwardRef); case MemoComponent: case SimpleMemoComponent: - return filterByElementTypeMap.get(ElementTypeMemo) === true; + return hideElementsWithTypes.has(ElementTypeMemo); case HostComponent: - return filterByElementTypeMap.get(ElementTypeHostComponent) === true; + return hideElementsWithTypes.has(ElementTypeHostComponent); case HostRoot: return false; // We never support filtering roots case DehydratedSuspenseComponent: @@ -350,14 +322,14 @@ export function attach( case CONTEXT_PROVIDER_SYMBOL_STRING: case CONTEXT_CONSUMER_NUMBER: case CONTEXT_CONSUMER_SYMBOL_STRING: - return filterByElementTypeMap.get(ElementTypeContext) === true; + return hideElementsWithTypes.has(ElementTypeContext); case SUSPENSE_NUMBER: case SUSPENSE_SYMBOL_STRING: case DEPRECATED_PLACEHOLDER_SYMBOL_STRING: - return filterByElementTypeMap.get(ElementTypeSuspense) === true; + return hideElementsWithTypes.has(ElementTypeSuspense); case PROFILER_NUMBER: case PROFILER_SYMBOL_STRING: - return filterByElementTypeMap.get(ElementTypeProfiler) === true; + return hideElementsWithTypes.has(ElementTypeProfiler); default: return false; } diff --git a/src/constants.js b/src/constants.js index ea4e7642db..f5c7683d69 100644 --- a/src/constants.js +++ b/src/constants.js @@ -5,7 +5,8 @@ export const TREE_OPERATION_REMOVE = 2; export const TREE_OPERATION_REORDER_CHILDREN = 3; export const TREE_OPERATION_UPDATE_TREE_BASE_DURATION = 4; -export const LOCAL_STORAGE_FILTERS_KEY = 'React::DevTools::filters'; +export const LOCAL_STORAGE_FILTER_PREFERENCES_KEY = + 'React::DevTools::filterPreferences'; export const LOCAL_STORAGE_RELOAD_AND_PROFILE_KEY = 'React::DevTools::reloadAndProfile'; diff --git a/src/devtools/views/ButtonIcon.js b/src/devtools/views/ButtonIcon.js index a42e02bdbc..3099068b5b 100644 --- a/src/devtools/views/ButtonIcon.js +++ b/src/devtools/views/ButtonIcon.js @@ -4,10 +4,12 @@ import React from 'react'; import styles from './ButtonIcon.css'; export type IconType = + | 'add' | 'cancel' | 'close' | 'collapsed' | 'copy' + | 'delete' | 'down' | 'expanded' | 'export' @@ -26,12 +28,16 @@ export type IconType = | 'view-source'; type Props = {| + className?: string, type: IconType, |}; -export default function ButtonIcon({ type }: Props) { +export default function ButtonIcon({ className = '', type }: Props) { let pathData = null; switch (type) { + case 'add': + pathData = PATH_ADD; + break; case 'cancel': pathData = PATH_CANCEL; break; @@ -44,6 +50,9 @@ export default function ButtonIcon({ type }: Props) { case 'copy': pathData = PATH_COPY; break; + case 'delete': + pathData = PATH_DELETE; + break; case 'down': pathData = PATH_DOWN; break; @@ -100,7 +109,7 @@ export default function ButtonIcon({ type }: Props) { return ( { + const clonedFilterPreferences = { ...filterPreferences }; + setFilterPreferences(clonedFilterPreferences); + saveFilterPreferences(clonedFilterPreferences); + }, [filterPreferences]); + + const { hideElementsWithTypes } = filterPreferences; + + return ( + + <div>) + + + ); +} diff --git a/src/devtools/views/Settings/Settings.css b/src/devtools/views/Settings/Settings.css index 9e255d1233..a514192750 100644 --- a/src/devtools/views/Settings/Settings.css +++ b/src/devtools/views/Settings/Settings.css @@ -71,6 +71,7 @@ } .ScreenshotThrottling { + display: inline-block; background-color: var(--color-background-hover); padding: 0.25rem 0.5rem; border-radius: 0.25rem; diff --git a/src/devtools/views/Settings/Settings.js b/src/devtools/views/Settings/Settings.js index b578fe5b38..ae23bbbc7d 100644 --- a/src/devtools/views/Settings/Settings.js +++ b/src/devtools/views/Settings/Settings.js @@ -5,6 +5,7 @@ import { useSubscription } from '../hooks'; import { StoreContext } from '../context'; import { SettingsContext } from './SettingsContext'; import Store from 'src/devtools/store'; +import FilterList from './FilterList'; import portaledContent from '../portaledContent'; import styles from './Settings.css'; @@ -134,6 +135,7 @@ function Settings(_: {||}) {
Components tree
+ + +
{store.supportsCaptureScreenshots && ( diff --git a/src/devtools/views/hooks.js b/src/devtools/views/hooks.js index 6d48915d20..b9e7dfc0b4 100644 --- a/src/devtools/views/hooks.js +++ b/src/devtools/views/hooks.js @@ -38,14 +38,20 @@ export function useIsOverflowing( // Forked from https://usehooks.com/useLocalStorage/ export function useLocalStorage( key: string, - initialValue: T + initialValue: T | (() => T) ): [T, (value: T | (() => T)) => void] { const getValueFromLocalStorage = useCallback(() => { try { const item = window.localStorage.getItem(key); - return item ? JSON.parse(item) : initialValue; + if (item != null) { + return JSON.parse(item); + } } catch (error) { console.log(error); + } + if (typeof initialValue === 'function') { + return (initialValue: any)(); + } else { return initialValue; } }, [initialValue, key]); diff --git a/src/types.js b/src/types.js index d831fcef37..b1af981fde 100644 --- a/src/types.js +++ b/src/types.js @@ -12,6 +12,10 @@ export type Wall = {| send: (event: string, payload: any, transferable?: Array) => void, |}; +// WARNING +// The values below are referenced by FilterPreferences (which is saved via localStorage). +// Do not change them or it will break previously saved user customizations. +// If new element types are added, use new numbers rather than re-ordering existing ones. export const ElementTypeClass = 1; export const ElementTypeContext = 2; export const ElementTypeEventComponent = 3; @@ -30,16 +34,15 @@ export const ElementTypeSuspense = 12; // or to enable/disable certain functionality. export type ElementType = 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12; -export const FilterByElementType = 1; -export const FilterByName = 2; -export const FilterByPath = 3; +export type FilterPreferences = {| + // Hide all elements of types in this Set. + // We hide host components only by default. + hideElementsWithTypes: Set, -export type Filter = - | {| - type: 1, - value: ElementType, - |} - | {| - type: 2 | 3, - value: RegExp, - |}; + // Hide all elements with displayNames matching one or more of the RegExps in this Set. + hideElementsWithDisplayNames: Set, + + // Hide all elements within paths matching one or more of the RegExps in this Set. + // This filter is only used for elements that include debug source location. + hideElementsWithPaths: Set, +|}; diff --git a/src/utils.js b/src/utils.js index 9d07194fc7..b20abd07fd 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1,10 +1,10 @@ // @flow import LRU from 'lru-cache'; -import { LOCAL_STORAGE_FILTERS_KEY } from './constants'; +import { LOCAL_STORAGE_FILTER_PREFERENCES_KEY } from './constants'; import { ElementTypeHostComponent } from './types'; -import type { Filter } from './types'; +import type { FilterPreferences } from './types'; const FB_MODULE_RE = /^(.*) \[from (.*)\]$/; const cachedDisplayNames: WeakMap = new WeakMap(); @@ -81,11 +81,43 @@ function toCodePoint(string: string) { return string.codePointAt(0); } -export function getSavedFilters(): Array { - const filters = localStorage.getItem(LOCAL_STORAGE_FILTERS_KEY); - if (filters != null) { - return ((JSON.parse(filters): any): Array); +export function getDefaultFilterPreferences(): FilterPreferences { + return { + hideElementsWithTypes: new Set([ElementTypeHostComponent]), + hideElementsWithDisplayNames: new Set(), + hideElementsWithPaths: new Set(), + }; +} + +export function getSavedFilterPreferences(): FilterPreferences { + const raw = localStorage.getItem(LOCAL_STORAGE_FILTER_PREFERENCES_KEY); + if (raw != null) { + const json = JSON.parse(raw); + return { + hideElementsWithTypes: new Set(json.hideElementsWithTypes), + hideElementsWithDisplayNames: new Set(json.hideElementsWithDisplayNames), + hideElementsWithPaths: new Set(json.hideElementsWithPaths), + }; } else { - return [{ type: 1, value: ElementTypeHostComponent }]; + return getDefaultFilterPreferences(); } } + +export function saveFilterPreferences( + filterPreferences: FilterPreferences +): void { + localStorage.setItem( + LOCAL_STORAGE_FILTER_PREFERENCES_KEY, + JSON.stringify({ + hideElementsWithTypes: Array.from( + filterPreferences.hideElementsWithTypes + ), + hideElementsWithDisplayNames: Array.from( + filterPreferences.hideElementsWithDisplayNames + ), + hideElementsWithPaths: Array.from( + filterPreferences.hideElementsWithPaths + ), + }) + ); +} From 27a182003913e80771cbe0d4f44a026bdc7a15f3 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sun, 28 Apr 2019 15:33:39 -0700 Subject: [PATCH 05/16] Refactored filter preferences a bit more to be stored in the Store --- src/devtools/store.js | 26 ++++++++- src/devtools/views/Settings/FilterList.css | 2 +- src/devtools/views/Settings/FilterList.js | 63 ++++++++++++++-------- src/devtools/views/Settings/Settings.css | 2 +- 4 files changed, 68 insertions(+), 25 deletions(-) diff --git a/src/devtools/store.js b/src/devtools/store.js index 242bb15757..4752818f67 100644 --- a/src/devtools/store.js +++ b/src/devtools/store.js @@ -10,7 +10,11 @@ import { TREE_OPERATION_UPDATE_TREE_BASE_DURATION, } from '../constants'; import { ElementTypeRoot } from '../types'; -import { utfDecodeString } from '../utils'; +import { + getSavedFilterPreferences, + saveFilterPreferences, + utfDecodeString, +} from '../utils'; import { __DEBUG__ } from '../constants'; import ProfilingCache from './ProfilingCache'; import { printStore } from 'src/__tests__/storeSerializer'; @@ -20,7 +24,7 @@ import type { ImportedProfilingData, ProfilingSnapshotNode, } from './views/Profiler/types'; -import type { ElementType, Bridge } from '../types'; +import type { Bridge, ElementType, FilterPreferences } from '../types'; const debug = (methodName, ...args) => { if (__DEBUG__) { @@ -65,6 +69,8 @@ export default class Store extends EventEmitter { // Should new nodes be collapsed by default when added to the tree? _collapseNodesByDefault: boolean = true; + _filterPreferences: FilterPreferences; + // At least one of the injected renderers contains (DEV only) owner metadata. _hasOwnerMetadata: boolean = false; @@ -137,6 +143,8 @@ export default class Store extends EventEmitter { localStorage.getItem(LOCAL_STORAGE_COLLAPSE_ROOTS_BY_DEFAULT_KEY) !== 'false'; + this._filterPreferences = getSavedFilterPreferences(); + if (config != null) { const { isProfiling, @@ -231,6 +239,20 @@ export default class Store extends EventEmitter { this.emit('collapseNodesByDefault'); } + get filterPreferences(): FilterPreferences { + return this._filterPreferences; + } + set filterPreferences(value: FilterPreferences): void { + this._filterPreferences = value; + + saveFilterPreferences(value); + + // TODO (filter) Dump all nodes, update renderer preferences, and re-initialize tree. + // TODO (filter) Invariant check that we aren't profiling. + + this.emit('filterPreferences'); + } + get hasOwnerMetadata(): boolean { return this._hasOwnerMetadata; } diff --git a/src/devtools/views/Settings/FilterList.css b/src/devtools/views/Settings/FilterList.css index ed6024ed69..52b592e5e9 100644 --- a/src/devtools/views/Settings/FilterList.css +++ b/src/devtools/views/Settings/FilterList.css @@ -1,4 +1,4 @@ .Filter { display: block; - padding: 0.5rem 0; + padding: 0 0 0.5rem; } diff --git a/src/devtools/views/Settings/FilterList.js b/src/devtools/views/Settings/FilterList.js index cb6201661d..af9e8c36f2 100644 --- a/src/devtools/views/Settings/FilterList.js +++ b/src/devtools/views/Settings/FilterList.js @@ -1,37 +1,58 @@ // @flow -import React, { Fragment, useCallback, useState } from 'react'; -import { getSavedFilterPreferences, saveFilterPreferences } from 'src/utils'; +import React, { Fragment, useCallback, useContext, useMemo } from 'react'; import { ElementTypeHostComponent } from 'src/types'; +import Store from 'src/devtools/store'; +import { StoreContext } from '../context'; +import { useSubscription } from '../hooks'; import styles from './FilterList.css'; -export default function FilterList(_: {||}) { - const [filterPreferences, setFilterPreferences] = useState( - getSavedFilterPreferences - ); - const updateFilterPreferences = useCallback(() => { - const clonedFilterPreferences = { ...filterPreferences }; - setFilterPreferences(clonedFilterPreferences); - saveFilterPreferences(clonedFilterPreferences); - }, [filterPreferences]); +import type { FilterPreferences } from 'src/types'; - const { hideElementsWithTypes } = filterPreferences; +export default function FilterList(_: {||}) { + const store = useContext(StoreContext); + + const filterPreferencesSubscription = useMemo( + () => ({ + getCurrentValue: () => store.filterPreferences, + subscribe: (callback: Function) => { + store.addListener('filterPreferences', callback); + return () => store.removeListener('filterPreferences', callback); + }, + }), + [store] + ); + const filterPreferences = useSubscription( + filterPreferencesSubscription + ); + + const updateFilterPreferences = useCallback( + ({ currentTarget }) => { + const filterPreferences = store.filterPreferences; + if (currentTarget.checked) { + filterPreferences.hideElementsWithTypes.add(ElementTypeHostComponent); + } else { + filterPreferences.hideElementsWithTypes.delete( + ElementTypeHostComponent + ); + } + store.filterPreferences = { ...filterPreferences }; + }, + [store] + ); + + // TODO (filter) Disable toggles if isProfiling return ( diff --git a/src/devtools/views/Settings/Settings.css b/src/devtools/views/Settings/Settings.css index a514192750..8f3d567346 100644 --- a/src/devtools/views/Settings/Settings.css +++ b/src/devtools/views/Settings/Settings.css @@ -67,7 +67,7 @@ .CheckboxOption { display: block; - padding: 0.5rem 0; + padding: 0 0 0.5rem; } .ScreenshotThrottling { From a241780dc249bc959c6b7e663dbe2e6deec1ab42 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 29 Apr 2019 13:08:35 -0700 Subject: [PATCH 06/16] Filter changes are applied to the renderer without reloading --- src/backend/agent.js | 12 +++++++++++- src/backend/renderer.js | 32 +++++++++++++++++++++++++++++--- src/backend/types.js | 3 ++- src/devtools/store.js | 4 +++- 4 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/backend/agent.js b/src/backend/agent.js index 5364d048c5..fed7344dbe 100644 --- a/src/backend/agent.js +++ b/src/backend/agent.js @@ -16,7 +16,7 @@ import type { RendererID, RendererInterface, } from './types'; -import type { Bridge } from '../types'; +import type { Bridge, FilterPreferences } from '../types'; const debug = (methodName, ...args) => { if (__DEBUG__) { @@ -118,6 +118,7 @@ export default class Agent extends EventEmitter { this.syncSelectionFromNativeElementsPanel ); bridge.addListener('shutdown', this.shutdown); + bridge.addListener('updateFilterPreferences', this.updateFilterPreferences); bridge.addListener('viewElementSource', this.viewElementSource); if (this._isProfiling) { @@ -489,6 +490,15 @@ export default class Agent extends EventEmitter { this._bridge.send('profilingStatus', this._isProfiling); }; + updateFilterPreferences = (filterPreferences: FilterPreferences) => { + for (let rendererID in this._rendererInterfaces) { + const renderer = ((this._rendererInterfaces[ + (rendererID: any) + ]: any): RendererInterface); + renderer.updateFilterPreferences(filterPreferences); + } + }; + viewElementSource = ({ id, rendererID }: InspectSelectParams) => { const renderer = this._rendererInterfaces[rendererID]; if (renderer == null) { diff --git a/src/backend/renderer.js b/src/backend/renderer.js index f1ee1bc7da..6a7326e3ba 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -48,6 +48,7 @@ import type { RendererInterface, } from './types'; import type { InspectedElement } from 'src/devtools/views/Components/types'; +import type { FilterPreferences } from 'src/types'; function getInternalReactConstants(version) { const ReactSymbols = { @@ -264,12 +265,36 @@ export function attach( } }; - const { + let { hideElementsWithTypes, - // TOOD (filter) hideElementsWithDisplayNames, - // TOOD (filter) hideElementsWithPaths, + hideElementsWithDisplayNames, + hideElementsWithPaths, } = getSavedFilterPreferences(); + // TODO (filter) We could make this more efficient. + function updateFilterPreferences(filterPreferences: FilterPreferences) { + // Recursively unmount and then re-mount all roots. + hook.getFiberRoots(rendererID).forEach(root => { + currentRootID = getFiberID(getPrimaryFiber(root.current)); + unmountFiberChildrenRecursively(root.current); + recordUnmount(root.current, false); + currentRootID = -1; + }); + + hideElementsWithTypes = filterPreferences.hideElementsWithTypes; + hideElementsWithDisplayNames = + filterPreferences.hideElementsWithDisplayNames; + hideElementsWithPaths = filterPreferences.hideElementsWithPaths; + + // Recursively re-mount all roots with new filter criteria applied. + hook.getFiberRoots(rendererID).forEach(root => { + currentRootID = getFiberID(getPrimaryFiber(root.current)); + mountFiberRecursively(root.current, null); + flushPendingEvents(root); + currentRootID = -1; + }); + } + // NOTICE Keep in sync with getDataForFiber() function shouldFilterFiber(fiber: Fiber): boolean { const { tag } = fiber; @@ -2288,5 +2313,6 @@ export function attach( setTrackedPath, startProfiling, stopProfiling, + updateFilterPreferences, }; } diff --git a/src/backend/types.js b/src/backend/types.js index ae5f395ba2..a12dda6292 100644 --- a/src/backend/types.js +++ b/src/backend/types.js @@ -1,6 +1,6 @@ // @flow -import type { ElementType } from 'src/types'; +import type { ElementType, FilterPreferences } from 'src/types'; import type { InspectedElement } from 'src/devtools/views/Components/types'; type BundleType = @@ -139,6 +139,7 @@ export type RendererInterface = { setTrackedPath: (path: Array | null) => void, startProfiling: () => void, stopProfiling: () => void, + updateFilterPreferences: (filterPreferences: FilterPreferences) => void, }; export type Handler = (data: any) => void; diff --git a/src/devtools/store.js b/src/devtools/store.js index 4752818f67..3e6a409d65 100644 --- a/src/devtools/store.js +++ b/src/devtools/store.js @@ -247,8 +247,10 @@ export default class Store extends EventEmitter { saveFilterPreferences(value); - // TODO (filter) Dump all nodes, update renderer preferences, and re-initialize tree. + // TODO (filter) Dump all nodes, update renderer preferences, and re-initialize tree. // TODO (filter) Invariant check that we aren't profiling. + // TODO (filter) Flushing every time a filter setting is changed is too expensive. We probably need an explitit configm + this._bridge.send('updateFilterPreferences', value); this.emit('filterPreferences'); } From eb9705abc87d6b9a03eaec55cdf8465e69e9ab14 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 30 Apr 2019 08:17:20 -0700 Subject: [PATCH 07/16] Removed FilterList UI for now (collapsed into Settings component) --- src/devtools/views/Settings/FilterList.css | 4 -- src/devtools/views/Settings/FilterList.js | 61 ---------------------- src/devtools/views/Settings/Settings.js | 44 +++++++++++++++- 3 files changed, 42 insertions(+), 67 deletions(-) delete mode 100644 src/devtools/views/Settings/FilterList.css delete mode 100644 src/devtools/views/Settings/FilterList.js diff --git a/src/devtools/views/Settings/FilterList.css b/src/devtools/views/Settings/FilterList.css deleted file mode 100644 index 52b592e5e9..0000000000 --- a/src/devtools/views/Settings/FilterList.css +++ /dev/null @@ -1,4 +0,0 @@ -.Filter { - display: block; - padding: 0 0 0.5rem; -} diff --git a/src/devtools/views/Settings/FilterList.js b/src/devtools/views/Settings/FilterList.js deleted file mode 100644 index af9e8c36f2..0000000000 --- a/src/devtools/views/Settings/FilterList.js +++ /dev/null @@ -1,61 +0,0 @@ -// @flow - -import React, { Fragment, useCallback, useContext, useMemo } from 'react'; -import { ElementTypeHostComponent } from 'src/types'; -import Store from 'src/devtools/store'; -import { StoreContext } from '../context'; -import { useSubscription } from '../hooks'; - -import styles from './FilterList.css'; - -import type { FilterPreferences } from 'src/types'; - -export default function FilterList(_: {||}) { - const store = useContext(StoreContext); - - const filterPreferencesSubscription = useMemo( - () => ({ - getCurrentValue: () => store.filterPreferences, - subscribe: (callback: Function) => { - store.addListener('filterPreferences', callback); - return () => store.removeListener('filterPreferences', callback); - }, - }), - [store] - ); - const filterPreferences = useSubscription( - filterPreferencesSubscription - ); - - const updateFilterPreferences = useCallback( - ({ currentTarget }) => { - const filterPreferences = store.filterPreferences; - if (currentTarget.checked) { - filterPreferences.hideElementsWithTypes.add(ElementTypeHostComponent); - } else { - filterPreferences.hideElementsWithTypes.delete( - ElementTypeHostComponent - ); - } - store.filterPreferences = { ...filterPreferences }; - }, - [store] - ); - - // TODO (filter) Disable toggles if isProfiling - - return ( - - - - ); -} diff --git a/src/devtools/views/Settings/Settings.js b/src/devtools/views/Settings/Settings.js index ae23bbbc7d..4ede5f1bf6 100644 --- a/src/devtools/views/Settings/Settings.js +++ b/src/devtools/views/Settings/Settings.js @@ -1,15 +1,17 @@ // @flow import React, { useCallback, useContext, useMemo } from 'react'; +import { ElementTypeHostComponent } from 'src/types'; import { useSubscription } from '../hooks'; import { StoreContext } from '../context'; import { SettingsContext } from './SettingsContext'; import Store from 'src/devtools/store'; -import FilterList from './FilterList'; import portaledContent from '../portaledContent'; import styles from './Settings.css'; +import type { FilterPreferences } from 'src/types'; + function Settings(_: {||}) { const store = useContext(StoreContext); const { displayDensity, setDisplayDensity, theme, setTheme } = useContext( @@ -44,6 +46,35 @@ function Settings(_: {||}) { collapseNodesByDefaultSubscription ); + const filterPreferencesSubscription = useMemo( + () => ({ + getCurrentValue: () => store.filterPreferences, + subscribe: (callback: Function) => { + store.addListener('filterPreferences', callback); + return () => store.removeListener('filterPreferences', callback); + }, + }), + [store] + ); + const filterPreferences = useSubscription( + filterPreferencesSubscription + ); + + const updateFilterPreferences = useCallback( + ({ currentTarget }) => { + const filterPreferences = store.filterPreferences; + if (currentTarget.checked) { + filterPreferences.hideElementsWithTypes.add(ElementTypeHostComponent); + } else { + filterPreferences.hideElementsWithTypes.delete( + ElementTypeHostComponent + ); + } + store.filterPreferences = { ...filterPreferences }; + }, + [store] + ); + const updateDisplayDensity = useCallback( ({ currentTarget }) => { setDisplayDensity(currentTarget.value); @@ -145,7 +176,16 @@ function Settings(_: {||}) { Collapse newly added components by default - +
{store.supportsCaptureScreenshots && ( From c4c51c0821d6ac37513da12d9174d0fe47a1390e Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 30 Apr 2019 09:48:15 -0700 Subject: [PATCH 08/16] Prevent filter preference changes while profiling is active --- src/backend/renderer.js | 7 +++++++ src/devtools/store.js | 7 ++++++- src/devtools/views/Settings/Settings.js | 15 +++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/backend/renderer.js b/src/backend/renderer.js index 6a7326e3ba..1370e45128 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -273,6 +273,12 @@ export function attach( // TODO (filter) We could make this more efficient. function updateFilterPreferences(filterPreferences: FilterPreferences) { + if (this._isProfiling) { + // Re-mounting a tree while profiling is in progress might break a lot of assumptions. + // If necessary, we could support this- but it doesn't seem like a necessary use case. + throw Error('Cannot modify filter preferences while profiling'); + } + // Recursively unmount and then re-mount all roots. hook.getFiberRoots(rendererID).forEach(root => { currentRootID = getFiberID(getPrimaryFiber(root.current)); @@ -2218,6 +2224,7 @@ export function attach( rootPseudoKeys.delete(id); } + // TODO (profiling) This breaks after filter preferences have been updated. function getPathFrame(fiber: Fiber): PathFrame { let { displayName, key } = getDataForFiber(fiber); const index = fiber.index; diff --git a/src/devtools/store.js b/src/devtools/store.js index 3e6a409d65..539dccf71c 100644 --- a/src/devtools/store.js +++ b/src/devtools/store.js @@ -243,12 +243,17 @@ export default class Store extends EventEmitter { return this._filterPreferences; } set filterPreferences(value: FilterPreferences): void { + if (this._isProfiling) { + // Re-mounting a tree while profiling is in progress might break a lot of assumptions. + // If necessary, we could support this- but it doesn't seem like a necessary use case. + throw Error('Cannot modify filter preferences while profiling'); + } + this._filterPreferences = value; saveFilterPreferences(value); // TODO (filter) Dump all nodes, update renderer preferences, and re-initialize tree. - // TODO (filter) Invariant check that we aren't profiling. // TODO (filter) Flushing every time a filter setting is changed is too expensive. We probably need an explitit configm this._bridge.send('updateFilterPreferences', value); diff --git a/src/devtools/views/Settings/Settings.js b/src/devtools/views/Settings/Settings.js index 4ede5f1bf6..cc4faeff51 100644 --- a/src/devtools/views/Settings/Settings.js +++ b/src/devtools/views/Settings/Settings.js @@ -46,6 +46,20 @@ function Settings(_: {||}) { collapseNodesByDefaultSubscription ); + // Re-mounting a tree while profiling is in progress might break a lot of assumptions. + // If necessary, we could support this- but it doesn't seem like a necessary use case. + const isProfilingSubscription = useMemo( + () => ({ + getCurrentValue: () => store.isProfiling, + subscribe: (callback: Function) => { + store.addListener('isProfiling', callback); + return () => store.removeListener('isProfiling', callback); + }, + }), + [store] + ); + const isProfiling = useSubscription(isProfilingSubscription); + const filterPreferencesSubscription = useMemo( () => ({ getCurrentValue: () => store.filterPreferences, @@ -182,6 +196,7 @@ function Settings(_: {||}) { checked={filterPreferences.hideElementsWithTypes.has( ElementTypeHostComponent )} + disabled={isProfiling} onChange={updateFilterPreferences} />{' '} Hide host components (e.g. <div>) From a2ba90bf96b387fd5b3147fdecf5231b9f726c26 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 30 Apr 2019 10:04:15 -0700 Subject: [PATCH 09/16] Fixed a path selection bug when filter pereferences are updated --- src/backend/renderer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/renderer.js b/src/backend/renderer.js index 1370e45128..153018f7e3 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -295,6 +295,7 @@ export function attach( // Recursively re-mount all roots with new filter criteria applied. hook.getFiberRoots(rendererID).forEach(root => { currentRootID = getFiberID(getPrimaryFiber(root.current)); + setRootPseudoKey(currentRootID, root.current); mountFiberRecursively(root.current, null); flushPendingEvents(root); currentRootID = -1; @@ -2224,7 +2225,6 @@ export function attach( rootPseudoKeys.delete(id); } - // TODO (profiling) This breaks after filter preferences have been updated. function getPathFrame(fiber: Fiber): PathFrame { let { displayName, key } = getDataForFiber(fiber); const index = fiber.index; From 0b4bfbc98fdf3cf8c11a25ef7d8f8841cbc549d6 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 30 Apr 2019 12:52:43 -0700 Subject: [PATCH 10/16] Add support for hide-by-name and hide-by-path component filter regexps --- src/backend/renderer.js | 300 +++++++++++++++++----------------------- src/devtools/store.js | 5 +- src/utils.js | 50 +++++-- 3 files changed, 169 insertions(+), 186 deletions(-) diff --git a/src/backend/renderer.js b/src/backend/renderer.js index 153018f7e3..4808b694e4 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -37,7 +37,6 @@ import type { DevToolsHook, Fiber, FiberCommitsBackend, - FiberData, InteractionBackend, InteractionsBackend, InteractionWithCommitsBackend, @@ -48,7 +47,7 @@ import type { RendererInterface, } from './types'; import type { InspectedElement } from 'src/devtools/views/Components/types'; -import type { FilterPreferences } from 'src/types'; +import type { ElementType, FilterPreferences } from 'src/types'; function getInternalReactConstants(version) { const ReactSymbols = { @@ -245,18 +244,15 @@ export function attach( const debug = (name: string, fiber: Fiber, parentFiber: ?Fiber): void => { if (__DEBUG__) { - const fiberData = getDataForFiber(fiber); - const fiberDisplayName = (fiberData && fiberData.displayName) || 'null'; - const parentFiberData = - parentFiber == null ? null : getDataForFiber(parentFiber); - const parentFiberDisplayName = - (parentFiberData && parentFiberData.displayName) || 'null'; + const displayName = getDisplayNameForFiber(fiber) || 'null'; + const parentDisplayName = + (parentFiber !== null && getDisplayNameForFiber(parentFiber)) || 'null'; // NOTE: calling getFiberID or getPrimaryFiber is unsafe here // because it will put them in the map. For now, we'll omit them. // TODO: better debugging story for this. console.log( - `[renderer] %c${name} %c${fiberDisplayName} %c${ - parentFiber ? parentFiberDisplayName : '' + `[renderer] %c${name} %c${displayName} %c${ + parentFiber ? parentDisplayName : '' }`, 'color: red; font-weight: bold;', 'color: blue;', @@ -271,7 +267,10 @@ export function attach( hideElementsWithPaths, } = getSavedFilterPreferences(); - // TODO (filter) We could make this more efficient. + // TODO (filter) Should we make this operation more efficient? + // For example, we could add a new recursive unmount tree operation. + // The unmount operations are already significantly smaller than mount opreations though. + // This is something to keep in mind for later. function updateFilterPreferences(filterPreferences: FilterPreferences) { if (this._isProfiling) { // Re-mounting a tree while profiling is in progress might break a lot of assumptions. @@ -302,32 +301,11 @@ export function attach( }); } - // NOTICE Keep in sync with getDataForFiber() + // NOTICE Keep in sync with get*ForFiber methods function shouldFilterFiber(fiber: Fiber): boolean { - const { tag } = fiber; - - // TODO (filter) This does not yet support display name or path based filtering. + const { _debugSource, tag, type } = fiber; switch (tag) { - case ClassComponent: - case IncompleteClassComponent: - return hideElementsWithTypes.has(ElementTypeClass); - case FunctionComponent: - return hideElementsWithTypes.has(ElementTypeFunction); - case IndeterminateComponent: - return ( - hideElementsWithTypes.has(ElementTypeClass) || - hideElementsWithTypes.has(ElementTypeFunction) - ); - case ForwardRef: - return hideElementsWithTypes.has(ElementTypeForwardRef); - case MemoComponent: - case SimpleMemoComponent: - return hideElementsWithTypes.has(ElementTypeMemo); - case HostComponent: - return hideElementsWithTypes.has(ElementTypeHostComponent); - case HostRoot: - return false; // We never support filtering roots case DehydratedSuspenseComponent: // TODO: ideally we would show dehydrated Suspense immediately. // However, it has some special behavior (like disconnecting @@ -341,7 +319,7 @@ export function attach( case Fragment: return true; default: - const typeSymbol = getTypeSymbol(fiber.type); + const typeSymbol = getTypeSymbol(type); switch (typeSymbol) { case CONCURRENT_MODE_NUMBER: @@ -350,22 +328,35 @@ export function attach( case STRICT_MODE_NUMBER: case STRICT_MODE_SYMBOL_STRING: return true; - case CONTEXT_PROVIDER_NUMBER: - case CONTEXT_PROVIDER_SYMBOL_STRING: - case CONTEXT_CONSUMER_NUMBER: - case CONTEXT_CONSUMER_SYMBOL_STRING: - return hideElementsWithTypes.has(ElementTypeContext); - case SUSPENSE_NUMBER: - case SUSPENSE_SYMBOL_STRING: - case DEPRECATED_PLACEHOLDER_SYMBOL_STRING: - return hideElementsWithTypes.has(ElementTypeSuspense); - case PROFILER_NUMBER: - case PROFILER_SYMBOL_STRING: - return hideElementsWithTypes.has(ElementTypeProfiler); default: - return false; + break; } } + + const elementType = getTypeForFiber(fiber); + if (hideElementsWithTypes.has(elementType)) { + return true; + } + + if (hideElementsWithDisplayNames.size > 0) { + const displayName = getDisplayNameForFiber(fiber) || ''; + for (let displayNameRegExp of hideElementsWithDisplayNames) { + if (displayNameRegExp.test(displayName)) { + return true; + } + } + } + + if (_debugSource !== null && hideElementsWithPaths.size > 0) { + const { fileName } = _debugSource; + for (let pathRegExp of hideElementsWithPaths) { + if (pathRegExp.test(fileName)) { + return true; + } + } + } + + return false; } function getTypeSymbol(type: any): Symbol | number { @@ -377,9 +368,9 @@ export function attach( : symbolOrNumber; } - // NOTICE Keep in sync with shouldFilterFiber() - function getDataForFiber(fiber: Fiber): FiberData { - const { elementType, type, key, tag } = fiber; + // NOTICE Keep in sync with shouldFilterFiber() and other get*ForFiber methods + function getDisplayNameForFiber(fiber: Fiber): string | null { + const { elementType, type, tag } = fiber; // This is to support lazy components with a Promise as the type. // see https://github.com/facebook/react/pull/13397 @@ -390,96 +381,47 @@ export function attach( } } - let fiberData: FiberData = ((null: any): FiberData); - let displayName: string = ((null: any): string); let resolvedContext: any = null; switch (tag) { case ClassComponent: case IncompleteClassComponent: - fiberData = { - displayName: getDisplayName(resolvedType), - key, - type: ElementTypeClass, - }; - break; + return getDisplayName(resolvedType); case FunctionComponent: case IndeterminateComponent: - fiberData = { - displayName: getDisplayName(resolvedType), - key, - type: ElementTypeFunction, - }; - break; + return getDisplayName(resolvedType); case EventComponent: - fiberData = { - displayName: null, - key, - type: ElementTypeEventComponent, - }; - break; + return null; case EventTarget: switch (getTypeSymbol(elementType.type)) { case EVENT_TARGET_TOUCH_HIT_NUMBER: case EVENT_TARGET_TOUCH_HIT_STRING: - displayName = 'TouchHitTarget'; - break; + return 'TouchHitTarget'; default: - displayName = 'EventTarget'; - break; + return 'EventTarget'; } - fiberData = { - displayName, - key, - type: ElementTypeEventTarget, - }; - break; case ForwardRef: const functionName = getDisplayName(resolvedType.render, ''); - displayName = + return ( resolvedType.displayName || - (functionName !== '' ? `ForwardRef(${functionName})` : 'ForwardRef'); - - fiberData = { - displayName, - key, - type: ElementTypeForwardRef, - }; - break; + (functionName !== '' ? `ForwardRef(${functionName})` : 'ForwardRef') + ); case HostRoot: - return { - displayName: null, - key: null, - type: ElementTypeRoot, - }; + return null; case HostComponent: - return { - displayName: type, - key, - type: ElementTypeHostComponent, - }; + return type; case HostPortal: case HostText: case Fragment: - return { - displayName: null, - key, - type: ElementTypeOtherOrUnknown, - }; + return null; case MemoComponent: case SimpleMemoComponent: if (elementType.displayName) { - displayName = elementType.displayName; + return elementType.displayName; } else { - displayName = type.displayName || type.name; - displayName = displayName ? `Memo(${displayName})` : 'Memo'; + const displayName = type.displayName || type.name; + return displayName ? `Memo(${displayName})` : 'Memo'; } - fiberData = { - displayName, - key, - type: ElementTypeMemo, - }; - break; default: const typeSymbol = getTypeSymbol(type); @@ -487,26 +429,14 @@ export function attach( case CONCURRENT_MODE_NUMBER: case CONCURRENT_MODE_SYMBOL_STRING: case DEPRECATED_ASYNC_MODE_SYMBOL_STRING: - return { - displayName: null, - key: null, - type: ElementTypeOtherOrUnknown, - }; + return null; case CONTEXT_PROVIDER_NUMBER: case CONTEXT_PROVIDER_SYMBOL_STRING: // 16.3.0 exposed the context object as "context" // PR #12501 changed it to "_context" for 16.3.1+ // NOTE Keep in sync with inspectElement() resolvedContext = fiber.type._context || fiber.type.context; - displayName = `${resolvedContext.displayName || - 'Context'}.Provider`; - - fiberData = { - displayName, - key, - type: ElementTypeContext, - }; - break; + return `${resolvedContext.displayName || 'Context'}.Provider`; case CONTEXT_CONSUMER_NUMBER: case CONTEXT_CONSUMER_SYMBOL_STRING: // 16.3-16.5 read from "type" because the Consumer is the actual context object. @@ -516,54 +446,81 @@ export function attach( // NOTE: TraceUpdatesBackendManager depends on the name ending in '.Consumer' // If you change the name, figure out a more resilient way to detect it. - displayName = `${resolvedContext.displayName || - 'Context'}.Consumer`; - - fiberData = { - displayName, - key, - type: ElementTypeContext, - }; - break; + return `${resolvedContext.displayName || 'Context'}.Consumer`; case STRICT_MODE_NUMBER: case STRICT_MODE_SYMBOL_STRING: - fiberData = { - displayName: null, - key, - type: ElementTypeOtherOrUnknown, - }; - break; + return null; case SUSPENSE_NUMBER: case SUSPENSE_SYMBOL_STRING: case DEPRECATED_PLACEHOLDER_SYMBOL_STRING: - fiberData = { - displayName: 'Suspense', - key, - type: ElementTypeSuspense, - }; - break; + return 'Suspense'; case PROFILER_NUMBER: case PROFILER_SYMBOL_STRING: - fiberData = { - displayName: `Profiler(${fiber.memoizedProps.id})`, - key, - type: ElementTypeProfiler, - }; - break; + return `Profiler(${fiber.memoizedProps.id})`; default: // Unknown element type. // This may mean a new element type that has not yet been added to DevTools. - fiberData = { - displayName: null, - key, - type: ElementTypeOtherOrUnknown, - }; - break; + return null; } - break; } + } - return fiberData; + // NOTICE Keep in sync with shouldFilterFiber() and other get*ForFiber methods + function getTypeForFiber(fiber: Fiber): ElementType { + const { type, tag } = fiber; + + switch (tag) { + case ClassComponent: + case IncompleteClassComponent: + return ElementTypeClass; + case FunctionComponent: + case IndeterminateComponent: + return ElementTypeFunction; + case EventComponent: + return ElementTypeEventComponent; + case EventTarget: + return ElementTypeEventTarget; + case ForwardRef: + return ElementTypeForwardRef; + case HostRoot: + return ElementTypeRoot; + case HostComponent: + return ElementTypeHostComponent; + case HostPortal: + case HostText: + case Fragment: + return ElementTypeOtherOrUnknown; + case MemoComponent: + case SimpleMemoComponent: + return ElementTypeMemo; + default: + const typeSymbol = getTypeSymbol(type); + + switch (typeSymbol) { + case CONCURRENT_MODE_NUMBER: + case CONCURRENT_MODE_SYMBOL_STRING: + case DEPRECATED_ASYNC_MODE_SYMBOL_STRING: + return ElementTypeOtherOrUnknown; + case CONTEXT_PROVIDER_NUMBER: + case CONTEXT_PROVIDER_SYMBOL_STRING: + return ElementTypeContext; + case CONTEXT_CONSUMER_NUMBER: + case CONTEXT_CONSUMER_SYMBOL_STRING: + return ElementTypeContext; + case STRICT_MODE_NUMBER: + case STRICT_MODE_SYMBOL_STRING: + return ElementTypeOtherOrUnknown; + case SUSPENSE_NUMBER: + case SUSPENSE_SYMBOL_STRING: + case DEPRECATED_PLACEHOLDER_SYMBOL_STRING: + return ElementTypeSuspense; + case PROFILER_NUMBER: + case PROFILER_SYMBOL_STRING: + return ElementTypeProfiler; + default: + return ElementTypeOtherOrUnknown; + } + } } // This is a slightly annoying indirection. @@ -787,7 +744,9 @@ export function attach( pushOperation(isProfilingSupported ? 1 : 0); pushOperation(hasOwnerMetadata ? 1 : 0); } else { - const { displayName, key, type } = getDataForFiber(fiber); + const { key } = fiber; + const displayName = getDisplayNameForFiber(fiber); + const type = getTypeForFiber(fiber); const { _debugOwner } = fiber; const ownerID = @@ -1647,7 +1606,7 @@ export function attach( ) { // 16.3-16.5 read from "type" because the Consumer is the actual context object. // 16.6+ should read from "type._context" because Consumer can be different (in DEV). - // NOTE Keep in sync with getDataForFiber() + // NOTE Keep in sync with get*ForFiber methods const consumerResolvedContext = type._context || type; // Global context value. @@ -1664,7 +1623,7 @@ export function attach( ) { // 16.3.0 exposed the context object as "context" // PR #12501 changed it to "_context" for 16.3.1+ - // NOTE Keep in sync with getDataForFiber() + // NOTE Keep in sync with get*ForFiber methods const providerResolvedContext = currentType._context || currentType.context; if (providerResolvedContext === consumerResolvedContext) { @@ -1689,7 +1648,7 @@ export function attach( let owner = _debugOwner; while (owner !== null) { owners.push({ - displayName: getDataForFiber(owner).displayName || 'Unknown', + displayName: getDisplayNameForFiber(owner) || 'Unknown', id: getFiberID(getPrimaryFiber(owner)), }); owner = owner._debugOwner; @@ -1719,7 +1678,7 @@ export function attach( // Can view component source location. canViewSource, - displayName: getDataForFiber(fiber).displayName, + displayName: getDisplayNameForFiber(fiber), // Inspectable properties. // TODO Review sanitization approach for the below inspectable values. @@ -2183,7 +2142,7 @@ export function attach( if (child === null) { break; } - const displayName = getDataForFiber(child).displayName; + const displayName = getDisplayNameForFiber(child); if (displayName !== null) { // Prefer display names that we get from user-defined components. // We want to avoid using e.g. 'Suspense' unless we find nothing else. @@ -2226,7 +2185,8 @@ export function attach( } function getPathFrame(fiber: Fiber): PathFrame { - let { displayName, key } = getDataForFiber(fiber); + const { key } = fiber; + let displayName = getDisplayNameForFiber(fiber); const index = fiber.index; switch (fiber.tag) { case HostRoot: diff --git a/src/devtools/store.js b/src/devtools/store.js index 539dccf71c..7c062247cf 100644 --- a/src/devtools/store.js +++ b/src/devtools/store.js @@ -251,10 +251,11 @@ export default class Store extends EventEmitter { this._filterPreferences = value; + // Update persisted filter preferences stored in localStorage. saveFilterPreferences(value); - // TODO (filter) Dump all nodes, update renderer preferences, and re-initialize tree. - // TODO (filter) Flushing every time a filter setting is changed is too expensive. We probably need an explitit configm + // Notify the renderer that filter prefernces have changed. + // This is an expensive opreation; it unmounts and remounts the entire tree. this._bridge.send('updateFilterPreferences', value); this.emit('filterPreferences'); diff --git a/src/utils.js b/src/utils.js index b20abd07fd..0e5caa2c93 100644 --- a/src/utils.js +++ b/src/utils.js @@ -89,35 +89,57 @@ export function getDefaultFilterPreferences(): FilterPreferences { }; } +function getSavedFilterPreferencesFilter(key, value) { + if (typeof value === 'string' && value.indexOf('__REGEXP__') === 0) { + const match = value.substr(9).match(/\/(.*)\/(.*)?/); + return new RegExp(match[1], match[2] || ''); + } + return value; +} + export function getSavedFilterPreferences(): FilterPreferences { const raw = localStorage.getItem(LOCAL_STORAGE_FILTER_PREFERENCES_KEY); if (raw != null) { - const json = JSON.parse(raw); + const json = JSON.parse(raw, getSavedFilterPreferencesFilter); return { hideElementsWithTypes: new Set(json.hideElementsWithTypes), - hideElementsWithDisplayNames: new Set(json.hideElementsWithDisplayNames), - hideElementsWithPaths: new Set(json.hideElementsWithPaths), + hideElementsWithDisplayNames: new Set( + json.hideElementsWithDisplayNames.map(source => new RegExp(source)) + ), + hideElementsWithPaths: new Set( + json.hideElementsWithPaths.map(source => new RegExp(source)) + ), }; } else { return getDefaultFilterPreferences(); } } +function saveFilterPreferencesFilter(key, value) { + if (value instanceof RegExp) { + return '__REGEXP__' + value.toString(); + } + return value; +} + export function saveFilterPreferences( filterPreferences: FilterPreferences ): void { localStorage.setItem( LOCAL_STORAGE_FILTER_PREFERENCES_KEY, - JSON.stringify({ - hideElementsWithTypes: Array.from( - filterPreferences.hideElementsWithTypes - ), - hideElementsWithDisplayNames: Array.from( - filterPreferences.hideElementsWithDisplayNames - ), - hideElementsWithPaths: Array.from( - filterPreferences.hideElementsWithPaths - ), - }) + JSON.stringify( + { + hideElementsWithTypes: Array.from( + filterPreferences.hideElementsWithTypes + ), + hideElementsWithDisplayNames: Array.from( + filterPreferences.hideElementsWithDisplayNames + ), + hideElementsWithPaths: Array.from( + filterPreferences.hideElementsWithPaths + ), + }, + saveFilterPreferencesFilter + ) ); } From 84292201644143dd3f2aace2753f10caec339840 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 1 May 2019 10:43:40 -0700 Subject: [PATCH 11/16] Added filter UI (but with a lot of $FlowFixMe comments) --- src/backend/agent.js | 8 +- src/backend/renderer.js | 62 ++- src/backend/types.js | 4 +- src/constants.js | 2 +- src/devtools/store.js | 24 +- src/devtools/views/Button.js | 4 +- src/devtools/views/ButtonIcon.js | 50 ++- .../Components/ComponentFiltersModal.css | 119 +++++ .../views/Components/ComponentFiltersModal.js | 406 ++++++++++++++++++ .../ComponentFiltersModalContext.js | 37 ++ .../ToggleComponentFiltersModalButton.js | 42 ++ src/devtools/views/Components/Tree.css | 1 + src/devtools/views/Components/Tree.js | 78 ++-- .../Profiler/ClearProfilingDataButton.js | 2 +- src/devtools/views/Settings/Settings.js | 58 --- .../views/Settings/SettingsContext.js | 5 + src/devtools/views/root.css | 8 + src/types.js | 38 +- src/utils.js | 79 ++-- 19 files changed, 831 insertions(+), 196 deletions(-) create mode 100644 src/devtools/views/Components/ComponentFiltersModal.css create mode 100644 src/devtools/views/Components/ComponentFiltersModal.js create mode 100644 src/devtools/views/Components/ComponentFiltersModalContext.js create mode 100644 src/devtools/views/Components/ToggleComponentFiltersModalButton.js diff --git a/src/backend/agent.js b/src/backend/agent.js index fed7344dbe..32982814df 100644 --- a/src/backend/agent.js +++ b/src/backend/agent.js @@ -16,7 +16,7 @@ import type { RendererID, RendererInterface, } from './types'; -import type { Bridge, FilterPreferences } from '../types'; +import type { Bridge, ComponentFilter } from '../types'; const debug = (methodName, ...args) => { if (__DEBUG__) { @@ -118,7 +118,7 @@ export default class Agent extends EventEmitter { this.syncSelectionFromNativeElementsPanel ); bridge.addListener('shutdown', this.shutdown); - bridge.addListener('updateFilterPreferences', this.updateFilterPreferences); + bridge.addListener('updateComponentFilters', this.updateComponentFilters); bridge.addListener('viewElementSource', this.viewElementSource); if (this._isProfiling) { @@ -490,12 +490,12 @@ export default class Agent extends EventEmitter { this._bridge.send('profilingStatus', this._isProfiling); }; - updateFilterPreferences = (filterPreferences: FilterPreferences) => { + updateComponentFilters = (componentFilters: Array) => { for (let rendererID in this._rendererInterfaces) { const renderer = ((this._rendererInterfaces[ (rendererID: any) ]: any): RendererInterface); - renderer.updateFilterPreferences(filterPreferences); + renderer.updateComponentFilters(componentFilters); } }; diff --git a/src/backend/renderer.js b/src/backend/renderer.js index 4808b694e4..403f6277c9 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -2,6 +2,9 @@ import { gte } from 'semver'; import { + ComponentFilterDisplayName, + ComponentFilterElementType, + ComponentFilterPath, ElementTypeClass, ElementTypeContext, ElementTypeEventComponent, @@ -17,7 +20,7 @@ import { } from 'src/types'; import { getDisplayName, - getSavedFilterPreferences, + getSavedComponentFilters, getUID, utfEncodeString, } from 'src/utils'; @@ -47,7 +50,7 @@ import type { RendererInterface, } from './types'; import type { InspectedElement } from 'src/devtools/views/Components/types'; -import type { ElementType, FilterPreferences } from 'src/types'; +import type { ComponentFilter, ElementType } from 'src/types'; function getInternalReactConstants(version) { const ReactSymbols = { @@ -261,17 +264,53 @@ export function attach( } }; - let { - hideElementsWithTypes, - hideElementsWithDisplayNames, - hideElementsWithPaths, - } = getSavedFilterPreferences(); + // Configurable Components tree filters. + const hideElementsWithDisplayNames: Set = new Set(); + const hideElementsWithPaths: Set = new Set(); + const hideElementsWithTypes: Set = new Set(); + + function applyComponentFilters(componentFilters: Array) { + hideElementsWithTypes.clear(); + hideElementsWithDisplayNames.clear(); + hideElementsWithPaths.clear(); + + componentFilters.forEach(componentFilter => { + if (!componentFilter.isEnabled) { + return; + } + + switch (componentFilter.type) { + case ComponentFilterDisplayName: + if (componentFilter.isValid && componentFilter.value !== '') { + hideElementsWithDisplayNames.add( + new RegExp(componentFilter.value, 'i') + ); + } + break; + case ComponentFilterElementType: + hideElementsWithTypes.add(componentFilter.value); + break; + case ComponentFilterPath: + if (componentFilter.isValid && componentFilter.value !== '') { + hideElementsWithPaths.add(new RegExp(componentFilter.value, 'i')); + } + break; + default: + console.warn( + `Invalid component filter type "${componentFilter.type}"` + ); + break; + } + }); + } + + applyComponentFilters(getSavedComponentFilters()); // TODO (filter) Should we make this operation more efficient? // For example, we could add a new recursive unmount tree operation. // The unmount operations are already significantly smaller than mount opreations though. // This is something to keep in mind for later. - function updateFilterPreferences(filterPreferences: FilterPreferences) { + function updateComponentFilters(componentFilters: Array) { if (this._isProfiling) { // Re-mounting a tree while profiling is in progress might break a lot of assumptions. // If necessary, we could support this- but it doesn't seem like a necessary use case. @@ -286,10 +325,7 @@ export function attach( currentRootID = -1; }); - hideElementsWithTypes = filterPreferences.hideElementsWithTypes; - hideElementsWithDisplayNames = - filterPreferences.hideElementsWithDisplayNames; - hideElementsWithPaths = filterPreferences.hideElementsWithPaths; + applyComponentFilters(componentFilters); // Recursively re-mount all roots with new filter criteria applied. hook.getFiberRoots(rendererID).forEach(root => { @@ -2280,6 +2316,6 @@ export function attach( setTrackedPath, startProfiling, stopProfiling, - updateFilterPreferences, + updateComponentFilters, }; } diff --git a/src/backend/types.js b/src/backend/types.js index a12dda6292..1675416f96 100644 --- a/src/backend/types.js +++ b/src/backend/types.js @@ -1,6 +1,6 @@ // @flow -import type { ElementType, FilterPreferences } from 'src/types'; +import type { ComponentFilter, ElementType } from 'src/types'; import type { InspectedElement } from 'src/devtools/views/Components/types'; type BundleType = @@ -139,7 +139,7 @@ export type RendererInterface = { setTrackedPath: (path: Array | null) => void, startProfiling: () => void, stopProfiling: () => void, - updateFilterPreferences: (filterPreferences: FilterPreferences) => void, + updateComponentFilters: (somponentFilters: Array) => void, }; export type Handler = (data: any) => void; diff --git a/src/constants.js b/src/constants.js index f5c7683d69..fcbf9cb65e 100644 --- a/src/constants.js +++ b/src/constants.js @@ -6,7 +6,7 @@ export const TREE_OPERATION_REORDER_CHILDREN = 3; export const TREE_OPERATION_UPDATE_TREE_BASE_DURATION = 4; export const LOCAL_STORAGE_FILTER_PREFERENCES_KEY = - 'React::DevTools::filterPreferences'; + 'React::DevTools::componentFilters'; export const LOCAL_STORAGE_RELOAD_AND_PROFILE_KEY = 'React::DevTools::reloadAndProfile'; diff --git a/src/devtools/store.js b/src/devtools/store.js index 7c062247cf..6da87d3032 100644 --- a/src/devtools/store.js +++ b/src/devtools/store.js @@ -11,8 +11,8 @@ import { } from '../constants'; import { ElementTypeRoot } from '../types'; import { - getSavedFilterPreferences, - saveFilterPreferences, + getSavedComponentFilters, + saveComponentFilters, utfDecodeString, } from '../utils'; import { __DEBUG__ } from '../constants'; @@ -24,7 +24,7 @@ import type { ImportedProfilingData, ProfilingSnapshotNode, } from './views/Profiler/types'; -import type { Bridge, ElementType, FilterPreferences } from '../types'; +import type { Bridge, ComponentFilter, ElementType } from '../types'; const debug = (methodName, ...args) => { if (__DEBUG__) { @@ -69,7 +69,7 @@ export default class Store extends EventEmitter { // Should new nodes be collapsed by default when added to the tree? _collapseNodesByDefault: boolean = true; - _filterPreferences: FilterPreferences; + _componentFilters: Array; // At least one of the injected renderers contains (DEV only) owner metadata. _hasOwnerMetadata: boolean = false; @@ -143,7 +143,7 @@ export default class Store extends EventEmitter { localStorage.getItem(LOCAL_STORAGE_COLLAPSE_ROOTS_BY_DEFAULT_KEY) !== 'false'; - this._filterPreferences = getSavedFilterPreferences(); + this._componentFilters = getSavedComponentFilters(); if (config != null) { const { @@ -239,26 +239,26 @@ export default class Store extends EventEmitter { this.emit('collapseNodesByDefault'); } - get filterPreferences(): FilterPreferences { - return this._filterPreferences; + get componentFilters(): Array { + return this._componentFilters; } - set filterPreferences(value: FilterPreferences): void { + set componentFilters(value: Array): void { if (this._isProfiling) { // Re-mounting a tree while profiling is in progress might break a lot of assumptions. // If necessary, we could support this- but it doesn't seem like a necessary use case. throw Error('Cannot modify filter preferences while profiling'); } - this._filterPreferences = value; + this._componentFilters = value; // Update persisted filter preferences stored in localStorage. - saveFilterPreferences(value); + saveComponentFilters(value); // Notify the renderer that filter prefernces have changed. // This is an expensive opreation; it unmounts and remounts the entire tree. - this._bridge.send('updateFilterPreferences', value); + this._bridge.send('updateComponentFilters', value); - this.emit('filterPreferences'); + this.emit('componentFilters'); } get hasOwnerMetadata(): boolean { diff --git a/src/devtools/views/Button.js b/src/devtools/views/Button.js index 78a9c8201f..bb55bca22f 100644 --- a/src/devtools/views/Button.js +++ b/src/devtools/views/Button.js @@ -9,13 +9,13 @@ import tooltipStyles from './Tooltip.css'; type Props = { children: React$Node, className?: string, - title: string, + title?: string, }; export default function Button({ children, className = '', - title, + title = '', ...rest }: Props) { let button = ( diff --git a/src/devtools/views/ButtonIcon.js b/src/devtools/views/ButtonIcon.js index 3099068b5b..0e05ce771f 100644 --- a/src/devtools/views/ButtonIcon.js +++ b/src/devtools/views/ButtonIcon.js @@ -6,6 +6,7 @@ import styles from './ButtonIcon.css'; export type IconType = | 'add' | 'cancel' + | 'clear' | 'close' | 'collapsed' | 'copy' @@ -21,7 +22,11 @@ export type IconType = | 'previous' | 'record' | 'reload' + | 'save' | 'search' + | 'settings' + | 'toggle_off' + | 'toggle_on' | 'undo' | 'up' | 'view-dom' @@ -41,6 +46,9 @@ export default function ButtonIcon({ className = '', type }: Props) { case 'cancel': pathData = PATH_CANCEL; break; + case 'clear': + pathData = PATH_CLEAR; + break; case 'close': pathData = PATH_CLOSE; break; @@ -86,9 +94,21 @@ export default function ButtonIcon({ className = '', type }: Props) { case 'reload': pathData = PATH_RELOAD; break; + case 'save': + pathData = PATH_SAVE; + break; case 'search': pathData = PATH_SEARCH; break; + case 'settings': + pathData = PATH_SETTINGS; + break; + case 'toggle_off': + pathData = PATH_TOGGLE_OFF; + break; + case 'toggle_on': + pathData = PATH_TOGGLE_ON; + break; case 'undo': pathData = PATH_UNDO; break; @@ -120,9 +140,14 @@ export default function ButtonIcon({ className = '', type }: Props) { ); } -const PATH_ADD = 'M19 13h-6v6h-2v-6H5v-2h6V5h2v6h6v2z'; +const PATH_ADD = + 'M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm5 11h-4v4h-2v-4H7v-2h4V7h2v4h4v2z'; const PATH_CANCEL = ` + M19 6.41L17.59 5 12 10.59 6.41 5 5 6.41 10.59 12 5 17.59 6.41 19 12 13.41 17.59 19 19 17.59 13.41 12z +`; + +const PATH_CLEAR = ` M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zM4 12c0-4.42 3.58-8 8-8 1.85 0 3.55.63 4.9 1.69L5.69 16.9C4.63 15.55 4 13.85 4 12zm8 8c-1.85 0-3.55-.63-4.9-1.69L18.31 7.1C19.37 8.45 20 10.15 20 12c0 4.42-3.58 8-8 8z `; @@ -138,8 +163,8 @@ const PATH_COPY = ` `; const PATH_DELETE = ` - M6 19c0 1.1.9 2 2 2h8c1.1 0 2-.9 2-2V7H6v12zm2.46-7.12l1.41-1.41L12 12.59l2.12-2.12 1.41 1.41L13.41 14l2.12 - 2.12-1.41 1.41L12 15.41l-2.12 2.12-1.41-1.41L10.59 14l-2.13-2.12zM15.5 4l-1-1h-5l-1 1H5v2h14V4z + M12 2C6.47 2 2 6.47 2 12s4.47 10 10 10 10-4.47 10-10S17.53 2 12 2zm5 13.59L15.59 17 12 + 13.41 8.41 17 7 15.59 10.59 12 7 8.41 8.41 7 12 10.59 15.59 7 17 8.41 13.41 12 17 15.59z `; const PATH_DOWN = 'M7.41 8.59L12 13.17l4.59-4.58L18 10l-6 6-6-6 1.41-1.41z'; @@ -177,11 +202,30 @@ const PATH_RELOAD = ` 3.14.69 4.22 1.78L13 11h7V4l-2.35 2.35z `; +const PATH_SAVE = ` + M17 3H5c-1.11 0-2 .9-2 2v14c0 1.1.89 2 2 2h14c1.1 0 2-.9 2-2V7l-4-4zm-5 16c-1.66 0-3-1.34-3-3s1.34-3 3-3 3 1.34 3 3-1.34 3-3 3zm3-10H5V5h10v4z +`; + const PATH_SEARCH = ` M8.5,22H3.7l-1.4-1.5V3.8l1.3-1.5h17.2l1,1.5v4.9h-1.3V4.3l-0.4-0.6H4.2L3.6,4.3V20l0.7,0.7h4.2V22z M23,13.9l-4.6,3.6l4.6,4.6l-1.1,1.1l-4.7-4.4l-3.3,4.4l-3.2-12.3L23,13.9z `; +const PATH_SETTINGS = ` + M15.95 10.78c.03-.25.05-.51.05-.78s-.02-.53-.06-.78l1.69-1.32c.15-.12.19-.34.1-.51l-1.6-2.77c-.1-.18-.31-.24-.49-.18l-1.99.8c-.42-.32-.86-.58-1.35-.78L12 + 2.34c-.03-.2-.2-.34-.4-.34H8.4c-.2 0-.36.14-.39.34l-.3 2.12c-.49.2-.94.47-1.35.78l-1.99-.8c-.18-.07-.39 + 0-.49.18l-1.6 2.77c-.1.18-.06.39.1.51l1.69 + 1.32c-.04.25-.07.52-.07.78s.02.53.06.78L2.37 12.1c-.15.12-.19.34-.1.51l1.6 2.77c.1.18.31.24.49.18l1.99-.8c.42.32.86.58 + 1.35.78l.3 2.12c.04.2.2.34.4.34h3.2c.2 0 .37-.14.39-.34l.3-2.12c.49-.2.94-.47 1.35-.78l1.99.8c.18.07.39 0 + .49-.18l1.6-2.77c.1-.18.06-.39-.1-.51l-1.67-1.32zM10 13c-1.65 0-3-1.35-3-3s1.35-3 3-3 3 1.35 3 3-1.35 3-3 3z +`; + +const PATH_TOGGLE_OFF = + 'M17 7H7c-2.76 0-5 2.24-5 5s2.24 5 5 5h10c2.76 0 5-2.24 5-5s-2.24-5-5-5zM7 15c-1.66 0-3-1.34-3-3s1.34-3 3-3 3 1.34 3 3-1.34 3-3 3z'; + +const PATH_TOGGLE_ON = + 'M17 7H7c-2.76 0-5 2.24-5 5s2.24 5 5 5h10c2.76 0 5-2.24 5-5s-2.24-5-5-5zm0 8c-1.66 0-3-1.34-3-3s1.34-3 3-3 3 1.34 3 3-1.34 3-3 3z'; + const PATH_UNDO = ` M12.5 8c-2.65 0-5.05.99-6.9 2.6L2 7v9h9l-3.62-3.62c1.39-1.16 3.16-1.88 5.12-1.88 3.54 0 6.55 2.31 7.6 5.5l2.37-.78C21.08 11.03 17.15 8 12.5 8z diff --git a/src/devtools/views/Components/ComponentFiltersModal.css b/src/devtools/views/Components/ComponentFiltersModal.css new file mode 100644 index 0000000000..1e16c7e3de --- /dev/null +++ b/src/devtools/views/Components/ComponentFiltersModal.css @@ -0,0 +1,119 @@ +.Background { + position: absolute; + width: 100%; + height: 100%; + display: flex; + align-items: flex-start; + justify-content: center; + padding: 1rem; + background-color: var(--color-modal-background); + overflow: auto; +} + +.Modal { + position: relative; + z-index: 3; + min-width: 20rem; + max-width: 100%; + display: inline-block; + background-color: var(--color-background); + padding: 0.5rem; + border: 1px solid var(--color-border); + border-radius: 0.25rem; +} + +.LeftRight { + display: flex; +} +.Left { +} +.Right { + flex: 1; + display: flex; + align-items: center; + justify-content: flex-end; +} + +.ButtonIcon { + margin-right: 0.25rem; +} + +.NoFiltersCell { + padding: 0.25rem 0; + color: var(--color-dim); +} + +.Table { + min-width: 20rem; + margin-top: 0.5rem; + border-spacing: 0; +} + +.TableRow { + padding-bottom: 0.5rem; +} + +.TableCell { + padding: 0; + padding-right: 0.5rem; +} +.TableCell:last-of-type { + text-align: right; + padding-right: 0; +} + +.Select { +} + +.Input { + border: none; + border-radius: 0.125rem; + padding: 0.125rem; +} + +.CancelButton { + margin-right: 0.25rem; +} + +.InvalidRegExp, +.InvalidRegExp:active, +.InvalidRegExp:focus, +.InvalidRegExp:hover { + color: var(--color-value-invalid); +} + +.ToggleOffInvalid, +.ToggleOnInvalid, +.ToggleOff, +.ToggleOn { + border-radius: 0.75rem; + width: 1rem; + height: 0.625rem; + display: flex; + align-items: center; + padding: 0.125rem; +} +.ToggleOffInvalid { + background-color: var(--color-toggle-background-invalid); + justify-content: flex-start; +} +.ToggleOnInvalid { + background-color: var(--color-toggle-background-invalid); + justify-content: flex-end; +} +.ToggleOff { + background-color: var(--color-toggle-background-off); + justify-content: flex-start; +} +.ToggleOn { + background-color: var(--color-toggle-background-on); + justify-content: flex-end; +} + +.ToggleInsideOff, +.ToggleInsideOn { + border-radius: 0.375rem; + width: 0.375rem; + height: 0.375rem; + background-color: var(--color-toggle-text); +} diff --git a/src/devtools/views/Components/ComponentFiltersModal.js b/src/devtools/views/Components/ComponentFiltersModal.js new file mode 100644 index 0000000000..e7a29f4e58 --- /dev/null +++ b/src/devtools/views/Components/ComponentFiltersModal.js @@ -0,0 +1,406 @@ +// @flow + +import React, { + useCallback, + useContext, + useMemo, + useRef, + useState, +} from 'react'; +import { useModalDismissSignal, useSubscription } from '../hooks'; +import { ComponentFiltersModalContext } from './ComponentFiltersModalContext'; +import { StoreContext } from '../context'; +import Button from '../Button'; +import ButtonIcon from '../ButtonIcon'; +import Toggle from '../Toggle'; +import Store from 'src/devtools/store'; +import { + ComponentFilterElementType, + ComponentFilterDisplayName, + ComponentFilterPath, + ElementTypeClass, + ElementTypeContext, + ElementTypeEventTarget, + ElementTypeFunction, + ElementTypeForwardRef, + ElementTypeHostComponent, + ElementTypeMemo, + ElementTypeOtherOrUnknown, + ElementTypeProfiler, + ElementTypeSuspense, +} from 'src/types'; +import styles from './ComponentFiltersModal.css'; + +import type { + ComponentFilter, + ElementType, + ElementTypeComponentFilter, + FilterType, + RegExpComponentFilter, +} from 'src/types'; + +export default function ComponentFiltersModalWrapper(_: {||}) { + const store = useContext(StoreContext); + + const { isModalShowing, setIsModalShowing } = useContext( + ComponentFiltersModalContext + ); + + // Re-mounting a tree while profiling is in progress might break a lot of assumptions. + // If necessary, we could support this- but it doesn't seem like a necessary use case. + const isProfilingSubscription = useMemo( + () => ({ + getCurrentValue: () => store.isProfiling, + subscribe: (callback: Function) => { + store.addListener('isProfiling', callback); + return () => store.removeListener('isProfiling', callback); + }, + }), + [store] + ); + const isProfiling = useSubscription(isProfilingSubscription); + if (isProfiling && isModalShowing) { + setIsModalShowing(false); + } + + return isModalShowing ? ( + + ) : null; +} + +type Props = {| + store: Store, + setIsModalShowing: (value: boolean) => void, +|}; + +function ComponentFiltersModal({ store, setIsModalShowing }: Props) { + const dismissModal = useCallback(() => setIsModalShowing(false), [ + setIsModalShowing, + ]); + + const modalRef = useRef(null); + + useModalDismissSignal(modalRef, dismissModal); + + const { + addFilter, + changeFilterType, + updateFilterValueElementType, + updateFilterValueRegExp, + componentFilters, + removeFilter, + saveFilters, + toggleFilterIsEnabled, + } = useComponentFilters(); + + const saveAndClose = useCallback(() => { + saveFilters(); + dismissModal(); + }, [dismissModal, saveFilters]); + + return ( +
+
+
+
Hide components where...
+
+ +
+
+ + + {componentFilters.length === 0 && ( + + + + )} + {componentFilters.map((componentFilter, index) => ( + + + + + + + + ))} + +
+ No filters have been added. +
+ + toggleFilterIsEnabled(componentFilter, isEnabled) + } + title={ + componentFilter.isValid === false + ? 'Filter invalid' + : componentFilter.isEnabled + ? 'Filter enabled' + : 'Filter disabled' + } + > + + + + + + {componentFilter.type === ComponentFilterElementType + ? 'equals' + : 'matches'} + + {componentFilter.type === ComponentFilterElementType ? ( + + ) : ( + + updateFilterValueRegExp( + componentFilter, + currentTarget.value + ) + } + value={componentFilter.value} + /> + )} + + +
+
+
+ + +
+
+
+
+ ); +} + +type ToggleIconProps = {| + isEnabled: boolean, + isValid: boolean, +|}; +function ToggleIcon({ isEnabled, isValid }: ToggleIconProps) { + let className; + if (isValid) { + className = isEnabled ? styles.ToggleOn : styles.ToggleOff; + } else { + className = isEnabled ? styles.ToggleOnInvalid : styles.ToggleOffInvalid; + } + return ( +
+
+
+ ); +} + +function useComponentFilters() { + const store = useContext(StoreContext); + + const [componentFilters, setComponentFilters] = useState< + Array + >(() => [...store.componentFilters]); + + const addFilter = useCallback(() => { + setComponentFilters(componentFilters => { + return [ + ...componentFilters, + { + type: ComponentFilterElementType, + value: ElementTypeHostComponent, + isEnabled: true, + }, + ]; + }); + }, []); + + const changeFilterType = useCallback( + (componentFilter: ComponentFilter, type: FilterType) => { + setComponentFilters(componentFilters => { + const cloned: Array = [...componentFilters]; + const index = componentFilters.indexOf(componentFilter); + if (index >= 0) { + if (type === ComponentFilterElementType) { + // $FlowFixMe TODO (filters) + cloned[index] = ({ + type, + isEnabled: componentFilter.isEnabled, + value: ElementTypeHostComponent, + }: ElementTypeComponentFilter); + } else if ( + type === ComponentFilterDisplayName || + type === ComponentFilterPath + ) { + // $FlowFixMe TODO (filters) + cloned[index] = ({ + type, + isEnabled: componentFilter.isEnabled, + isValid: true, + value: '', + }: RegExpComponentFilter); + } + } + return cloned; + }); + }, + [] + ); + + const updateFilterValueElementType = useCallback( + (componentFilter: ComponentFilter, value: ElementType) => { + if (componentFilter.type !== ComponentFilterElementType) { + throw Error('Invalid value for element type filter'); + } + + setComponentFilters(componentFilters => { + const cloned: Array = [...componentFilters]; + const index = componentFilters.indexOf(componentFilter); + if (index >= 0) { + // $FlowFixMe TODO (filters) + cloned[index] = { + ...componentFilter, + value, + }; + } + return cloned; + }); + }, + [] + ); + + const updateFilterValueRegExp = useCallback( + (componentFilter: ComponentFilter, value: string) => { + if (componentFilter.type === ComponentFilterElementType) { + throw Error('Invalid value for element type filter'); + } + + setComponentFilters(componentFilters => { + const cloned: Array = [...componentFilters]; + const index = componentFilters.indexOf(componentFilter); + if (index >= 0) { + let isValid = true; + try { + new RegExp(value); + } catch (error) { + isValid = false; + } + // $FlowFixMe TODO (filters) + cloned[index] = { + ...componentFilter, + isValid, + value, + }; + } + return cloned; + }); + }, + [] + ); + + const removeFilter = useCallback((index: number) => { + setComponentFilters(componentFilters => { + const cloned: Array = [...componentFilters]; + cloned.splice(index, 1); + return cloned; + }); + }, []); + + const saveFilters = useCallback(() => { + store.componentFilters = [...componentFilters]; + }, [componentFilters, store]); + + const toggleFilterIsEnabled = useCallback( + (componentFilter: ComponentFilter, isEnabled: boolean) => { + setComponentFilters(componentFilters => { + const cloned: Array = [...componentFilters]; + const index = componentFilters.indexOf(componentFilter); + if (index >= 0) { + // $FlowFixMe TODO (filters) + cloned[index] = { + ...cloned[index], + isEnabled, + }; + } + return cloned; + }); + }, + [] + ); + + return { + addFilter, + changeFilterType, + componentFilters, + removeFilter, + saveFilters, + toggleFilterIsEnabled, + updateFilterValueElementType, + updateFilterValueRegExp, + }; +} diff --git a/src/devtools/views/Components/ComponentFiltersModalContext.js b/src/devtools/views/Components/ComponentFiltersModalContext.js new file mode 100644 index 0000000000..6d2c46452a --- /dev/null +++ b/src/devtools/views/Components/ComponentFiltersModalContext.js @@ -0,0 +1,37 @@ +// @flow + +import React, { createContext, useMemo, useState } from 'react'; + +type Context = {| + isModalShowing: boolean, + setIsModalShowing: (value: boolean) => void, +|}; + +const ComponentFiltersModalContext = createContext( + ((null: any): Context) +); +ComponentFiltersModalContext.displayName = 'ComponentFiltersModalContext'; + +type Props = {| + children: React$Node, +|}; + +function ComponentFiltersModalContextController({ children }: Props) { + const [isModalShowing, setIsModalShowing] = useState(false); + + const value = useMemo( + () => ({ + isModalShowing, + setIsModalShowing, + }), + [isModalShowing] + ); + + return ( + + {children} + + ); +} + +export { ComponentFiltersModalContext, ComponentFiltersModalContextController }; diff --git a/src/devtools/views/Components/ToggleComponentFiltersModalButton.js b/src/devtools/views/Components/ToggleComponentFiltersModalButton.js new file mode 100644 index 0000000000..6268492030 --- /dev/null +++ b/src/devtools/views/Components/ToggleComponentFiltersModalButton.js @@ -0,0 +1,42 @@ +// @flow + +import React, { useContext, useMemo } from 'react'; +import { useSubscription } from '../hooks'; +import { ComponentFiltersModalContext } from './ComponentFiltersModalContext'; +import { StoreContext } from '../context'; +import Toggle from '../Toggle'; +import ButtonIcon from '../ButtonIcon'; +import Store from 'src/devtools/store'; + +export default function ToggleCommitFilterModalButton() { + const store = useContext(StoreContext); + + const { isModalShowing, setIsModalShowing } = useContext( + ComponentFiltersModalContext + ); + + // Re-mounting a tree while profiling is in progress might break a lot of assumptions. + // If necessary, we could support this- but it doesn't seem like a necessary use case. + const isProfilingSubscription = useMemo( + () => ({ + getCurrentValue: () => store.isProfiling, + subscribe: (callback: Function) => { + store.addListener('isProfiling', callback); + return () => store.removeListener('isProfiling', callback); + }, + }), + [store] + ); + const isProfiling = useSubscription(isProfilingSubscription); + + return ( + + + + ); +} diff --git a/src/devtools/views/Components/Tree.css b/src/devtools/views/Components/Tree.css index 98b9b85870..3ce44f0828 100644 --- a/src/devtools/views/Components/Tree.css +++ b/src/devtools/views/Components/Tree.css @@ -1,4 +1,5 @@ .Tree { + position: relative; height: 100%; width: 100%; display: flex; diff --git a/src/devtools/views/Components/Tree.js b/src/devtools/views/Components/Tree.js index 0a96f163b0..8c3c4031f7 100644 --- a/src/devtools/views/Components/Tree.js +++ b/src/devtools/views/Components/Tree.js @@ -18,6 +18,9 @@ import ElementView from './Element'; import InspectHostNodesToggle from './InspectHostNodesToggle'; import OwnersStack from './OwnersStack'; import SearchInput from './SearchInput'; +import { ComponentFiltersModalContextController } from './ComponentFiltersModalContext'; +import ToggleComponentFiltersModalButton from './ToggleComponentFiltersModalButton'; +import ComponentFiltersModal from './ComponentFiltersModal'; import styles from './Tree.css'; @@ -269,42 +272,47 @@ export default function Tree(props: Props) { ); return ( -
-
- -
- {ownerStack.length > 0 ? : } + +
+
+ +
+ {ownerStack.length > 0 ? : } +
+ +
+
+ + {({ height, width }) => ( + // $FlowFixMe https://github.com/facebook/flow/issues/7341 + + {ElementView} + + )} + +
+
-
- - {({ height, width }) => ( - // $FlowFixMe https://github.com/facebook/flow/issues/7341 - - {ElementView} - - )} - -
-
+ ); } diff --git a/src/devtools/views/Profiler/ClearProfilingDataButton.js b/src/devtools/views/Profiler/ClearProfilingDataButton.js index 082d3ec7b1..a8be6b0f6c 100644 --- a/src/devtools/views/Profiler/ClearProfilingDataButton.js +++ b/src/devtools/views/Profiler/ClearProfilingDataButton.js @@ -18,7 +18,7 @@ export default function ClearProfilingDataButton() { onClick={clear} title="Clear profiling data" > - + ); } diff --git a/src/devtools/views/Settings/Settings.js b/src/devtools/views/Settings/Settings.js index cc4faeff51..645ee25422 100644 --- a/src/devtools/views/Settings/Settings.js +++ b/src/devtools/views/Settings/Settings.js @@ -1,7 +1,6 @@ // @flow import React, { useCallback, useContext, useMemo } from 'react'; -import { ElementTypeHostComponent } from 'src/types'; import { useSubscription } from '../hooks'; import { StoreContext } from '../context'; import { SettingsContext } from './SettingsContext'; @@ -10,8 +9,6 @@ import portaledContent from '../portaledContent'; import styles from './Settings.css'; -import type { FilterPreferences } from 'src/types'; - function Settings(_: {||}) { const store = useContext(StoreContext); const { displayDensity, setDisplayDensity, theme, setTheme } = useContext( @@ -46,49 +43,6 @@ function Settings(_: {||}) { collapseNodesByDefaultSubscription ); - // Re-mounting a tree while profiling is in progress might break a lot of assumptions. - // If necessary, we could support this- but it doesn't seem like a necessary use case. - const isProfilingSubscription = useMemo( - () => ({ - getCurrentValue: () => store.isProfiling, - subscribe: (callback: Function) => { - store.addListener('isProfiling', callback); - return () => store.removeListener('isProfiling', callback); - }, - }), - [store] - ); - const isProfiling = useSubscription(isProfilingSubscription); - - const filterPreferencesSubscription = useMemo( - () => ({ - getCurrentValue: () => store.filterPreferences, - subscribe: (callback: Function) => { - store.addListener('filterPreferences', callback); - return () => store.removeListener('filterPreferences', callback); - }, - }), - [store] - ); - const filterPreferences = useSubscription( - filterPreferencesSubscription - ); - - const updateFilterPreferences = useCallback( - ({ currentTarget }) => { - const filterPreferences = store.filterPreferences; - if (currentTarget.checked) { - filterPreferences.hideElementsWithTypes.add(ElementTypeHostComponent); - } else { - filterPreferences.hideElementsWithTypes.delete( - ElementTypeHostComponent - ); - } - store.filterPreferences = { ...filterPreferences }; - }, - [store] - ); - const updateDisplayDensity = useCallback( ({ currentTarget }) => { setDisplayDensity(currentTarget.value); @@ -189,18 +143,6 @@ function Settings(_: {||}) { />{' '} Collapse newly added components by default - -
{store.supportsCaptureScreenshots && ( diff --git a/src/devtools/views/Settings/SettingsContext.js b/src/devtools/views/Settings/SettingsContext.js index 0cc5dfb9a4..e6f0ffbe80 100644 --- a/src/devtools/views/Settings/SettingsContext.js +++ b/src/devtools/views/Settings/SettingsContext.js @@ -247,6 +247,11 @@ function updateThemeVariables( updateStyleHelper(theme, 'color-tab-selected-border', documentElements); updateStyleHelper(theme, 'color-text', documentElements); updateStyleHelper(theme, 'color-text-selected', documentElements); + updateStyleHelper(theme, 'color-toggle-background-invalid', documentElements); + updateStyleHelper(theme, 'color-toggle-background-on', documentElements); + updateStyleHelper(theme, 'color-toggle-background-off', documentElements); + updateStyleHelper(theme, 'color-toggle-text', documentElements); + updateStyleHelper(theme, 'color-toggle-text', documentElements); updateStyleHelper(theme, 'color-tooltip-background', documentElements); updateStyleHelper(theme, 'color-tooltip-text', documentElements); diff --git a/src/devtools/views/root.css b/src/devtools/views/root.css index ca0582a171..296d7dd1a3 100644 --- a/src/devtools/views/root.css +++ b/src/devtools/views/root.css @@ -48,6 +48,10 @@ --light-color-tab-selected-border: #0088fa; --light-color-text: #000000; --light-color-text-selected: #ffffff; + --light-color-toggle-background-invalid: #fc3a4b; + --light-color-toggle-background-on: #0088fa; + --light-color-toggle-background-off: #cfd1d5; + --light-color-toggle-text: #ffffff; --light-color-tooltip-background: rgba(0, 0, 0, 0.9); --light-color-tooltip-text: #ffffff; @@ -96,6 +100,10 @@ --dark-color-tab-selected-border: #178fb9; --dark-color-text: #ffffff; --dark-color-text-selected: #ffffff; + --dark-color-toggle-background-invalid: #fc3a4b; + --dark-color-toggle-background-on: #178fb9; + --dark-color-toggle-background-off: #777d88; + --dark-color-toggle-text: #ffffff; --dark-color-tooltip-background: rgba(255, 255, 255, 0.9); --dark-color-tooltip-text: #000000; diff --git a/src/types.js b/src/types.js index b1af981fde..fdcac11395 100644 --- a/src/types.js +++ b/src/types.js @@ -13,7 +13,7 @@ export type Wall = {| |}; // WARNING -// The values below are referenced by FilterPreferences (which is saved via localStorage). +// The values below are referenced by ComponentFilters (which are saved via localStorage). // Do not change them or it will break previously saved user customizations. // If new element types are added, use new numbers rather than re-ordering existing ones. export const ElementTypeClass = 1; @@ -34,15 +34,33 @@ export const ElementTypeSuspense = 12; // or to enable/disable certain functionality. export type ElementType = 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12; -export type FilterPreferences = {| - // Hide all elements of types in this Set. - // We hide host components only by default. - hideElementsWithTypes: Set, +// WARNING +// The values below are referenced by ComponentFilters (which are saved via localStorage). +// Do not change them or it will break previously saved user customizations. +// If new filter types are added, use new numbers rather than re-ordering existing ones. +export const ComponentFilterElementType = 1; +export const ComponentFilterDisplayName = 2; +export const ComponentFilterPath = 3; - // Hide all elements with displayNames matching one or more of the RegExps in this Set. - hideElementsWithDisplayNames: Set, +export type FilterType = 1 | 2 | 3; - // Hide all elements within paths matching one or more of the RegExps in this Set. - // This filter is only used for elements that include debug source location. - hideElementsWithPaths: Set, +// Hide all elements of types in this Set. +// We hide host components only by default. +export type ElementTypeComponentFilter = {| + isEnabled: boolean, + type: 1, + value: ElementType, |}; + +// Hide all elements with displayNames or paths matching one or more of the RegExps in this Set. +// Path filters are only used when elements include debug source location. +export type RegExpComponentFilter = {| + isEnabled: boolean, + isValid: boolean, + type: 2 | 3, + value: string, +|}; + +export type ComponentFilter = + | ElementTypeComponentFilter + | RegExpComponentFilter; diff --git a/src/utils.js b/src/utils.js index 0e5caa2c93..1200687434 100644 --- a/src/utils.js +++ b/src/utils.js @@ -2,9 +2,9 @@ import LRU from 'lru-cache'; import { LOCAL_STORAGE_FILTER_PREFERENCES_KEY } from './constants'; -import { ElementTypeHostComponent } from './types'; +import { ComponentFilterElementType, ElementTypeHostComponent } from './types'; -import type { FilterPreferences } from './types'; +import type { ComponentFilter } from './types'; const FB_MODULE_RE = /^(.*) \[from (.*)\]$/; const cachedDisplayNames: WeakMap = new WeakMap(); @@ -81,65 +81,34 @@ function toCodePoint(string: string) { return string.codePointAt(0); } -export function getDefaultFilterPreferences(): FilterPreferences { - return { - hideElementsWithTypes: new Set([ElementTypeHostComponent]), - hideElementsWithDisplayNames: new Set(), - hideElementsWithPaths: new Set(), - }; +// TODO (filters) Save the filters as the frontend needs them (an array, with type and "enabled" status) +// Convert the fitlers to Sets for the renderer to consume. + +export function getDefaultComponentFilters(): Array { + return [ + { + type: ComponentFilterElementType, + value: ElementTypeHostComponent, + isEnabled: true, + }, + ]; } -function getSavedFilterPreferencesFilter(key, value) { - if (typeof value === 'string' && value.indexOf('__REGEXP__') === 0) { - const match = value.substr(9).match(/\/(.*)\/(.*)?/); - return new RegExp(match[1], match[2] || ''); - } - return value; +export function getSavedComponentFilters(): Array { + try { + const raw = localStorage.getItem(LOCAL_STORAGE_FILTER_PREFERENCES_KEY); + if (raw != null) { + return JSON.parse(raw); + } + } catch (error) {} + return getDefaultComponentFilters(); } -export function getSavedFilterPreferences(): FilterPreferences { - const raw = localStorage.getItem(LOCAL_STORAGE_FILTER_PREFERENCES_KEY); - if (raw != null) { - const json = JSON.parse(raw, getSavedFilterPreferencesFilter); - return { - hideElementsWithTypes: new Set(json.hideElementsWithTypes), - hideElementsWithDisplayNames: new Set( - json.hideElementsWithDisplayNames.map(source => new RegExp(source)) - ), - hideElementsWithPaths: new Set( - json.hideElementsWithPaths.map(source => new RegExp(source)) - ), - }; - } else { - return getDefaultFilterPreferences(); - } -} - -function saveFilterPreferencesFilter(key, value) { - if (value instanceof RegExp) { - return '__REGEXP__' + value.toString(); - } - return value; -} - -export function saveFilterPreferences( - filterPreferences: FilterPreferences +export function saveComponentFilters( + componentFilters: Array ): void { localStorage.setItem( LOCAL_STORAGE_FILTER_PREFERENCES_KEY, - JSON.stringify( - { - hideElementsWithTypes: Array.from( - filterPreferences.hideElementsWithTypes - ), - hideElementsWithDisplayNames: Array.from( - filterPreferences.hideElementsWithDisplayNames - ), - hideElementsWithPaths: Array.from( - filterPreferences.hideElementsWithPaths - ), - }, - saveFilterPreferencesFilter - ) + JSON.stringify(componentFilters) ); } From 540207901d4af2ed894b76b6693f9d9c84f2711a Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 1 May 2019 13:31:55 -0700 Subject: [PATCH 12/16] Added some component filtering tests --- babel.config.js | 1 + package.json | 1 + .../storeComponentFilters-test.js.snap | 98 +++++++++ src/__tests__/storeComponentFilters-test.js | 191 ++++++++++++++++++ src/backend/renderer.js | 9 +- .../Components/ComponentFiltersModal.css | 2 +- .../views/Components/ComponentFiltersModal.js | 8 +- src/types.js | 2 +- yarn.lock | 15 ++ 9 files changed, 318 insertions(+), 9 deletions(-) create mode 100644 src/__tests__/__snapshots__/storeComponentFilters-test.js.snap create mode 100644 src/__tests__/storeComponentFilters-test.js diff --git a/babel.config.js b/babel.config.js index 59bfbc27e2..12a0e78baf 100644 --- a/babel.config.js +++ b/babel.config.js @@ -29,6 +29,7 @@ module.exports = api => { plugins: [ ['@babel/plugin-transform-flow-strip-types'], ['@babel/plugin-proposal-class-properties', { loose: false }], + ['@babel/plugin-transform-react-jsx-source'], ], presets: [ ['@babel/preset-env', { targets }], diff --git a/package.json b/package.json index 4becfc24d1..7bf761f93b 100644 --- a/package.json +++ b/package.json @@ -70,6 +70,7 @@ "@babel/core": "^7.1.6", "@babel/plugin-proposal-class-properties": "^7.1.0", "@babel/plugin-transform-flow-strip-types": "^7.1.6", + "@babel/plugin-transform-react-jsx-source": "^7.2.0", "@babel/preset-env": "^7.1.6", "@babel/preset-flow": "^7.0.0", "@babel/preset-react": "^7.0.0", diff --git a/src/__tests__/__snapshots__/storeComponentFilters-test.js.snap b/src/__tests__/__snapshots__/storeComponentFilters-test.js.snap new file mode 100644 index 0000000000..86dbf043da --- /dev/null +++ b/src/__tests__/__snapshots__/storeComponentFilters-test.js.snap @@ -0,0 +1,98 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Store component filters should filter by display name: 1: mount 1`] = ` +[root] + ▾ + + ▾ + + ▾ + +`; + +exports[`Store component filters should filter by display name: 2: filter "Foo" 1`] = ` +[root] + + ▾ + + ▾ + +`; + +exports[`Store component filters should filter by display name: 3: filter "Ba" 1`] = ` +[root] + ▾ + + + +`; + +exports[`Store component filters should filter by display name: 4: filter "B.z" 1`] = ` +[root] + ▾ + + ▾ + + +`; + +exports[`Store component filters should filter by path: 1: mount 1`] = ` +[root] + ▾ +
+`; + +exports[`Store component filters should filter by path: 2: hide all components declared within this test filed 1`] = `[root]`; + +exports[`Store component filters should filter by path: 3: hide components in a made up fake path 1`] = ` +[root] + ▾ +
+`; + +exports[`Store component filters should ignore invalid ElementTypeRoot filter: 1: mount 1`] = ` +[root] + ▾ +
+`; + +exports[`Store component filters should ignore invalid ElementTypeRoot filter: 2: add invalid filter 1`] = ` +[root] + ▾ +
+`; + +exports[`Store component filters should support filtering by element type: 1: mount 1`] = ` +[root] + ▾ + ▾
+ ▾ +
+`; + +exports[`Store component filters should support filtering by element type: 2: hide host components 1`] = ` +[root] + ▾ + +`; + +exports[`Store component filters should support filtering by element type: 3: hide class components 1`] = ` +[root] + ▾
+ ▾ +
+`; + +exports[`Store component filters should support filtering by element type: 4: hide class and function components 1`] = ` +[root] + ▾
+
+`; + +exports[`Store component filters should support filtering by element type: 5: disable all filters 1`] = ` +[root] + ▾ + ▾
+ ▾ +
+`; diff --git a/src/__tests__/storeComponentFilters-test.js b/src/__tests__/storeComponentFilters-test.js new file mode 100644 index 0000000000..547bb3c801 --- /dev/null +++ b/src/__tests__/storeComponentFilters-test.js @@ -0,0 +1,191 @@ +// @flow + +describe('Store component filters', () => { + let React; + let ReactDOM; + let TestUtils; + let Types; + let store; + + const createElementTypeFilter = (elementType, isEnabled = true) => ({ + type: Types.ComponentFilterElementType, + isEnabled, + value: elementType, + }); + + const createDisplayNameFilter = (source, isEnabled = true) => { + let isValid = true; + try { + new RegExp(source); + } catch (error) { + isValid = false; + } + return { + type: Types.ComponentFilterDisplayName, + isEnabled, + isValid, + value: source, + }; + }; + + const createLocationFilter = (source, isEnabled = true) => { + let isValid = true; + try { + new RegExp(source); + } catch (error) { + isValid = false; + } + return { + type: Types.ComponentFilterLocation, + isEnabled, + isValid, + value: source, + }; + }; + + const act = (callback: Function) => { + TestUtils.act(() => { + callback(); + }); + jest.runAllTimers(); // Flush Bridge operations + }; + + beforeEach(() => { + store = global.store; + store.collapseNodesByDefault = false; + store.componentFilters = []; + + React = require('react'); + ReactDOM = require('react-dom'); + TestUtils = require('react-dom/test-utils'); + Types = require('src/types'); + }); + + it('should support filtering by element type', () => { + class Root extends React.Component<{| children: React$Node |}> { + render() { + return
{this.props.children}
; + } + } + const Component = () =>
Hi
; + + act(() => + ReactDOM.render( + + + , + document.createElement('div') + ) + ); + expect(store).toMatchSnapshot('1: mount'); + + act( + () => + (store.componentFilters = [ + createElementTypeFilter(Types.ElementTypeHostComponent), + ]) + ); + + expect(store).toMatchSnapshot('2: hide host components'); + + act( + () => + (store.componentFilters = [ + createElementTypeFilter(Types.ElementTypeClass), + ]) + ); + + expect(store).toMatchSnapshot('3: hide class components'); + + act( + () => + (store.componentFilters = [ + createElementTypeFilter(Types.ElementTypeClass), + createElementTypeFilter(Types.ElementTypeFunction), + ]) + ); + + expect(store).toMatchSnapshot('4: hide class and function components'); + + act( + () => + (store.componentFilters = [ + createElementTypeFilter(Types.ElementTypeClass, false), + createElementTypeFilter(Types.ElementTypeFunction, false), + ]) + ); + + expect(store).toMatchSnapshot('5: disable all filters'); + }); + + it('should ignore invalid ElementTypeRoot filter', () => { + const Root = () =>
Hi
; + + act(() => ReactDOM.render(, document.createElement('div'))); + expect(store).toMatchSnapshot('1: mount'); + + act( + () => + (store.componentFilters = [ + createElementTypeFilter(Types.ElementTypeRoot), + ]) + ); + + expect(store).toMatchSnapshot('2: add invalid filter'); + }); + + it('should filter by display name', () => { + const Text = ({ label }) => label; + const Foo = () => ; + const Bar = () => ; + const Baz = () => ; + + act(() => + ReactDOM.render( + + + + + , + document.createElement('div') + ) + ); + expect(store).toMatchSnapshot('1: mount'); + + act(() => (store.componentFilters = [createDisplayNameFilter('Foo')])); + expect(store).toMatchSnapshot('2: filter "Foo"'); + + act(() => (store.componentFilters = [createDisplayNameFilter('Ba')])); + expect(store).toMatchSnapshot('3: filter "Ba"'); + + act(() => (store.componentFilters = [createDisplayNameFilter('B.z')])); + expect(store).toMatchSnapshot('4: filter "B.z"'); + }); + + it('should filter by path', () => { + const Component = () =>
Hi
; + + act(() => ReactDOM.render(, document.createElement('div'))); + expect(store).toMatchSnapshot('1: mount'); + + act( + () => + (store.componentFilters = [ + createLocationFilter(__filename.replace(__dirname, '')), + ]) + ); + + expect(store).toMatchSnapshot( + '2: hide all components declared within this test filed' + ); + + act( + () => + (store.componentFilters = [ + createLocationFilter('this:is:a:made:up:path'), + ]) + ); + + expect(store).toMatchSnapshot('3: hide components in a made up fake path'); + }); +}); diff --git a/src/backend/renderer.js b/src/backend/renderer.js index 403f6277c9..cce0c7c6a7 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -4,7 +4,7 @@ import { gte } from 'semver'; import { ComponentFilterDisplayName, ComponentFilterElementType, - ComponentFilterPath, + ComponentFilterLocation, ElementTypeClass, ElementTypeContext, ElementTypeEventComponent, @@ -290,7 +290,7 @@ export function attach( case ComponentFilterElementType: hideElementsWithTypes.add(componentFilter.value); break; - case ComponentFilterPath: + case ComponentFilterLocation: if (componentFilter.isValid && componentFilter.value !== '') { hideElementsWithPaths.add(new RegExp(componentFilter.value, 'i')); } @@ -354,6 +354,9 @@ export function attach( case HostText: case Fragment: return true; + case HostRoot: + // It is never valid to filter the root element. + return false; default: const typeSymbol = getTypeSymbol(type); @@ -383,7 +386,7 @@ export function attach( } } - if (_debugSource !== null && hideElementsWithPaths.size > 0) { + if (_debugSource != null && hideElementsWithPaths.size > 0) { const { fileName } = _debugSource; for (let pathRegExp of hideElementsWithPaths) { if (pathRegExp.test(fileName)) { diff --git a/src/devtools/views/Components/ComponentFiltersModal.css b/src/devtools/views/Components/ComponentFiltersModal.css index 1e16c7e3de..43107c0c92 100644 --- a/src/devtools/views/Components/ComponentFiltersModal.css +++ b/src/devtools/views/Components/ComponentFiltersModal.css @@ -66,7 +66,7 @@ } .Input { - border: none; + border: 1px solid var(--color-border); border-radius: 0.125rem; padding: 0.125rem; } diff --git a/src/devtools/views/Components/ComponentFiltersModal.js b/src/devtools/views/Components/ComponentFiltersModal.js index e7a29f4e58..6ff939292d 100644 --- a/src/devtools/views/Components/ComponentFiltersModal.js +++ b/src/devtools/views/Components/ComponentFiltersModal.js @@ -17,7 +17,7 @@ import Store from 'src/devtools/store'; import { ComponentFilterElementType, ComponentFilterDisplayName, - ComponentFilterPath, + ComponentFilterLocation, ElementTypeClass, ElementTypeContext, ElementTypeEventTarget, @@ -164,9 +164,9 @@ function ComponentFiltersModal({ store, setIsModalShowing }: Props) { ) } > - + - + @@ -295,7 +295,7 @@ function useComponentFilters() { }: ElementTypeComponentFilter); } else if ( type === ComponentFilterDisplayName || - type === ComponentFilterPath + type === ComponentFilterLocation ) { // $FlowFixMe TODO (filters) cloned[index] = ({ diff --git a/src/types.js b/src/types.js index fdcac11395..7e38c7a4e5 100644 --- a/src/types.js +++ b/src/types.js @@ -40,7 +40,7 @@ export type ElementType = 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12; // If new filter types are added, use new numbers rather than re-ordering existing ones. export const ComponentFilterElementType = 1; export const ComponentFilterDisplayName = 2; -export const ComponentFilterPath = 3; +export const ComponentFilterLocation = 3; export type FilterType = 1 | 2 | 3; diff --git a/yarn.lock b/yarn.lock index 2aee4bbdbd..6ed41c6740 100644 --- a/yarn.lock +++ b/yarn.lock @@ -383,6 +383,13 @@ dependencies: "@babel/helper-plugin-utils" "^7.0.0" +"@babel/plugin-syntax-jsx@^7.2.0": + version "7.2.0" + resolved "https://registry.yarnpkg.com/@babel/plugin-syntax-jsx/-/plugin-syntax-jsx-7.2.0.tgz#0b85a3b4bc7cdf4cc4b8bf236335b907ca22e7c7" + integrity sha512-VyN4QANJkRW6lDBmENzRszvZf3/4AXaj9YR7GwrWeeN9tEBPuXbmDYVU9bYBN0D70zCWVwUy0HWq2553VCb6Hw== + dependencies: + "@babel/helper-plugin-utils" "^7.0.0" + "@babel/plugin-syntax-object-rest-spread@^7.0.0", "@babel/plugin-syntax-object-rest-spread@^7.2.0": version "7.2.0" resolved "https://registry.yarnpkg.com/@babel/plugin-syntax-object-rest-spread/-/plugin-syntax-object-rest-spread-7.2.0.tgz#3b7a3e733510c57e820b9142a6579ac8b0dfad2e" @@ -605,6 +612,14 @@ "@babel/helper-plugin-utils" "^7.0.0" "@babel/plugin-syntax-jsx" "^7.0.0" +"@babel/plugin-transform-react-jsx-source@^7.2.0": + version "7.2.0" + resolved "https://registry.yarnpkg.com/@babel/plugin-transform-react-jsx-source/-/plugin-transform-react-jsx-source-7.2.0.tgz#20c8c60f0140f5dd3cd63418d452801cf3f7180f" + integrity sha512-A32OkKTp4i5U6aE88GwwcuV4HAprUgHcTq0sSafLxjr6AW0QahrCRCjxogkbbcdtpbXkuTOlgpjophCxb6sh5g== + dependencies: + "@babel/helper-plugin-utils" "^7.0.0" + "@babel/plugin-syntax-jsx" "^7.2.0" + "@babel/plugin-transform-react-jsx@^7.0.0": version "7.1.6" resolved "https://registry.yarnpkg.com/@babel/plugin-transform-react-jsx/-/plugin-transform-react-jsx-7.1.6.tgz#e6188e7d2a2dcd2796d45a87f8b0a8c906f57d1a" From e45deb5471ac8ec836635d9949fb6557dcec6a93 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 1 May 2019 13:59:54 -0700 Subject: [PATCH 13/16] Removed $FlowFixMe comments --- .../views/Components/ComponentFiltersModal.js | 99 +++++++++++-------- src/utils.js | 3 - 2 files changed, 58 insertions(+), 44 deletions(-) diff --git a/src/devtools/views/Components/ComponentFiltersModal.js b/src/devtools/views/Components/ComponentFiltersModal.js index 6ff939292d..a23e1af8f5 100644 --- a/src/devtools/views/Components/ComponentFiltersModal.js +++ b/src/devtools/views/Components/ComponentFiltersModal.js @@ -159,8 +159,7 @@ function ComponentFiltersModal({ store, setIsModalShowing }: Props) { onChange={({ currentTarget }) => changeFilterType( componentFilter, - // $FlowFixMe TODO (filters) - parseInt(currentTarget.value, 10) + ((parseInt(currentTarget.value, 10): any): FilterType) ) } > @@ -182,8 +181,10 @@ function ComponentFiltersModal({ store, setIsModalShowing }: Props) { onChange={({ currentTarget }) => updateFilterValueElementType( componentFilter, - // $FlowFixMe TODO (filters) - parseInt(currentTarget.value, 10) + ((parseInt( + currentTarget.value, + 10 + ): any): ElementType) ) } > @@ -287,23 +288,25 @@ function useComponentFilters() { const index = componentFilters.indexOf(componentFilter); if (index >= 0) { if (type === ComponentFilterElementType) { - // $FlowFixMe TODO (filters) - cloned[index] = ({ - type, + cloned[index] = { + type: ComponentFilterElementType, isEnabled: componentFilter.isEnabled, value: ElementTypeHostComponent, - }: ElementTypeComponentFilter); - } else if ( - type === ComponentFilterDisplayName || - type === ComponentFilterLocation - ) { - // $FlowFixMe TODO (filters) - cloned[index] = ({ - type, + }; + } else if (type === ComponentFilterDisplayName) { + cloned[index] = { + type: ComponentFilterDisplayName, isEnabled: componentFilter.isEnabled, isValid: true, value: '', - }: RegExpComponentFilter); + }; + } else if (type === ComponentFilterLocation) { + cloned[index] = { + type: ComponentFilterLocation, + isEnabled: componentFilter.isEnabled, + isValid: true, + value: '', + }; } } return cloned; @@ -320,13 +323,14 @@ function useComponentFilters() { setComponentFilters(componentFilters => { const cloned: Array = [...componentFilters]; - const index = componentFilters.indexOf(componentFilter); - if (index >= 0) { - // $FlowFixMe TODO (filters) - cloned[index] = { - ...componentFilter, - value, - }; + if (componentFilter.type === ComponentFilterElementType) { + const index = componentFilters.indexOf(componentFilter); + if (index >= 0) { + cloned[index] = { + ...componentFilter, + value, + }; + } } return cloned; }); @@ -342,20 +346,24 @@ function useComponentFilters() { setComponentFilters(componentFilters => { const cloned: Array = [...componentFilters]; - const index = componentFilters.indexOf(componentFilter); - if (index >= 0) { - let isValid = true; - try { - new RegExp(value); - } catch (error) { - isValid = false; + if ( + componentFilter.type === ComponentFilterDisplayName || + componentFilter.type === ComponentFilterLocation + ) { + const index = componentFilters.indexOf(componentFilter); + if (index >= 0) { + let isValid = true; + try { + new RegExp(value); + } catch (error) { + isValid = false; + } + cloned[index] = { + ...componentFilter, + isValid, + value, + }; } - // $FlowFixMe TODO (filters) - cloned[index] = { - ...componentFilter, - isValid, - value, - }; } return cloned; }); @@ -381,11 +389,20 @@ function useComponentFilters() { const cloned: Array = [...componentFilters]; const index = componentFilters.indexOf(componentFilter); if (index >= 0) { - // $FlowFixMe TODO (filters) - cloned[index] = { - ...cloned[index], - isEnabled, - }; + if (componentFilter.type === ComponentFilterElementType) { + cloned[index] = { + ...((cloned[index]: any): ElementTypeComponentFilter), + isEnabled, + }; + } else if ( + componentFilter.type === ComponentFilterDisplayName || + componentFilter.type === ComponentFilterLocation + ) { + cloned[index] = { + ...((cloned[index]: any): RegExpComponentFilter), + isEnabled, + }; + } } return cloned; }); diff --git a/src/utils.js b/src/utils.js index 1200687434..499a72bb1d 100644 --- a/src/utils.js +++ b/src/utils.js @@ -81,9 +81,6 @@ function toCodePoint(string: string) { return string.codePointAt(0); } -// TODO (filters) Save the filters as the frontend needs them (an array, with type and "enabled" status) -// Convert the fitlers to Sets for the renderer to consume. - export function getDefaultComponentFilters(): Array { return [ { From e6da4b9058987611018e64309225c38566516dff Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 1 May 2019 14:01:03 -0700 Subject: [PATCH 14/16] Renamed FilterType -> ComponentFilterType --- src/devtools/views/Components/ComponentFiltersModal.js | 9 ++++++--- src/types.js | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/devtools/views/Components/ComponentFiltersModal.js b/src/devtools/views/Components/ComponentFiltersModal.js index a23e1af8f5..45e00b7ff4 100644 --- a/src/devtools/views/Components/ComponentFiltersModal.js +++ b/src/devtools/views/Components/ComponentFiltersModal.js @@ -33,9 +33,9 @@ import styles from './ComponentFiltersModal.css'; import type { ComponentFilter, + ComponentFilterType, ElementType, ElementTypeComponentFilter, - FilterType, RegExpComponentFilter, } from 'src/types'; @@ -159,7 +159,10 @@ function ComponentFiltersModal({ store, setIsModalShowing }: Props) { onChange={({ currentTarget }) => changeFilterType( componentFilter, - ((parseInt(currentTarget.value, 10): any): FilterType) + ((parseInt( + currentTarget.value, + 10 + ): any): ComponentFilterType) ) } > @@ -282,7 +285,7 @@ function useComponentFilters() { }, []); const changeFilterType = useCallback( - (componentFilter: ComponentFilter, type: FilterType) => { + (componentFilter: ComponentFilter, type: ComponentFilterType) => { setComponentFilters(componentFilters => { const cloned: Array = [...componentFilters]; const index = componentFilters.indexOf(componentFilter); diff --git a/src/types.js b/src/types.js index 7e38c7a4e5..7f89612cc7 100644 --- a/src/types.js +++ b/src/types.js @@ -42,7 +42,7 @@ export const ComponentFilterElementType = 1; export const ComponentFilterDisplayName = 2; export const ComponentFilterLocation = 3; -export type FilterType = 1 | 2 | 3; +export type ComponentFilterType = 1 | 2 | 3; // Hide all elements of types in this Set. // We hide host components only by default. From 4a4786ac2faaa558a4bb32a51cf6eaedcefb2cdc Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 1 May 2019 14:18:20 -0700 Subject: [PATCH 15/16] Misc cleanup --- src/__tests__/storeComponentFilters-test.js | 7 +++++ src/backend/renderer.js | 30 ++++++++++--------- src/devtools/views/ButtonIcon.js | 14 --------- .../views/Settings/SettingsContext.js | 1 - src/devtools/views/hooks.js | 2 +- 5 files changed, 24 insertions(+), 30 deletions(-) diff --git a/src/__tests__/storeComponentFilters-test.js b/src/__tests__/storeComponentFilters-test.js index 547bb3c801..8f408f49f7 100644 --- a/src/__tests__/storeComponentFilters-test.js +++ b/src/__tests__/storeComponentFilters-test.js @@ -61,6 +61,13 @@ describe('Store component filters', () => { Types = require('src/types'); }); + it('should throw if filters are updated while profiling', () => { + act(() => store.startProfiling()); + expect(() => (store.componentFilters = [])).toThrow( + 'Cannot modify filter preferences while profiling' + ); + }); + it('should support filtering by element type', () => { class Root extends React.Component<{| children: React$Node |}> { render() { diff --git a/src/backend/renderer.js b/src/backend/renderer.js index cce0c7c6a7..6b79b12210 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -306,7 +306,7 @@ export function attach( applyComponentFilters(getSavedComponentFilters()); - // TODO (filter) Should we make this operation more efficient? + // If necessary, we can revisit optimizing this operation. // For example, we could add a new recursive unmount tree operation. // The unmount operations are already significantly smaller than mount opreations though. // This is something to keep in mind for later. @@ -317,7 +317,7 @@ export function attach( throw Error('Cannot modify filter preferences while profiling'); } - // Recursively unmount and then re-mount all roots. + // Recursively unmount all roots. hook.getFiberRoots(rendererID).forEach(root => { currentRootID = getFiberID(getPrimaryFiber(root.current)); unmountFiberChildrenRecursively(root.current); @@ -372,16 +372,18 @@ export function attach( } } - const elementType = getTypeForFiber(fiber); + const elementType = getElementTypeForFiber(fiber); if (hideElementsWithTypes.has(elementType)) { return true; } if (hideElementsWithDisplayNames.size > 0) { - const displayName = getDisplayNameForFiber(fiber) || ''; - for (let displayNameRegExp of hideElementsWithDisplayNames) { - if (displayNameRegExp.test(displayName)) { - return true; + const displayName = getDisplayNameForFiber(fiber); + if (displayName != null) { + for (let displayNameRegExp of hideElementsWithDisplayNames) { + if (displayNameRegExp.test(displayName)) { + return true; + } } } } @@ -473,14 +475,14 @@ export function attach( case CONTEXT_PROVIDER_SYMBOL_STRING: // 16.3.0 exposed the context object as "context" // PR #12501 changed it to "_context" for 16.3.1+ - // NOTE Keep in sync with inspectElement() + // NOTE Keep in sync with inspectElementRaw() resolvedContext = fiber.type._context || fiber.type.context; return `${resolvedContext.displayName || 'Context'}.Provider`; case CONTEXT_CONSUMER_NUMBER: case CONTEXT_CONSUMER_SYMBOL_STRING: // 16.3-16.5 read from "type" because the Consumer is the actual context object. // 16.6+ should read from "type._context" because Consumer can be different (in DEV). - // NOTE Keep in sync with inspectElement() + // NOTE Keep in sync with inspectElementRaw() resolvedContext = fiber.type._context || fiber.type; // NOTE: TraceUpdatesBackendManager depends on the name ending in '.Consumer' @@ -505,7 +507,7 @@ export function attach( } // NOTICE Keep in sync with shouldFilterFiber() and other get*ForFiber methods - function getTypeForFiber(fiber: Fiber): ElementType { + function getElementTypeForFiber(fiber: Fiber): ElementType { const { type, tag } = fiber; switch (tag) { @@ -785,7 +787,7 @@ export function attach( } else { const { key } = fiber; const displayName = getDisplayNameForFiber(fiber); - const type = getTypeForFiber(fiber); + const elementType = getElementTypeForFiber(fiber); const { _debugOwner } = fiber; const ownerID = @@ -796,7 +798,7 @@ export function attach( let keyStringID = getStringID(key); pushOperation(TREE_OPERATION_ADD); pushOperation(id); - pushOperation(type); + pushOperation(elementType); pushOperation(parentID); pushOperation(ownerID); pushOperation(displayNameStringID); @@ -1645,7 +1647,7 @@ export function attach( ) { // 16.3-16.5 read from "type" because the Consumer is the actual context object. // 16.6+ should read from "type._context" because Consumer can be different (in DEV). - // NOTE Keep in sync with get*ForFiber methods + // NOTE Keep in sync with getDisplayNameForFiber() const consumerResolvedContext = type._context || type; // Global context value. @@ -1662,7 +1664,7 @@ export function attach( ) { // 16.3.0 exposed the context object as "context" // PR #12501 changed it to "_context" for 16.3.1+ - // NOTE Keep in sync with get*ForFiber methods + // NOTE Keep in sync with getDisplayNameForFiber() const providerResolvedContext = currentType._context || currentType.context; if (providerResolvedContext === consumerResolvedContext) { diff --git a/src/devtools/views/ButtonIcon.js b/src/devtools/views/ButtonIcon.js index 0e05ce771f..6e67143f89 100644 --- a/src/devtools/views/ButtonIcon.js +++ b/src/devtools/views/ButtonIcon.js @@ -25,8 +25,6 @@ export type IconType = | 'save' | 'search' | 'settings' - | 'toggle_off' - | 'toggle_on' | 'undo' | 'up' | 'view-dom' @@ -103,12 +101,6 @@ export default function ButtonIcon({ className = '', type }: Props) { case 'settings': pathData = PATH_SETTINGS; break; - case 'toggle_off': - pathData = PATH_TOGGLE_OFF; - break; - case 'toggle_on': - pathData = PATH_TOGGLE_ON; - break; case 'undo': pathData = PATH_UNDO; break; @@ -220,12 +212,6 @@ const PATH_SETTINGS = ` .49-.18l1.6-2.77c.1-.18.06-.39-.1-.51l-1.67-1.32zM10 13c-1.65 0-3-1.35-3-3s1.35-3 3-3 3 1.35 3 3-1.35 3-3 3z `; -const PATH_TOGGLE_OFF = - 'M17 7H7c-2.76 0-5 2.24-5 5s2.24 5 5 5h10c2.76 0 5-2.24 5-5s-2.24-5-5-5zM7 15c-1.66 0-3-1.34-3-3s1.34-3 3-3 3 1.34 3 3-1.34 3-3 3z'; - -const PATH_TOGGLE_ON = - 'M17 7H7c-2.76 0-5 2.24-5 5s2.24 5 5 5h10c2.76 0 5-2.24 5-5s-2.24-5-5-5zm0 8c-1.66 0-3-1.34-3-3s1.34-3 3-3 3 1.34 3 3-1.34 3-3 3z'; - const PATH_UNDO = ` M12.5 8c-2.65 0-5.05.99-6.9 2.6L2 7v9h9l-3.62-3.62c1.39-1.16 3.16-1.88 5.12-1.88 3.54 0 6.55 2.31 7.6 5.5l2.37-.78C21.08 11.03 17.15 8 12.5 8z diff --git a/src/devtools/views/Settings/SettingsContext.js b/src/devtools/views/Settings/SettingsContext.js index e6f0ffbe80..0136c8bbc8 100644 --- a/src/devtools/views/Settings/SettingsContext.js +++ b/src/devtools/views/Settings/SettingsContext.js @@ -251,7 +251,6 @@ function updateThemeVariables( updateStyleHelper(theme, 'color-toggle-background-on', documentElements); updateStyleHelper(theme, 'color-toggle-background-off', documentElements); updateStyleHelper(theme, 'color-toggle-text', documentElements); - updateStyleHelper(theme, 'color-toggle-text', documentElements); updateStyleHelper(theme, 'color-tooltip-background', documentElements); updateStyleHelper(theme, 'color-tooltip-text', documentElements); diff --git a/src/devtools/views/hooks.js b/src/devtools/views/hooks.js index b9e7dfc0b4..c6d8ce0984 100644 --- a/src/devtools/views/hooks.js +++ b/src/devtools/views/hooks.js @@ -50,7 +50,7 @@ export function useLocalStorage( console.log(error); } if (typeof initialValue === 'function') { - return (initialValue: any)(); + return ((initialValue: any): () => T)(); } else { return initialValue; } From 83b521c7b472823548292480f7bff4d5bb6c5c1f Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 1 May 2019 17:44:38 -0700 Subject: [PATCH 16/16] Updated Flamechart to support multiple nodes at root --- src/devtools/views/Profiler/CommitFlamegraph.js | 16 +++++++++------- .../views/Profiler/CommitFlamegraphListItem.js | 5 ++++- .../views/Profiler/FlamegraphChartBuilder.js | 11 +++++++++-- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/devtools/views/Profiler/CommitFlamegraph.js b/src/devtools/views/Profiler/CommitFlamegraph.js index 84a1ca5134..881da0e77b 100644 --- a/src/devtools/views/Profiler/CommitFlamegraph.js +++ b/src/devtools/views/Profiler/CommitFlamegraph.js @@ -18,7 +18,7 @@ import type { CommitDetailsFrontend, CommitTreeFrontend } from './types'; export type ItemData = {| chartData: ChartData, scaleX: (value: number, fallbackValue: number) => number, - selectedChartNode: ChartNode, + selectedChartNode: ChartNode | null, selectedChartNodeIndex: number, selectFiber: (id: number | null, name: string | null) => void, width: number, @@ -127,18 +127,20 @@ function CommitFlamegraph({ chartNode => chartNode.id === selectedFiberID ): any): ChartNode); } - // The selected node might not be in the tree for this commit, - // so it's important that we have a fallback plan. - if (chartNode == null) { - return chartData.rows[0][0]; - } return chartNode; }, [chartData, selectedFiberID, selectedChartNodeIndex]); const itemData = useMemo( () => ({ chartData, - scaleX: scale(0, selectedChartNode.treeBaseDuration, 0, width), + scaleX: scale( + 0, + selectedChartNode !== null + ? selectedChartNode.treeBaseDuration + : chartData.baseDuration, + 0, + width + ), selectedChartNode, selectedChartNodeIndex, selectFiber, diff --git a/src/devtools/views/Profiler/CommitFlamegraphListItem.js b/src/devtools/views/Profiler/CommitFlamegraphListItem.js index 8d55d781b0..171ebebb45 100644 --- a/src/devtools/views/Profiler/CommitFlamegraphListItem.js +++ b/src/devtools/views/Profiler/CommitFlamegraphListItem.js @@ -41,7 +41,10 @@ function CommitFlamegraphListItem({ data, index, style }: Props) { const row = rows[index]; - let selectedNodeOffset = scaleX(selectedChartNode.offset, width); + let selectedNodeOffset = scaleX( + selectedChartNode !== null ? selectedChartNode.offset : 0, + width + ); return ( diff --git a/src/devtools/views/Profiler/FlamegraphChartBuilder.js b/src/devtools/views/Profiler/FlamegraphChartBuilder.js index 8096972175..435c6f8806 100644 --- a/src/devtools/views/Profiler/FlamegraphChartBuilder.js +++ b/src/devtools/views/Profiler/FlamegraphChartBuilder.js @@ -16,6 +16,7 @@ export type ChartNode = {| |}; export type ChartData = {| + baseDuration: number, depth: number, idToDepthMap: Map, maxSelfDuration: number, @@ -108,10 +109,16 @@ export function getChartData({ throw Error(`Could not find root node with id "${rootID}" in commit tree`); } - // TODO: Looks like there's an assumption here that a root has only one child. Is that so with a fragment in the root? - walkTree(root.children[0]); + // Don't assume a single root. + // Component filters or Fragments might lead to multiple "roots" in a flame graph. + let baseDuration = 0; + root.children.forEach(childID => { + const chartNode = walkTree(childID, baseDuration); + baseDuration += chartNode.treeBaseDuration; + }); const chartData = { + baseDuration, depth: maxDepth, idToDepthMap, maxSelfDuration,