Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: False Positive "plain object" Warning #7718

Closed
spudly opened this issue Sep 13, 2016 · 8 comments
Closed

Bug: False Positive "plain object" Warning #7718

spudly opened this issue Sep 13, 2016 · 8 comments

Comments

@spudly
Copy link
Contributor

spudly commented Sep 13, 2016

There is a bug in the logic for determining if props is a plain object. This bug results in a false positive, flooding the console with hundreds of instances of this warning: "Expected props argument to be a plain object. Properties defined in its prototype chain will be ignored.".

The code that's failing is in ReactElement.js, Line 210 in the v15.3.1 branch (line number is probably different in master)

config.proto == null || config.proto === Object.prototype,

This fails when the props object is created in one iframe, but uses an instance of React from another frame. Each browser window/tab/frame has it's own instance of the Object constructor. For instance, document.querySelector('iframe').contentWindow.Object === Object always results in false.

Check out this demo, which shows the false-positive warning message.

@spudly
Copy link
Contributor Author

spudly commented Sep 13, 2016

I just checked, and lodash's isPlainObject() function behaves as expected for objects created in different frames.

To fix this issue, we could either import and use lodash's function, or copy the isPlainObject() implementation into the React codebase.

@sophiebits
Copy link
Contributor

Ugh. I think we should just revert this warning. It's caused frustration in many cases.

@gaearon
Copy link
Collaborator

gaearon commented Sep 13, 2016

👍

@spudly
Copy link
Contributor Author

spudly commented Sep 13, 2016

Fine by me.

P.S. Sorry for the bizarre use case. I'm doing some pretty hacky stuff to isolate plugins inside an iframe.

@sophiebits
Copy link
Contributor

Okay, let's remove these lines and any associated tests:

https://github.com/facebook/react/blob/v15.3.1/src/isomorphic/classic/element/ReactElement.js#L207-L215

@aweary
Copy link
Contributor

aweary commented Sep 13, 2016

@spudly do you want to submit a PR for this? 😃

@spudly
Copy link
Contributor Author

spudly commented Sep 13, 2016

Sure, seems simple enough. I'll take care of it.

@spudly
Copy link
Contributor Author

spudly commented Sep 14, 2016

Submitted #7724

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants