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 for #204 #401

Merged
merged 1 commit into from
Dec 16, 2016
Merged

Fix for #204 #401

merged 1 commit into from
Dec 16, 2016

Conversation

arunoda
Copy link
Contributor

@arunoda arunoda commented Dec 16, 2016

Fixes #204

React addons require React in a special way.
That causes Webpack to push React into the app's bundle.
This fix adds new externals entries to prevent that.

React addons require React in a special way.
That causes Webpack to push React into the app's bundle.
This fix adds new externals entries to prevent that.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 34.211% when pulling d05909a on arunoda:fix-204 into 45e36fd on zeit:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 34.211% when pulling d05909a on arunoda:fix-204 into 45e36fd on zeit:master.

@rauchg
Copy link
Member

rauchg commented Dec 16, 2016

Beautiful. Yet, pretty strange that react addons require react like that? What's the rationale?

@arunoda
Copy link
Contributor Author

arunoda commented Dec 16, 2016

Not exactly sure. But that might be the case that, FB uses their own module resolver for React.

@rauchg
Copy link
Member

rauchg commented Dec 16, 2016

@gaearon what's the rational behind the ./React thing?

@arunoda
Copy link
Contributor Author

arunoda commented Dec 16, 2016

May be related. See: facebook/react#6343

@gaearon
Copy link
Contributor

gaearon commented Dec 16, 2016

what's the rational behind the ./React thing?

It's just importing React entry point from the same folder. react/lib/TransitionGroup imports react/lib/React.

@arunoda
Copy link
Contributor Author

arunoda commented Dec 16, 2016

Thanks @gaearon
Oh I forgot to see other files :D

@frol
Copy link
Contributor

frol commented Dec 16, 2016

@arunoda Are you sure that this fix is correct?

Yes, these modules only work when packaged with the react module from npm (it does this because we need access to private APIs). If you want to use the prepackaged React and an addon, then we recommend use ReactWithAddons and map react-addons-css-transition-group to React.addons.CSSTransitionGroup.

(c) facebook/react#6343

I am not a JS expert, so I am not exactly sure, but it feels like this patch will just break the addons.

@arunoda
Copy link
Contributor Author

arunoda commented Dec 17, 2016

@frol it works. (and it should be).
It maps the internal .React with the one we've in the bundle.
Anyway, we are getting rid of the next-bundle soon. (#406)
With that there'll be no problem like this.

@arunoda arunoda deleted the fix-204 branch December 17, 2016 04:18
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants