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

Simplify environment injections #10175

Merged
merged 2 commits into from
Jul 13, 2017
Merged

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Jul 13, 2017

Not that it simplifies much.. But I just removed the function wrappers because they are no longer necessary. With flat bundles there’s no shared state so this is safe.

gaearon added 2 commits July 13, 2017 19:46
It was necessary when they shared global state. But now these are flat bundles so we can inject as side effect.
This feels a bit less convoluted to me.

Ideally we can get rid of this somehow soon.
@@ -23,40 +23,24 @@ var ReactDOMEventListener = require('ReactDOMEventListener');
var SelectEventPlugin = require('SelectEventPlugin');
var SimpleEventPlugin = require('SimpleEventPlugin');

var alreadyInjected = false;
ReactDOMEventListener.setHandleTopLevel(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just putting method content outside. Variable is no longer needed.

@gaearon gaearon requested review from sophiebits and flarnie July 13, 2017 18:55
Copy link
Collaborator

@sophiebits sophiebits left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is ok. At some point we were trying to get away from require-time side effects because they're incompatible with inline requires but I don't feel strongly.

@gaearon gaearon merged commit 2dcdc3c into facebook:master Jul 13, 2017
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.

3 participants