search

rollup / rollup-plugin-commonjs

This module has moved and is now available at @rollup/plugin-commonjs / https://github.com/rollup/plugins/blob/master/packages/commonjs
MIT License
502 stars 126 forks source link

Only able to import default from external deps #410

Closed devinivy closed 4 years ago

devinivy commented 5 years ago

It seems that references to external deps are always transformed such that their default export is imported. This can be seen here: https://github.com/rollup/rollup-plugin-commonjs/blob/de56bbc75490c5b7cdebe2daf2299e1f5809f33d/src/index.js#L129-L135

In my case I am writing a module in CJS that has a peer dependency on normalizr (so, normalizr is not part of my bundle, but I do require() it). Normalizr does not have a default export, but the rollup output invariably imports normalizr's default, which results in builds that fail at runtime.

I have attempted to adjust the way I assign variables to require('normalizr') and to set namedExports for normalizr, but it seems clear to me after reading through this library's codebase that there's no way to configure this behavior at the moment. I wonder if it would be more flexible to import the entire module (import * as normalizr ...) rather than the default (import normalizr ...).

For reference, here's my rollup configuration: https://github.com/BigRoomStudios/strange-middle-end/blob/master/rollup.config.js#L11-L34 And here's the build output: https://unpkg.com/browse/strange-middle-end@0.1.0/dist/strange-middle-end.module.js

Line 2 shows how normalizr is imported, and line 155 is the one that fails due to normalizr being undefined (again, since the default export is being imported, yet the library does not have a default export).

TrySound commented 5 years ago

Why not just use esm instead of commonjs? This plugin only provides interop between esm and cjs. It's not very correct as transpilation tool.

devinivy commented 5 years ago

I'm consciously using cjs rather than esm so that the library can run in nodejs without transpilation. It adds consistency to a broader ecosystem of software that this module participates in. I'm using rollup with this plugin because I thought it would work well for those goals. If I'm misusing rollup and this plugin, then that's no problem! I have used other tools in the past that I believe can deal with my use-case.

If you have the time, I would be interested to hear further clarification about the purpose of this plugin. I assumed one of its purposes would be to convert require() statements to import statements as part of its broader goal of converting cjs to esm. I believe that I would run into a similar issue if I wrote a library using esm that had a dependency on another cjs library which had a peer dependency on normalizr (or any module that has an esm build without a default export), which I would expect to happen from time to time. But perhaps it's uncommon—I don't see any related issues on this or the rollup repo.

At the end of the day if this is misuse of the plugin, or even if it's not misuse and you'd rather not address it since it's an uncommon edge-case, no problem! Thanks for taking a look.

eight04 commented 5 years ago

In rollup-plugin-cjs-es, we use an exportType option to decide whether the import statement should import a namespace or not.

shellscape commented 4 years ago

Hey folks (this is a canned reply, but we mean it!). Thanks to everyone who participated in this issue. We're getting ready to move this plugin to a new home at https://github.com/rollup/plugins, and we have to do some spring cleaning of the issues to make that happen. We're going to close this one, but it doesn't mean that it's not still valid. We've got some time yet before the move while we resolve pending Pull Requests, so if this issue is still relevant, please @ me and I'll make sure it gets transferred to the new repo. :beer: