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

add es6 module build #72

Closed
wants to merge 1 commit into from
Closed

add es6 module build #72

wants to merge 1 commit into from

Conversation

modosc
Copy link
Contributor

@modosc modosc commented Nov 23, 2016

based on reduxjs/redux#1369

also i still think there should be a minor or major version bump for this (and the old versions deprecated) - we still have to setup special rules in babel to make it work which is bad for a patch release.

@yocontra
Copy link
Owner

yocontra commented Nov 24, 2016

Seems like this PR might be overkill, what's wrong with pointing "main" at dist/react-responsive.js and adding a "files": [ "dist" ] attribute?

@modosc
Copy link
Contributor Author

modosc commented Nov 24, 2016

sorry, i'm not following - that's already the case: https://github.com/contra/react-responsive/blob/master/package.json#L12

@yocontra
Copy link
Owner

yocontra commented Nov 24, 2016

Something must be weird with your webpack config because this works fine for me. Your original error was from a file in src but the module points to dist and nothing in dist points back to src.

Maybe your webpack config is picking up the jsnext:main declaration in the package.json?

@yocontra
Copy link
Owner

I just removed that declaration and switched it to only publish dist, so run npm update and let me know if the issue still happens

@modosc
Copy link
Contributor Author

modosc commented Nov 28, 2016

yeah you're totally right - we're separately importing a single file from the distribution in and that's what's causing the error. i'll deal with this separately.

i still think that this pr does the right thing for jsnext:main builds but i don't have a vested interest in it, let me pull down your newest version and see if webpack does the right thing now

@modosc
Copy link
Contributor Author

modosc commented Nov 28, 2016

you're right - we were also pulling in a single file and that's where that error came from.

i updated to the most recent release and webpack is happy - i think this solution is the correct one for publishing the es6 module version but i'm not invested in it since we've got a workaround so feel free to close out.

@yocontra yocontra closed this Nov 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants