Rollup + Tree shaking throw an error

[TylorS]

Hello! First I'd just like to say thanks for maintaining this library! Very happy to use it for most.js

I spent some time experimenting with converting most.js' core codebase to ES2015 imports/exports to take advantage of tree-shaking for third-party libraries and applications that use it (PR here if interested). When I use rollup with a plugin to use node's import resolution (so it can find imports in node_modules) it has an option to use a packages jsnext:main as I'm sure you already know. However I have to disable it in our current configuration to allow things to build. I receive the following error

Error parsing /home/tylor/code/most/most/node_modules/symbol-observable/es/index.js: 'import' and 'export' may only appear at the top level (5:0) in /home/tylor/code/most/most/node_modules/symbol-observable/es/index.js

Here is the full error for sake of thoroughness
https://gist.github.com/TylorS/097632154b662778068ff8dc7c9852c7

This is not urgent for me as our build still currently requires using rollup's commonjs plugin and happily consumes this library, but that might not be the case for very long. I just wanted to let you know if this is in fact an issue with how things are exported or possibly you or someone else already knows the solution that I'm missing.

Thank you for your time :)

[benlesh]

Thanks @TylorS! I don't have a lot of experience running Rollup. Perhaps @robwormald could chime in here? Rob, have you hit this issue before?

[TylorS]

Hey @Blesh thank you for the response. I found the solution today. The issue was the combination of plugins I was using. We are currently using the buble, node-resolve, and commonjs rollup plugins.

The commonjs plugin wraps all the packages it finds from your node_modules in a wrapper to be compatible with the rest of the the rollup build, but does not expect for the package it is resolving to be using ES6 imports/exports. When buble comes around and sees the import/exports it throws the error from above.

The solution was ignoring symbol-observable from the commonjs plugin as seen here

Thanks again for this library.

[benlesh]

I see. Thank you for providing feedback on your findings. Hopefully this issue will help someone else in a similar situation.

[talmobi]

I have the same issues since Redux@3.6.0 updated its symbol-observable dependency. Your fix works @TylorS. So the issue is that rollup-plugin-commonjs wraps an already es6 module with import/exports into an es6 module with extra imports/exports?

Shouldn't the rollup-plugin-commonjs be able to detect this, you think?

[talmobi]

Well, I se now that a few issues and even a PR fix by @Rich-Harris himself has already been made at rollup-plugin-commonjs github regarding this issue:

rollup/rollup-plugin-commonjs#96