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
Rollup should fail on extensionless imports in a type: module
package
#4301
Comments
I would expect this to only be an issue for externals and not for preserveModules as here, all imports are rewritten. Otherwise, this is a complicated topic: rollup itself does not know about Node.js semantics, only @rollup/plugin-node-resolve does. So this would be a feature request for the node-resolve plugin. |
@lukastaegert I'm currently working on a PR to take care of requiring file extensions as an opt-in, also needed as a pre-cursor for file URLs and query params. should be up shortly. |
That's the official line, but we know that rollup does have some basic knowledge of NodeJS semantics even without any plugins specified in order to append file suffixes:
rollup appends the ".js" suffix to
NodeJS' Lines 38 to 54 in 2e91c85
Rollup core also needs to read and write modules from the filesystem without plugins - something that requires the builtin I now think that Rollup 3.x should revisit the decision to not fold node-resolve and commonjs functionality into rollup core in order to give a more seamless user experience as other popular bundlers do. Rollup already has to have knowledge of NodeJS - so it might as well completely support it without the hassle of coordinating and specifying compatible versions of rollup, node-resolve and commonjs plugins. |
It has long been discussed if the support for extensionless imports was a mistake, but I do not want to open this again, especially since TypeScript worked hard to shape expectations by enforcing it. But everything else is basically missing, Index imports, and all features that depend on package.json knowledge etc. That may be what most users want, but the separation has enabled quite a few niche usages for Rollup that set it apart. Also it allows others to completely customise the experience. There are quite a few tools that build on Rollup these days that give users the tailored experience, especially looking at Vite here. |
I see your point - there's a lot of legacy usage out there. But what if the node-resolve and commonjs plugins were to just ship with rollup 3.x core and could be used with an opt-in flag? That alone would make it easier to use in the vast majority of user scenarios - no versioning, and no need to load or import them. I'd argue the flag should be an opt-out style option in rollup 3.x, but that's less likely to gain consensus. |
curious, what do you mean by that? |
@kzc not sure if I hope that the node.js resolution flag for esm will never be activated by default and subsequently be removed, as it only causes problems and makes everything more complex. |
@lukastaegert I think Typescript had other (technical) reasons to not enforce extensions. (the esm version of) to achieve running node.js esm, one has to use explicit file extensions with Typescript files as well. |
I'm not suggesting to change the rollup core architecture. Just proposing to ship the plugins most users need with rollup core - node-resolve, commonjs and possibly json - and provide an opt-in shorthand option to enable those plugins with default options. It would only require a very small change to the rollup CLI. If users wish to use their traditional rollup config files, or command line options - it would not break backwards compatibility. |
yeah, I figured. with |
one thing I also noticed: // index.js
import { foo } from "./foo";
console.log(foo); // foo.js
export const foo = 1; with // index.js
import { foo } from './foo.js'; // <--- extension included :P
console.log(foo); // foo.js
const foo = 1;
export { foo }; if this is not a bug but intentional, I would argue this to be another reason for requiring file extensions. 😉 |
@kzc configuring rollup with commonjs and node-resolve can definitely be a PITA. I have tried so many times (for node.js server code and lambdas etc.), that I have usually always switched to webpack (if I wanted to have the dependencies included, which, unfortunately, are mostly still commonjs). I also get @lukastaegert reasoning to not have those included in core. for one, rollup started out as an speaking of which, there is more module proposals on the horizon. should those in turn be included as well? 🤔 |
Rollup Version
v2.61.1
Operating System (or Browser)
macOS
Node Version (if applicable)
v16.13.1
Link To Reproduction
https://replit.com/@elado/rollup-repro#src/index.js
Expected Behaviour
Rollup should fail a build when package has
"type": "module"
inpackage.json
and sources import extensionless files.This becomes necessary in certain scenarios, like
external
s andpreserveModules
, where the output file keeps selective imports that will fail at runtime.Example 1:
The output file will correctly contain
import { local } from './local.js'
, but this should fail at the rollup build.Example 2:
The import is kept as external and the output import remains the same, which results in a runtime error in type: module.
Actual Behaviour
Rollup build succeeds, but runtime fails.
The text was updated successfully, but these errors were encountered: