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

Fix issue with SyntheticEvents and Proxies #6189

Merged
merged 1 commit into from
Mar 4, 2016

Conversation

zpao
Copy link
Member

@zpao zpao commented Mar 4, 2016

Fixes #6187 (hacky but it seems to work)

@zpao
Copy link
Member Author

zpao commented Mar 4, 2016

Not quite, I missed another piece of this...

@sebmarkbage
Copy link
Collaborator

I suppose the subclasses should use Object.assign to inherit the static properties like they would on a native class.

@zpao
Copy link
Member Author

zpao commented Mar 4, 2016

The subclasses don't seem to be the problem though, it's the base class.

@sebmarkbage
Copy link
Collaborator

I think this might be setting up the wrong prototype:

return this.apply(target, {}, args);

Is that right? I think the second argument there should be extending the prototype of SyntheticEvent which has the right .constructor on it.

@sebmarkbage
Copy link
Collaborator

Or maybe not. That whole thing is so confusing. Create an issue to rewrite it and drop all the pooling shit?

@zpao
Copy link
Member Author

zpao commented Mar 4, 2016

Object.create to the rescue. I think it should be safe to use blindly there since we're already guarded to engines that support Proxies.

@facebook-github-bot
Copy link

@zpao updated the pull request.

@zpao zpao changed the title Handle the undefined Interface in the proxy Fix issue with SyntheticEvents and Proxies Mar 4, 2016
zpao added a commit that referenced this pull request Mar 4, 2016
Fix issue with SyntheticEvents and Proxies
@zpao zpao merged commit 8b22a82 into facebook:master Mar 4, 2016
@koba04
Copy link
Contributor

koba04 commented Mar 5, 2016

Sorry, It's my fault.
Thank you for fixing it!

This warning is also possible to detect adding properties in destructor instead of using Proxies.
The reason using Proxies is for making stack trace clearly.

#5947 (comment)

Or maybe not. That whole thing is so confusing. Create an issue to rewrite it and drop all the pooling shit?

I agree.
I'd like to consider and try refactoring SyntheticEvent.

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

Successfully merging this pull request may close these issues.

4 participants