From 3d09b4aaa5aa2e2096ba87e2448f4056185b0b6f Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 19 Apr 2019 01:54:39 +0100 Subject: [PATCH 1/5] Don't select search result unless user does something --- src/devtools/views/Components/TreeContext.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/devtools/views/Components/TreeContext.js b/src/devtools/views/Components/TreeContext.js index 0be84a9e1d..f66ad3e46e 100644 --- a/src/devtools/views/Components/TreeContext.js +++ b/src/devtools/views/Components/TreeContext.js @@ -353,10 +353,6 @@ function reduceSearchState(store: Store, state: State, action: Action): State { } } - if (searchIndex !== prevSearchIndex) { - // The user intentionally navigated between search results - didRequestSearch = true; - } if (searchText !== prevSearchText) { const newSearchIndex = searchResults.indexOf(selectedElementID); if (newSearchIndex === -1) { From 24736e4ff57832b9fcc1da8137729ae65a78124a Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 18 Apr 2019 17:58:48 -0700 Subject: [PATCH 2/5] Hardened reorder child invariants --- src/devtools/store.js | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/devtools/store.js b/src/devtools/store.js index 1010f5c56e..35ae73363c 100644 --- a/src/devtools/store.js +++ b/src/devtools/store.js @@ -714,12 +714,7 @@ export default class Store extends EventEmitter { weightDelta = -element.weight; if (element.children.length > 0) { - throw new Error( - 'Fiber ' + - id + - ' was removed before its children. ' + - 'This is a bug in React DevTools.' - ); + throw new Error(`Node ${id} was removed before its children.`); } this._idToElement.delete(id); @@ -759,7 +754,7 @@ export default class Store extends EventEmitter { case TREE_OPERATION_RESET_CHILDREN: id = ((operations[i + 1]: any): number); const numChildren = ((operations[i + 2]: any): number); - const children = ((operations.slice( + const nextChildren = ((operations.slice( i + 3, i + 3 + numChildren ): any): Array); @@ -767,7 +762,7 @@ export default class Store extends EventEmitter { i = i + 3 + numChildren; if (__DEBUG__) { - debug('Re-order', `Node ${id} children ${children.join(',')}`); + debug('Re-order', `Node ${id} children ${nextChildren.join(',')}`); } if (!this._idToElement.has(id)) { @@ -777,23 +772,28 @@ export default class Store extends EventEmitter { } element = ((this._idToElement.get(id): any): Element); + const prevChildren = element.children; - element.children = Array.from(children); - if (element.children.length !== prevChildren.length) { - throw new Error( - 'Fiber ' + - id + - ' received a different number of children on reorder. ' + - 'This is a bug in React DevTools.' + if ( + nextChildren.length !== prevChildren.length || + nextChildren.find(childID => { + const childElement = this._idToElement.get(childID); + return childElement == null || childElement.parentID !== id; + }) != null + ) { + throw Error( + `Children cannot be added or removed during a reorder operation.` ); } + element.children = Array.from(nextChildren); + if (!element.isCollapsed) { const prevWeight = element.weight; let nextWeight = element.type === ElementTypeRoot ? 0 : 1; - children.forEach(childID => { + nextChildren.forEach(childID => { const child = ((this._idToElement.get(childID): any): Element); nextWeight += child.isCollapsed ? 1 : child.weight; }); From 60b614f5272fd0fa400c555e011f8262582c5b55 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 18 Apr 2019 17:59:53 -0700 Subject: [PATCH 3/5] Renamed TREE_OPERATION_RESET_CHILDREN -> TREE_OPERATION_REORDER_CHILDREN --- src/backend/renderer.js | 4 ++-- src/constants.js | 2 +- src/devtools/store.js | 4 ++-- src/devtools/views/Profiler/CommitTreeBuilder.js | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/backend/renderer.js b/src/backend/renderer.js index 3c08c9adcd..af10a64a72 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -21,7 +21,7 @@ import { LOCAL_STORAGE_RELOAD_AND_PROFILE_KEY, TREE_OPERATION_ADD, TREE_OPERATION_REMOVE, - TREE_OPERATION_RESET_CHILDREN, + TREE_OPERATION_REORDER_CHILDREN, TREE_OPERATION_UPDATE_TREE_BASE_DURATION, } from '../constants'; import { getUID } from '../utils'; @@ -876,7 +876,7 @@ export function attach( const numChildren = nextChildren.length; beginNextOperation(3 + numChildren); - nextOperation[0] = TREE_OPERATION_RESET_CHILDREN; + nextOperation[0] = TREE_OPERATION_REORDER_CHILDREN; nextOperation[1] = getFiberID(getPrimaryFiber(fiber)); nextOperation[2] = numChildren; for (let i = 0; i < nextChildren.length; i++) { diff --git a/src/constants.js b/src/constants.js index ef69e21760..d64cc5da32 100644 --- a/src/constants.js +++ b/src/constants.js @@ -2,7 +2,7 @@ export const TREE_OPERATION_ADD = 1; export const TREE_OPERATION_REMOVE = 2; -export const TREE_OPERATION_RESET_CHILDREN = 3; +export const TREE_OPERATION_REORDER_CHILDREN = 3; export const TREE_OPERATION_UPDATE_TREE_BASE_DURATION = 4; export const LOCAL_STORAGE_RELOAD_AND_PROFILE_KEY = diff --git a/src/devtools/store.js b/src/devtools/store.js index 35ae73363c..b2b18e118c 100644 --- a/src/devtools/store.js +++ b/src/devtools/store.js @@ -6,7 +6,7 @@ import throttle from 'lodash.throttle'; import { TREE_OPERATION_ADD, TREE_OPERATION_REMOVE, - TREE_OPERATION_RESET_CHILDREN, + TREE_OPERATION_REORDER_CHILDREN, TREE_OPERATION_UPDATE_TREE_BASE_DURATION, } from '../constants'; import { ElementTypeRoot } from './types'; @@ -751,7 +751,7 @@ export default class Store extends EventEmitter { removedElementIDs[oldRemovedElementIDs.length] = id; break; } - case TREE_OPERATION_RESET_CHILDREN: + case TREE_OPERATION_REORDER_CHILDREN: id = ((operations[i + 1]: any): number); const numChildren = ((operations[i + 2]: any): number); const nextChildren = ((operations.slice( diff --git a/src/devtools/views/Profiler/CommitTreeBuilder.js b/src/devtools/views/Profiler/CommitTreeBuilder.js index b077f51e3b..5f905b1391 100644 --- a/src/devtools/views/Profiler/CommitTreeBuilder.js +++ b/src/devtools/views/Profiler/CommitTreeBuilder.js @@ -4,7 +4,7 @@ import { __DEBUG__, TREE_OPERATION_ADD, TREE_OPERATION_REMOVE, - TREE_OPERATION_RESET_CHILDREN, + TREE_OPERATION_REORDER_CHILDREN, TREE_OPERATION_UPDATE_TREE_BASE_DURATION, } from 'src/constants'; import { utfDecodeString } from 'src/utils'; @@ -286,7 +286,7 @@ function updateTree( ); } break; - case TREE_OPERATION_RESET_CHILDREN: + case TREE_OPERATION_REORDER_CHILDREN: id = ((operations[i + 1]: any): number); const numChildren = ((operations[i + 2]: any): number); const children = ((operations.slice( From c0b414d58140f2dda765ef114d981a967992026b Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 18 Apr 2019 18:13:34 -0700 Subject: [PATCH 4/5] Node scripts should fail if not NODE_ENV specified --- package.json | 2 +- shells/browser/shared/webpack.backend.js | 8 +++++++- shells/browser/shared/webpack.config.js | 5 +++++ shells/dev/webpack.config.js | 8 +++++++- 4 files changed, 20 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 60850a905b..48c6dd7a8e 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,7 @@ "precommit": "lint-staged", "prettier": "prettier --write '**/*.{js,json,css}'", "prettier:ci": "prettier --check '**/*.{js,json,css}'", - "start": "cd ./shells/dev && opener ./index.html && webpack --config webpack.config.js --watch", + "start": "cd ./shells/dev && opener ./index.html && cross-env NODE_ENV=development webpack --config webpack.config.js --watch", "start:prod": "cross-env NODE_ENV=production npm start", "test": "jest", "test-debug": "node --inspect-brk node_modules/.bin/jest --runInBand", diff --git a/shells/browser/shared/webpack.backend.js b/shells/browser/shared/webpack.backend.js index 66554210c0..99249b3053 100644 --- a/shells/browser/shared/webpack.backend.js +++ b/shells/browser/shared/webpack.backend.js @@ -3,7 +3,13 @@ const { resolve } = require('path'); const { DefinePlugin } = require('webpack'); const { getGitHubURL, getVersionString } = require('../../utils'); -const __DEV__ = process.env.NODE_ENV === 'development'; +const NODE_ENV = process.env.NODE_ENV; +if (!NODE_ENV) { + console.error('NODE_ENV not set'); + process.exit(1); +} + +const __DEV__ = NODE_ENV === 'development'; const GITHUB_URL = getGitHubURL(); const DEVTOOLS_VERSION = getVersionString(); diff --git a/shells/browser/shared/webpack.config.js b/shells/browser/shared/webpack.config.js index 827d75a30c..8ff77ebc6e 100644 --- a/shells/browser/shared/webpack.config.js +++ b/shells/browser/shared/webpack.config.js @@ -4,6 +4,11 @@ const { DefinePlugin } = require('webpack'); const { getGitHubURL, getVersionString } = require('../../utils'); const NODE_ENV = process.env.NODE_ENV; +if (!NODE_ENV) { + console.error('NODE_ENV not set'); + process.exit(1); +} + const __DEV__ = NODE_ENV === 'development'; const GITHUB_URL = getGitHubURL(); diff --git a/shells/dev/webpack.config.js b/shells/dev/webpack.config.js index deb695ed44..7cd9c53b2a 100644 --- a/shells/dev/webpack.config.js +++ b/shells/dev/webpack.config.js @@ -3,7 +3,13 @@ const { resolve } = require('path'); const { DefinePlugin } = require('webpack'); const { getGitHubURL, getVersionString } = require('../utils'); -const __DEV__ = process.env.NODE_ENV === 'development'; +const NODE_ENV = process.env.NODE_ENV; +if (!NODE_ENV) { + console.error('NODE_ENV not set'); + process.exit(1); +} + +const __DEV__ = NODE_ENV === 'development'; const GITHUB_URL = getGitHubURL(); const DEVTOOLS_VERSION = getVersionString(); From f286c0d901588b6d2ba811e4b9077a65279bdd70 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 18 Apr 2019 18:19:10 -0700 Subject: [PATCH 5/5] Increased contrast for selected+inactive background color --- src/devtools/views/root.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/devtools/views/root.css b/src/devtools/views/root.css index fd71c319ea..83dabea744 100644 --- a/src/devtools/views/root.css +++ b/src/devtools/views/root.css @@ -35,7 +35,7 @@ --light-color-dimmest: #eff0f1; --light-color-expand-collapse-toggle: #777d88; --light-color-hover-background: #ebf1fb; - --light-color-inactive-background: #f1f1f1; + --light-color-inactive-background: #e5e5e5; --light-color-jsx-arrow-brackets: #333333; --light-color-jsx-arrow-brackets-inverted: rgba(255, 255, 255, 0.7); --light-color-modal-background: rgba(255, 255, 255, 0.75);