Skip to content

Commit

Permalink
Perform hasOwnProperty check in Relay Flight (facebook#20220)
Browse files Browse the repository at this point in the history
We simulate JSON.stringify in this loop so we should do a has own check.
Otherwise we'll include things like constructor properties.

This will actually make things throw less even when it should.
  • Loading branch information
sebmarkbage authored and koto committed Jun 15, 2021
1 parent 37778b9 commit ccc7a84
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ export function processErrorChunk(
];
}

const hasOwnProperty = Object.prototype.hasOwnProperty;

function convertModelToJSON(
request: Request,
parent: {+[key: string]: ReactModel} | $ReadOnlyArray<ReactModel>,
Expand All @@ -88,12 +90,14 @@ function convertModelToJSON(
} else {
const jsonObj: {[key: string]: JSONValue} = {};
for (const nextKey in json) {
jsonObj[nextKey] = convertModelToJSON(
request,
json,
nextKey,
json[nextKey],
);
if (hasOwnProperty.call(json, nextKey)) {
jsonObj[nextKey] = convertModelToJSON(
request,
json,
nextKey,
json[nextKey],
);
}
}
return jsonObj;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,4 +214,23 @@ describe('ReactFlightDOMRelay', () => {
const model = readThrough(transport);
expect(model).toEqual(14);
});

it('should warn in DEV if a class instance polyfill is passed to a host component', () => {
function Bar() {}

function Foo() {}
Foo.prototype = Object.create(Bar.prototype);
// This is enumerable which some polyfills do.
Foo.prototype.constructor = Foo;
Foo.prototype.method = function() {};

expect(() => {
const transport = [];
ReactDOMFlightRelayServer.render(<input value={new Foo()} />, transport);
readThrough(transport);
}).toErrorDev(
'Only plain objects can be passed to client components from server components. ',
{withoutStack: true},
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ export function processErrorChunk(
];
}

const hasOwnProperty = Object.prototype.hasOwnProperty;

function convertModelToJSON(
request: Request,
parent: {+[key: string]: ReactModel} | $ReadOnlyArray<ReactModel>,
Expand All @@ -88,12 +90,14 @@ function convertModelToJSON(
} else {
const jsonObj: {[key: string]: JSONValue} = {};
for (const nextKey in json) {
jsonObj[nextKey] = convertModelToJSON(
request,
json,
nextKey,
json[nextKey],
);
if (hasOwnProperty.call(json, nextKey)) {
jsonObj[nextKey] = convertModelToJSON(
request,
json,
nextKey,
json[nextKey],
);
}
}
return jsonObj;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,26 @@ describe('ReactFlightNativeRelay', () => {
nativeFabricUIManager.__dumpHierarchyForJestTestsOnly(),
).toMatchSnapshot();
});

it('should warn in DEV if a class instance polyfill is passed to a host component', () => {
function Bar() {}

function Foo() {}
Foo.prototype = Object.create(Bar.prototype);
// This is enumerable which some polyfills do.
Foo.prototype.constructor = Foo;
Foo.prototype.method = function() {};

expect(() => {
const transport = [];
ReactNativeFlightRelayServer.render(
<input value={new Foo()} />,
transport,
);
readThrough(transport);
}).toErrorDev(
'Only plain objects can be passed to client components from server components. ',
{withoutStack: true},
);
});
});

0 comments on commit ccc7a84

Please sign in to comment.