Don't consumer iterators while inspecting (#19831)
Co-authored-by: Brian Vaughn <bvaughn@fb.com>
Todor Totev committed
Sep 22, 2020 at 21:23 UTC
92c7e49895032885cffaad77a69d71268dda762e
6 files changed
+129
-1
packages/react-devtools-shared/src/__tests__/__snapshots__/inspectedElementContext-test.js.snap
+13
@@ -200,6 +200,19 @@ exports[`InspectedElementContext should inspect the currently selected element:
200
}
201
`;
202
203
+exports[`InspectedElementContext should not consume iterables while inspecting: 1: Inspected element 2 1`] = `
204
+{
205
+ "id": 2,
206
+ "owners": null,
207
+ "context": null,
208
+ "hooks": null,
209
+ "props": {
210
+ "prop": {}
211
+ },
212
+ "state": null
213
+}
214
+`;
215
+
216
exports[`InspectedElementContext should not dehydrate nested values until explicitly requested: 1: Initially inspect element 1`] = `
217
{
218
"id": 2,
packages/react-devtools-shared/src/__tests__/inspectedElementContext-test.js
+51
@@ -796,6 +796,57 @@ describe('InspectedElementContext', () => {
796
done();
797
});
798
799
+ it('should not consume iterables while inspecting', async done => {
800
+ const Example = () => null;
801
+
802
+ function* generator() {
803
+ throw Error('Should not be consumed!');
804
+ }
805
+
806
+ const container = document.createElement('div');
807
+
808
+ const iterable = generator();
809
+ await utils.actAsync(() =>
810
+ ReactDOM.render(<Example prop={iterable} />, container),
811
+ );
812
+
813
+ const id = ((store.getElementIDAtIndex(0): any): number);
814
+
815
+ let inspectedElement = null;
816
+
817
+ function Suspender({target}) {
818
+ const {getInspectedElement} = React.useContext(InspectedElementContext);
819
+ inspectedElement = getInspectedElement(id);
820
+ return null;
821
+ }
822
+
823
+ await utils.actAsync(
824
+ () =>
825
+ TestRenderer.create(
826
+ <Contexts
827
+ defaultSelectedElementID={id}
828
+ defaultSelectedElementIndex={0}>
829
+ <React.Suspense fallback={null}>
830
+ <Suspender target={id} />
831
+ </React.Suspense>
832
+ </Contexts>,
833
+ ),
834
+ false,
835
+ );
836
+
837
+ expect(inspectedElement).not.toBeNull();
838
+ expect(inspectedElement).toMatchSnapshot(`1: Inspected element ${id}`);
839
+
840
+ const {prop} = (inspectedElement: any).props;
841
+ expect(prop[meta.inspectable]).toBe(false);
842
+ expect(prop[meta.name]).toBe('Generator');
843
+ expect(prop[meta.type]).toBe('opaque_iterator');
844
+ expect(prop[meta.preview_long]).toBe('Generator');
845
+ expect(prop[meta.preview_short]).toBe('Generator');
846
+
847
+ done();
848
+ });
849
+
850
it('should support objects with no prototype', async done => {
851
const Example = () => null;
852
packages/react-devtools-shared/src/__tests__/legacy/__snapshots__/inspectElement-test.js.snap
+17
@@ -18,6 +18,23 @@ Object {
18
}
19
`;
20
21
+exports[`InspectedElementContext should not consume iterables while inspecting: 1: Initial inspection 1`] = `
22
+Object {
23
+ "id": 2,
24
+ "type": "full-data",
25
+ "value": {
26
+ "id": 2,
27
+ "owners": null,
28
+ "context": {},
29
+ "hooks": null,
30
+ "props": {
31
+ "iteratable": {}
32
+ },
33
+ "state": null
34
+},
35
+}
36
+`;
37
+
38
exports[`InspectedElementContext should not dehydrate nested values until explicitly requested: 1: Initially inspect element 1`] = `
39
Object {
40
"id": 2,
packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js
+30
@@ -397,6 +397,36 @@ describe('InspectedElementContext', () => {
397
done();
398
});
399
400
+ it('should not consume iterables while inspecting', async done => {
401
+ const Example = () => null;
402
+
403
+ function* generator() {
404
+ yield 1;
405
+ yield 2;
406
+ }
407
+
408
+ const iteratable = generator();
409
+
410
+ act(() =>
411
+ ReactDOM.render(
412
+ <Example iteratable={iteratable} />,
413
+ document.createElement('div'),
414
+ ),
415
+ );
416
+
417
+ const id = ((store.getElementIDAtIndex(0): any): number);
418
+ const inspectedElement = await read(id);
419
+
420
+ expect(inspectedElement).toMatchSnapshot('1: Initial inspection');
421
+
422
+ // Inspecting should not consume the iterable.
423
+ expect(iteratable.next().value).toEqual(1);
424
+ expect(iteratable.next().value).toEqual(2);
425
+ expect(iteratable.next().value).toBeUndefined();
426
+
427
+ done();
428
+ });
429
+
430
it('should support custom objects with enumerable properties and getters', async done => {
431
class CustomData {
432
_number = 42;
packages/react-devtools-shared/src/hydration.js
+10
@@ -266,6 +266,16 @@ export function dehydrate(
266
return unserializableValue;
267
}
268
269
+ case 'opaque_iterator':
270
+ cleaned.push(path);
271
+ return {
272
+ inspectable: false,
273
+ preview_short: formatDataForPreview(data, false),
274
+ preview_long: formatDataForPreview(data, true),
275
+ name: data[Symbol.toStringTag],
276
+ type,
277
+ };
278
+
279
case 'date':
280
cleaned.push(path);
281
return {
packages/react-devtools-shared/src/utils.js
+8
-1
@@ -440,6 +440,7 @@ export type DataType =
440
| 'html_element'
441
| 'infinity'
442
| 'iterator'
443
+ | 'opaque_iterator'
444
| 'nan'
445
| 'null'
446
| 'number'
@@ -500,7 +501,9 @@ export function getDataType(data: Object): DataType {
501
// but this seems kind of awkward and expensive.
502
return 'array_buffer';
503
} else if (typeof data[Symbol.iterator] === 'function') {
503
- return 'iterator';
504
+ return data[Symbol.iterator]() === data
505
+ ? 'opaque_iterator'
506
+ : 'iterator';
507
} else if (data.constructor && data.constructor.name === 'RegExp') {
508
return 'regexp';
509
} else {
@@ -679,6 +682,7 @@ export function formatDataForPreview(
682
}
683
case 'iterator':
684
const name = data.constructor.name;
685
+
686
if (showFormattedValue) {
687
// TRICKY
688
// Don't use [...spread] syntax for this purpose.
@@ -717,6 +721,9 @@ export function formatDataForPreview(
721
} else {
722
return `${name}(${data.size})`;
723
}
724
+ case 'opaque_iterator': {
725
+ return data[Symbol.toStringTag];
726
+ }
727
case 'date':
728
return data.toString();
729
case 'object':