@samitouri / QOS-React-2 / commits / a56fd4c36d

Tidied up. Added comments. Renamed a few things.

Brian Vaughn committed Jun 18, 2019 at 08:40 UTC a56fd4c36dd7cbfca5a02fbe0389c3c7ff9f0100

9 files changed +99 -66

src/__tests__/__snapshots__/inspectedElementContext-test.js.snap

@@ -369,6 +369,7 @@ exports[`InspectedElementContext should not tear if hydration is requested after

369 "hooks": null,

370 "props": {

371 "nestedObject": {

372 + "value": 1,

373 "a": {}

374 }

375 },

@@ -385,6 +386,7 @@ exports[`InspectedElementContext should not tear if hydration is requested after

386 "hooks": null,

387 "props": {

388 "nestedObject": {

389 + "value": 2,

390 "a": {

391 "value": 2,

392 "b": {

src/__tests__/inspectedElementContext-test.js

+41 -31

@@ -1,7 +1,7 @@

1 // @flow

3 import typeof ReactTestRenderer from 'react-test-renderer';

4 -import type { GetPath } from 'src/devtools/views/Components/InspectedElementContext';

4 +import type { GetInspectedElementPath } from 'src/devtools/views/Components/InspectedElementContext';

5 import type Bridge from 'src/bridge';

6 import type Store from 'src/devtools/store';

@@ -81,8 +81,8 @@ describe('InspectedElementContext', () => {

81 let didFinish = false;

83 function Suspender({ target }) {

84 - const { read } = React.useContext(InspectedElementContext);

85 - const inspectedElement = read(id);

84 + const { getInspectedElement } = React.useContext(InspectedElementContext);

85 + const inspectedElement = getInspectedElement(id);

86 expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`);

87 didFinish = true;

88 return null;

@@ -121,8 +121,8 @@ describe('InspectedElementContext', () => {

121 let inspectedElement = null;

122

123 function Suspender({ target }) {

124 - const { read } = React.useContext(InspectedElementContext);

125 - inspectedElement = read(id);

124 + const { getInspectedElement } = React.useContext(InspectedElementContext);

125 + inspectedElement = getInspectedElement(id);

126 return null;

127 }

128

@@ -189,8 +189,8 @@ describe('InspectedElementContext', () => {

189 let inspectedElement = null;

190

191 function Suspender({ target }) {

192 - const { read } = React.useContext(InspectedElementContext);

193 - inspectedElement = read(target);

192 + const { getInspectedElement } = React.useContext(InspectedElementContext);

193 + inspectedElement = getInspectedElement(target);

194 return null;

195 }

196

@@ -283,8 +283,8 @@ describe('InspectedElementContext', () => {

283 let inspectedElement = null;

284

285 function Suspender({ target }) {

286 - const { read } = React.useContext(InspectedElementContext);

287 - inspectedElement = read(id);

286 + const { getInspectedElement } = React.useContext(InspectedElementContext);

287 + inspectedElement = getInspectedElement(id);

288 return null;

289 }

290

@@ -371,8 +371,8 @@ describe('InspectedElementContext', () => {

371 let didFinish = false;

372

373 function Suspender({ target }) {

374 - const { read } = React.useContext(InspectedElementContext);

375 - const inspectedElement = read(id);

374 + const { getInspectedElement } = React.useContext(InspectedElementContext);

375 + const inspectedElement = getInspectedElement(id);

376 expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`);

377 didFinish = true;

378 return null;

@@ -434,13 +434,13 @@ describe('InspectedElementContext', () => {

434

435 const id = ((store.getElementIDAtIndex(0): any): number);

436

437 - let getPath: GetPath = ((null: any): GetPath);

437 + let getInspectedElementPath: GetInspectedElementPath = ((null: any): GetInspectedElementPath);

438 let inspectedElement = null;

439

440 function Suspender({ target }) {

441 const context = React.useContext(InspectedElementContext);

442 - getPath = context.getPath;

443 - inspectedElement = context.read(target);

442 + getInspectedElementPath = context.getInspectedElementPath;

443 + inspectedElement = context.getInspectedElement(target);

444 return null;

445 }

446

@@ -458,13 +458,13 @@ describe('InspectedElementContext', () => {

458 ),

459 false

460 );

461 - expect(getPath).not.toBeNull();

461 + expect(getInspectedElementPath).not.toBeNull();

462 expect(inspectedElement).not.toBeNull();

463 expect(inspectedElement).toMatchSnapshot('1: Initially inspect element');

464

465 inspectedElement = null;

466 TestUtils.act(() => {

467 - getPath(id, ['props', 'nestedObject', 'a']);

467 + getInspectedElementPath(id, ['props', 'nestedObject', 'a']);

468 jest.runOnlyPendingTimers();

469 });

470 expect(inspectedElement).not.toBeNull();

@@ -472,7 +472,7 @@ describe('InspectedElementContext', () => {

472

473 inspectedElement = null;

474 TestUtils.act(() => {

475 - getPath(id, ['props', 'nestedObject', 'a', 'b', 'c']);

475 + getInspectedElementPath(id, ['props', 'nestedObject', 'a', 'b', 'c']);

476 jest.runOnlyPendingTimers();

477 });

478 expect(inspectedElement).not.toBeNull();

@@ -482,7 +482,15 @@ describe('InspectedElementContext', () => {

482

483 inspectedElement = null;

484 TestUtils.act(() => {

485 - getPath(id, ['props', 'nestedObject', 'a', 'b', 'c', 0, 'd']);

485 + getInspectedElementPath(id, [

486 + 'props',

487 + 'nestedObject',

488 + 'a',

489 + 'b',

490 + 'c',

491 + 0,

492 + 'd',

493 + ]);

494 jest.runOnlyPendingTimers();

495 });

496 expect(inspectedElement).not.toBeNull();

@@ -492,7 +500,7 @@ describe('InspectedElementContext', () => {

500

501 inspectedElement = null;

502 TestUtils.act(() => {

495 - getPath(id, ['hooks', 0, 'value']);

503 + getInspectedElementPath(id, ['hooks', 0, 'value']);

504 jest.runOnlyPendingTimers();

505 });

506 expect(inspectedElement).not.toBeNull();

@@ -500,7 +508,7 @@ describe('InspectedElementContext', () => {

508

509 inspectedElement = null;

510 TestUtils.act(() => {

503 - getPath(id, ['hooks', 0, 'value', 'foo', 'bar']);

511 + getInspectedElementPath(id, ['hooks', 0, 'value', 'foo', 'bar']);

512 jest.runOnlyPendingTimers();

513 });

514 expect(inspectedElement).not.toBeNull();

@@ -542,13 +550,13 @@ describe('InspectedElementContext', () => {

550

551 const id = ((store.getElementIDAtIndex(0): any): number);

552

545 - let getPath: GetPath = ((null: any): GetPath);

553 + let getInspectedElementPath: GetInspectedElementPath = ((null: any): GetInspectedElementPath);

554 let inspectedElement = null;

555

556 function Suspender({ target }) {

557 const context = React.useContext(InspectedElementContext);

550 - getPath = context.getPath;

551 - inspectedElement = context.read(id);

558 + getInspectedElementPath = context.getInspectedElementPath;

559 + inspectedElement = context.getInspectedElement(id);

560 return null;

561 }

562

@@ -566,13 +574,13 @@ describe('InspectedElementContext', () => {

574 ),

575 false

576 );

569 - expect(getPath).not.toBeNull();

577 + expect(getInspectedElementPath).not.toBeNull();

578 expect(inspectedElement).not.toBeNull();

579 expect(inspectedElement).toMatchSnapshot('1: Initially inspect element');

580

581 inspectedElement = null;

582 TestUtils.act(() => {

575 - getPath(id, ['props', 'nestedObject', 'a']);

583 + getInspectedElementPath(id, ['props', 'nestedObject', 'a']);

584 jest.runOnlyPendingTimers();

585 });

586 expect(inspectedElement).not.toBeNull();

@@ -580,7 +588,7 @@ describe('InspectedElementContext', () => {

588

589 inspectedElement = null;

590 TestUtils.act(() => {

583 - getPath(id, ['props', 'nestedObject', 'c']);

591 + getInspectedElementPath(id, ['props', 'nestedObject', 'c']);

592 jest.runOnlyPendingTimers();

593 });

594 expect(inspectedElement).not.toBeNull();

@@ -629,6 +637,7 @@ describe('InspectedElementContext', () => {

637 ReactDOM.render(

638 <Example

639 nestedObject={{

640 + value: 1,

641 a: {

642 value: 1,

643 b: {

@@ -643,13 +652,13 @@ describe('InspectedElementContext', () => {

652

653 const id = ((store.getElementIDAtIndex(0): any): number);

654

646 - let getPath: GetPath = ((null: any): GetPath);

655 + let getInspectedElementPath: GetInspectedElementPath = ((null: any): GetInspectedElementPath);

656 let inspectedElement = null;

657

658 function Suspender({ target }) {

659 const context = React.useContext(InspectedElementContext);

651 - getPath = context.getPath;

652 - inspectedElement = context.read(id);

660 + getInspectedElementPath = context.getInspectedElementPath;

661 + inspectedElement = context.getInspectedElement(id);

662 return null;

663 }

664

@@ -667,7 +676,7 @@ describe('InspectedElementContext', () => {

676 ),

677 false

678 );

670 - expect(getPath).not.toBeNull();

679 + expect(getInspectedElementPath).not.toBeNull();

680 expect(inspectedElement).not.toBeNull();

681 expect(inspectedElement).toMatchSnapshot('1: Initially inspect element');

682

@@ -675,6 +684,7 @@ describe('InspectedElementContext', () => {

684 ReactDOM.render(

685 <Example

686 nestedObject={{

687 + value: 2,

688 a: {

689 value: 2,

690 b: {

@@ -689,7 +699,7 @@ describe('InspectedElementContext', () => {

699

700 inspectedElement = null;

701 TestUtils.act(() => {

692 - getPath(id, ['props', 'nestedObject', 'a']);

702 + getInspectedElementPath(id, ['props', 'nestedObject', 'a']);

703 jest.runOnlyPendingTimers();

704 });

705 expect(inspectedElement).not.toBeNull();

src/backend/legacy/renderer.js

@@ -552,6 +552,8 @@ export function attach(

552 let currentlyInspectedElementID: number | null = null;

553 let currentlyInspectedPaths: Object = {};

554

555 + // Track the intersection of currently inspected paths,

556 + // so that we can send their data along if the element is re-rendered.

557 function mergeInspectedPaths(path: Array<string | number>) {

558 let current = currentlyInspectedPaths;

559 path.forEach(key => {

@@ -563,6 +565,8 @@ export function attach(

565 }

566

567 function createIsPathWhitelisted(key: string) {

568 + // This function helps prevent previously-inspected paths from being dehydrated in updates.

569 + // This is important to avoid a bad user experience where expanded toggles collapse on update.

570 return function isPathWhitelisted(path: Array<string | number>): boolean {

571 let current = currentlyInspectedPaths[key];

572 if (!current) {

src/backend/renderer.js

+9 -2

@@ -2137,6 +2137,8 @@ export function attach(

2137 );

2138 }

2139

2140 + // Track the intersection of currently inspected paths,

2141 + // so that we can send their data along if the element is re-rendered.

2142 function mergeInspectedPaths(path: Array<string | number>) {

2143 let current = currentlyInspectedPaths;

2144 path.forEach(key => {

@@ -2148,9 +2150,13 @@ export function attach(

2150 }

2151

2152 function createIsPathWhitelisted(isHooksPath: boolean, key: string | null) {

2153 + // This function helps prevent previously-inspected paths from being dehydrated in updates.

2154 + // This is important to avoid a bad user experience where expanded toggles collapse on update.

2155 return function isPathWhitelisted(path: Array<string | number>): boolean {

2156 // Dehydrating the 'subHooks' property makes the HooksTree UI a lot more complicated,

2157 // so it's easiest for now if we just don't break on this boundary.

2158 + // We can always dehydrate a level deeper (in the value object).

2159 + // TODO (hydration) This check depends on a LEVEL_THRESHOLD of 2 to avoid dehydrating a hook incorrectly.

2160 if (isHooksPath && path[path.length - 1] === 'subHooks') {

2161 return true;

2162 }

@@ -2225,8 +2231,10 @@ export function attach(

2231 mergeInspectedPaths(path);

2232 }

2233

2234 + // Clone before cleaning so that we preserve the full data.

2235 + // This will enable us to send patches without re-inspecting if hydrated paths are requested.

2236 + // (Reducing how often we shallow-render is a better DX for function components that use hooks.)

2237 const cleanedInspectedElement = { ...mostRecentlyInspectedElement };

2229 -

2238 cleanedInspectedElement.context = cleanForBridge(

2239 cleanedInspectedElement.context,

2240 createIsPathWhitelisted(false, 'context')

@@ -2308,7 +2316,6 @@ export function attach(

2316 const fiber = findCurrentFiberUsingSlowPathById(id);

2317 if (fiber !== null) {

2318 if (typeof overrideHookState === 'function') {

2311 - console.log('[renderer] overrideHookState()', { path, value, index });

2319 overrideHookState(fiber, index, path, value);

2320 }

2321 }

src/devtools/views/Components/HooksTree.js

+10 -8

@@ -23,12 +23,12 @@ type HooksTreeViewProps = {|

23 |};

25 export function HooksTreeView({ canEditHooks, hooks, id }: HooksTreeViewProps) {

26 - const { getPath } = useContext(InspectedElementContext);

26 + const { getInspectedElementPath } = useContext(InspectedElementContext);

27 const inspectPath = useCallback(

28 (path: Array<string | number>) => {

29 - getPath(id, ['hooks', ...path]);

29 + getInspectedElementPath(id, ['hooks', ...path]);

30 },

31 - [getPath, id]

31 + [getInspectedElementPath, id]

32 );

33 const handleCopy = useCallback(() => copy(serializeHooksForCopy(hooks)), [

34 hooks,

@@ -114,7 +114,9 @@ function HookView({

114

115 if (hook.hasOwnProperty(meta.inspected)) {

116 // This Hook is too deep and hasn't been hydrated.

117 - // TODO (hydration) show UI to load its data.

117 + if (__DEV__) {

118 + console.warn('Unexpected dehydrated hook; this is a DevTools error.');

119 + }

120 return (

121 <div className={styles.Hook}>

122 <div className={styles.NameValueRow}>

@@ -124,8 +126,6 @@ function HookView({

126 );

127 }

128

127 - // TODO Add click and key handlers for toggling element open/close state.

128 -

129 const isCustomHook = subHooks.length > 0;

130

131 const type = typeof value;

@@ -218,8 +218,10 @@ function HookView({

218 bridge.send('overrideHookState', {

219 id,

220 hookID,

221 - // Hooks override function expects a relative path for the specified hook (id).

222 - // This should not include the fake tree structure DevTools uses for display.

221 + // Hooks override function expects a relative path for the specified hook (id),

222 + // starting with its id within the (flat) hooks list structure.

223 + // This relative path does not include the fake tree structure DevTools uses for display,

224 + // so it's important that we remove that part of the path before sending the update.

225 path: absolutePath.slice(path.length + 1),

226 rendererID,

227 value,

src/devtools/views/Components/InspectedElementContext.js

+16 -8

@@ -26,12 +26,17 @@ import type {

26 } from 'src/devtools/views/Components/types';

27 import type { Resource, Thenable } from '../../cache';

29 -export type GetPath = (id: number, path: Array<string | number>) => void;

30 -export type Read = (id: number) => InspectedElementFrontend | null;

29 +export type GetInspectedElementPath = (

30 + id: number,

31 + path: Array<string | number>

32 +) => void;

33 +export type GetInspectedElement = (

34 + id: number

35 +) => InspectedElementFrontend | null;

37 type Context = {|

33 - getPath: GetPath,

34 - read: Read,

38 + getInspectedElementPath: GetInspectedElementPath,

39 + getInspectedElement: GetInspectedElement,

40 |};

42 const InspectedElementContext = createContext<Context>(((null: any): Context));

@@ -76,7 +81,8 @@ function InspectedElementContextController({ children }: Props) {

81 const bridge = useContext(BridgeContext);

82 const store = useContext(StoreContext);

79 - const getPath = useCallback<GetPath>(

84 + // Ask the backend to fill in a "dehydrated" path; this will result in a "inspectedElement".

85 + const getInspectedElementPath = useCallback<GetInspectedElementPath>(

86 (id: number, path: Array<string | number>) => {

87 const rendererID = store.getRendererIDForElement(id);

88 bridge.send('inspectElement', { id, path, rendererID });

@@ -84,7 +90,7 @@ function InspectedElementContextController({ children }: Props) {

90 [bridge, store]

91 );

87 - const read = useCallback<Read>(

93 + const getInspectedElement = useCallback<GetInspectedElement>(

94 (id: number) => {

95 const element = store.getElementByID(id);

96 if (element !== null) {

@@ -266,10 +272,10 @@ function InspectedElementContextController({ children }: Props) {

272 }, [bridge, selectedElementID, store]);

273

274 const value = useMemo(

269 - () => ({ getPath, read }),

275 + () => ({ getInspectedElement, getInspectedElementPath }),

276 // InspectedElement is used to invalidate the cache and schedule an update with React.

277 // eslint-disable-next-line react-hooks/exhaustive-deps

272 - [currentlyInspectedElement, getPath, read]

278 + [currentlyInspectedElement, getInspectedElement, getInspectedElementPath]

279 );

280

281 return (

@@ -289,6 +295,8 @@ function hydrateHelper(

295 if (path) {

296 const { length } = path;

297 if (length > 0) {

298 + // Hydration helper requires full paths, but inspection dehydrates with relative paths.

299 + // In that event it's important that we adjust the "cleaned" paths to match.

300 cleaned = cleaned.map(cleanedPath => cleanedPath.slice(length));

301 }

302 }

src/devtools/views/Components/KeyValue.js

-3

@@ -22,9 +22,6 @@ type KeyValueProps = {|

22 value: any,

23 |};

25 -// TODO (hydration) Don't display meta objects.

26 -// Add event listener to request a "read" instead.

27 -

25 export default function KeyValue({

26 depth,

27 inspectPath,

src/devtools/views/Components/SelectedElement.js

+15 -13

@@ -25,7 +25,7 @@ import {

26 import styles from './SelectedElement.css';

28 -import type { GetPath } from './InspectedElementContext';

28 +import type { GetInspectedElementPath } from './InspectedElementContext';

29 import type { Element, InspectedElement } from './types';

30 import type { ElementType } from 'src/types';

@@ -39,7 +39,9 @@ export default function SelectedElement(_: Props) {

39 const store = useContext(StoreContext);

40 const { dispatch: modalDialogDispatch } = useContext(ModalDialogContext);

42 - const { getPath, read } = useContext(InspectedElementContext);

42 + const { getInspectedElementPath, getInspectedElement } = useContext(

43 + InspectedElementContext

44 + );

46 const element =

47 inspectedElementID !== null

@@ -47,7 +49,7 @@ export default function SelectedElement(_: Props) {

49 : null;

51 const inspectedElement =

50 - inspectedElementID != null ? read(inspectedElementID) : null;

52 + inspectedElementID != null ? getInspectedElement(inspectedElementID) : null;

54 const highlightElement = useCallback(() => {

55 if (element !== null && inspectedElementID !== null) {

@@ -202,10 +204,10 @@ export default function SelectedElement(_: Props) {

204 {inspectedElement !== null && (

205 <InspectedElementView

206 key={

205 - inspectedElementID /* Ensure state resets between seleted Elements */

207 + inspectedElementID /* Force reset when seleted Element changes */

208 }

209 element={element}

208 - getPath={getPath}

210 + getInspectedElementPath={getInspectedElementPath}

211 inspectedElement={inspectedElement}

212 />

213 )}

@@ -217,7 +219,7 @@ export type InspectPath = (path: Array<string | number>) => void;

219

220 type InspectedElementViewProps = {|

221 element: Element,

220 - getPath: GetPath,

222 + getInspectedElementPath: GetInspectedElementPath,

223 inspectedElement: InspectedElement,

224 |};

225

@@ -225,7 +227,7 @@ const IS_SUSPENDED = 'Suspended';

227

228 function InspectedElementView({

229 element,

228 - getPath,

230 + getInspectedElementPath,

231 inspectedElement,

232 }: InspectedElementViewProps) {

233 const { id, type } = element;

@@ -247,21 +249,21 @@ function InspectedElementView({

249

250 const inspectContextPath = useCallback(

251 (path: Array<string | number>) => {

250 - getPath(id, ['context', ...path]);

252 + getInspectedElementPath(id, ['context', ...path]);

253 },

252 - [getPath, id]

254 + [getInspectedElementPath, id]

255 );

256 const inspectPropsPath = useCallback(

257 (path: Array<string | number>) => {

256 - getPath(id, ['props', ...path]);

258 + getInspectedElementPath(id, ['props', ...path]);

259 },

258 - [getPath, id]

260 + [getInspectedElementPath, id]

261 );

262 const inspectStatePath = useCallback(

263 (path: Array<string | number>) => {

262 - getPath(id, ['state', ...path]);

264 + getInspectedElementPath(id, ['state', ...path]);

265 },

264 - [getPath, id]

266 + [getInspectedElementPath, id]

267 );

268

269 let overrideContextFn = null;

src/hydration.js

+2 -1

@@ -42,7 +42,8 @@ type Dehydrated = {|

42 // Reducing this threshold will improve the speed of initial component inspection,

43 // but may decrease the responsiveness of expanding objects/arrays to inspect further.

44 //

45 -// Note that reducing the threshold to below two effectively breaks the inspected hooks interface.

45 +// Note that reducing the threshold to below 2 effectively breaks the inspected hooks interface.

46 +// It is only safe to dehydrate hooks within the "value" key, never within the "subHooks" array directly.

47 const LEVEL_THRESHOLD = 2;

49 /**