-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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(build): inform rollup of react-popper exports #887
Conversation
@TheSharpieOne can you zip up the output bundles and attach those and I'll test the external mechanism as outlined in that other issue to make sure this works. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice workaround!
👍 will test later tonight (hopefully if time permits) |
Tested with the version you linked. There's still a build error when running Server Side rendering. Line 8:
It looks like this is a result of the ReactPortal as outlined in that other issue. The importing of the react-portal seems to maybe mess up a few things: OLD WORKING CODE:
NEW LINE:
Since ReactDOM is undefined on the server side you'd expect this to be undefined without erroring. Not sure how to get this back? The rendering of portals done in Side note: It looks like prop-types is now getting included from the react-portal package as well. Size is up from 164KB uncompressed to 178KB - beta.2/ or 180KB with this version. It shouldn't be hard to include the remove prop-types babel plugin as it's pretty safe. |
@virgofx so what is the fix for |
Let me look at it tomorrow. We could do that ... I'd like to know why it changed. Not sure if it's a rollup change , or because of the way we're using one of our dependencies. |
This is not longer needed. |
This brings react-popper back to ^0.8.2, but adds some rollup config to allow the build to deal with how react-popper is defining the exports.
CC @supergibbs