Don't consumer iterators while inspecting (facebook#19831)

todortotev · Brian Vaughn · web-flow · commit 92c7e4989503 · 2020-09-22T14:23:20.000-04:00
Co-authored-by: Brian Vaughn &lt;bvaughn@fb.com&gt;
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
@@ -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
@@ -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(<Example prop={iterable} />, 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(
+          <Contexts
+            defaultSelectedElementID={id}
+            defaultSelectedElementIndex={0}>
+            <React.Suspense fallback={null}>
+              <Suspender target={id} />
+            </React.Suspense>
+          </Contexts>,
+        ),
+      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
@@ -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
@@ -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(
+        <Example iteratable={iteratable} />,
+        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
@@ -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
@@ -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':
