From 92c7e49895032885cffaad77a69d71268dda762e Mon Sep 17 00:00:00 2001 From: Todor Totev <51530311+todortotev@users.noreply.github.com> Date: Tue, 22 Sep 2020 21:23:20 +0300 Subject: [PATCH] Don't consumer iterators while inspecting (#19831) Co-authored-by: Brian Vaughn --- .../inspectedElementContext-test.js.snap | 13 +++++ .../__tests__/inspectedElementContext-test.js | 51 +++++++++++++++++++ .../__snapshots__/inspectElement-test.js.snap | 17 +++++++ .../__tests__/legacy/inspectElement-test.js | 30 +++++++++++ .../react-devtools-shared/src/hydration.js | 10 ++++ packages/react-devtools-shared/src/utils.js | 9 +++- 6 files changed, 129 insertions(+), 1 deletion(-) diff --git a/packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElementContext-test.js.snap b/packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElementContext-test.js.snap index 9f216fd599438..8270d0c5b5f1d 100644 --- a/packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElementContext-test.js.snap +++ b/packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElementContext-test.js.snap @@ -200,6 +200,19 @@ exports[`InspectedElementContext should inspect the currently selected element: } `; +exports[`InspectedElementContext should not consume iterables while inspecting: 1: Inspected element 2 1`] = ` +{ + "id": 2, + "owners": null, + "context": null, + "hooks": null, + "props": { + "prop": {} + }, + "state": null +} +`; + exports[`InspectedElementContext should not dehydrate nested values until explicitly requested: 1: Initially inspect element 1`] = ` { "id": 2, diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js b/packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js index 29f03a9e5adcc..e43b925658f92 100644 --- a/packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js @@ -796,6 +796,57 @@ describe('InspectedElementContext', () => { done(); }); + it('should not consume iterables while inspecting', async done => { + const Example = () => null; + + function* generator() { + throw Error('Should not be consumed!'); + } + + const container = document.createElement('div'); + + const iterable = generator(); + await utils.actAsync(() => + ReactDOM.render(, container), + ); + + const id = ((store.getElementIDAtIndex(0): any): number); + + let inspectedElement = null; + + function Suspender({target}) { + const {getInspectedElement} = React.useContext(InspectedElementContext); + inspectedElement = getInspectedElement(id); + return null; + } + + await utils.actAsync( + () => + TestRenderer.create( + + + + + , + ), + false, + ); + + expect(inspectedElement).not.toBeNull(); + expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`); + + const {prop} = (inspectedElement: any).props; + expect(prop[meta.inspectable]).toBe(false); + expect(prop[meta.name]).toBe('Generator'); + expect(prop[meta.type]).toBe('opaque_iterator'); + expect(prop[meta.preview_long]).toBe('Generator'); + expect(prop[meta.preview_short]).toBe('Generator'); + + done(); + }); + it('should support objects with no prototype', async done => { const Example = () => null; diff --git a/packages/react-devtools-shared/src/__tests__/legacy/__snapshots__/inspectElement-test.js.snap b/packages/react-devtools-shared/src/__tests__/legacy/__snapshots__/inspectElement-test.js.snap index 11eb16474363a..1ed17c109bb12 100644 --- a/packages/react-devtools-shared/src/__tests__/legacy/__snapshots__/inspectElement-test.js.snap +++ b/packages/react-devtools-shared/src/__tests__/legacy/__snapshots__/inspectElement-test.js.snap @@ -18,6 +18,23 @@ Object { } `; +exports[`InspectedElementContext should not consume iterables while inspecting: 1: Initial inspection 1`] = ` +Object { + "id": 2, + "type": "full-data", + "value": { + "id": 2, + "owners": null, + "context": {}, + "hooks": null, + "props": { + "iteratable": {} + }, + "state": null +}, +} +`; + exports[`InspectedElementContext should not dehydrate nested values until explicitly requested: 1: Initially inspect element 1`] = ` Object { "id": 2, diff --git a/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js b/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js index 18a965b24b56e..d4f60af305cb0 100644 --- a/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js +++ b/packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js @@ -397,6 +397,36 @@ describe('InspectedElementContext', () => { done(); }); + it('should not consume iterables while inspecting', async done => { + const Example = () => null; + + function* generator() { + yield 1; + yield 2; + } + + const iteratable = generator(); + + act(() => + ReactDOM.render( + , + document.createElement('div'), + ), + ); + + const id = ((store.getElementIDAtIndex(0): any): number); + const inspectedElement = await read(id); + + expect(inspectedElement).toMatchSnapshot('1: Initial inspection'); + + // Inspecting should not consume the iterable. + expect(iteratable.next().value).toEqual(1); + expect(iteratable.next().value).toEqual(2); + expect(iteratable.next().value).toBeUndefined(); + + done(); + }); + it('should support custom objects with enumerable properties and getters', async done => { class CustomData { _number = 42; diff --git a/packages/react-devtools-shared/src/hydration.js b/packages/react-devtools-shared/src/hydration.js index 1f6c0d2af6efa..24379d675c4a9 100644 --- a/packages/react-devtools-shared/src/hydration.js +++ b/packages/react-devtools-shared/src/hydration.js @@ -266,6 +266,16 @@ export function dehydrate( return unserializableValue; } + case 'opaque_iterator': + cleaned.push(path); + return { + inspectable: false, + preview_short: formatDataForPreview(data, false), + preview_long: formatDataForPreview(data, true), + name: data[Symbol.toStringTag], + type, + }; + case 'date': cleaned.push(path); return { diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js index 088df58d6f7ec..2c796c2228b8d 100644 --- a/packages/react-devtools-shared/src/utils.js +++ b/packages/react-devtools-shared/src/utils.js @@ -440,6 +440,7 @@ export type DataType = | 'html_element' | 'infinity' | 'iterator' + | 'opaque_iterator' | 'nan' | 'null' | 'number' @@ -500,7 +501,9 @@ export function getDataType(data: Object): DataType { // but this seems kind of awkward and expensive. return 'array_buffer'; } else if (typeof data[Symbol.iterator] === 'function') { - return 'iterator'; + return data[Symbol.iterator]() === data + ? 'opaque_iterator' + : 'iterator'; } else if (data.constructor && data.constructor.name === 'RegExp') { return 'regexp'; } else { @@ -679,6 +682,7 @@ export function formatDataForPreview( } case 'iterator': const name = data.constructor.name; + if (showFormattedValue) { // TRICKY // Don't use [...spread] syntax for this purpose. @@ -717,6 +721,9 @@ export function formatDataForPreview( } else { return `${name}(${data.size})`; } + case 'opaque_iterator': { + return data[Symbol.toStringTag]; + } case 'date': return data.toString(); case 'object':