Skip to content

Commit

Permalink
Don't consumer iterators while inspecting (facebook#19831)
Browse files Browse the repository at this point in the history
Co-authored-by: Brian Vaughn <bvaughn@fb.com>
  • Loading branch information
2 people authored and koto committed Jun 15, 2021
1 parent cfbab86 commit 7c08c8e
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 10 additions & 0 deletions packages/react-devtools-shared/src/hydration.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
9 changes: 8 additions & 1 deletion packages/react-devtools-shared/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,7 @@ export type DataType =
| 'html_element'
| 'infinity'
| 'iterator'
| 'opaque_iterator'
| 'nan'
| 'null'
| 'number'
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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':
Expand Down

0 comments on commit 7c08c8e

Please sign in to comment.