Skip to content
This repository was archived by the owner on Oct 26, 2018. It is now read-only.

Conversation

@uniqname
Copy link

@uniqname uniqname commented Dec 3, 2015

I could have just confused myself, but I believe Babel 6 changed the way cjs files are resolved with named import statements in es6. This change meant that cjs files that assigned objects to module.exports cannot be destructured within the import statement and results in a Module <whatever> does not export <named-export> error. The solution, it appears, is to assign values to properties on the exports object rather than a single object assigned to module.exports. Again, I may wrong on what exactly is happening, but it solved the problem for me and all tests pass. Feel free to reject with prejudice if I'm out of my mind. Thanks for the work!

@kimjoar
Copy link
Collaborator

kimjoar commented Dec 4, 2015

Hm, strange. Everything appears to work as expected on my end (with import statements and Babel 6.2.4). Not sure what the problem can be, though. I don't think this should be necessary to get import to work. Could you double-check that your on the latest Babel?

@uniqname
Copy link
Author

uniqname commented Dec 4, 2015

Ah ha! I've figured it out. It's not babel that was giving the problem. It's the bundler, rollup. The problem does not manifest using browserify, either with the current release or the pull request (haven't check webpack). This pull request solves the Module [...]/node_modules/redux-simple-router/lib/index.js does not export routeReducer (imported by [...]) error with rollup without any apparent repercussions in other scenarios. If you feel this is a bug with the way rollup works, I can file it with them. The intricacies of commonjs and module.exports vs exports are not clear enough for me to make a good argument either way.

@jlongster
Copy link
Member

I feel like that is a bug with rollup, sorry. I prefer this syntax. I would be interested in a link to a discussion over there as to why they don't interop well with this common scenario.

At some point we are probably going to switch to ES6 imports anyway. In fact, I'd rather see a PR that does that instead. Would you be interested in doing that? I bet that solves the problem.

@jlongster jlongster closed this Dec 6, 2015
@uniqname
Copy link
Author

uniqname commented Dec 6, 2015

Actually, a port to ES6 sounds great. I'll work on that.

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.

3 participants