[node-resolve] extensionless import no longer works in v11.0.0 #684

shirotech · 2020-12-02T10:21:56Z

Rollup Plugin Name: node-resolve
Rollup Plugin Version: v11.0.0
Rollup Version: v2.34.0
Operating System (or Browser): macOS
Node Version: v14.15.1
Link to reproduction (⚠️ read below): https://repl.it/@shirotech/rollup-plugin-repro

Expected Behavior

import extensionless detects .js and compiles

Actual Behavior

extensionless is imported as is without resolving .js

Additional Information

was working in v10.0.0
don't see any mention where this is a breaking change?

shellscape · 2020-12-02T11:59:57Z

We're going to need a lot more info in the issue. Errors, what you tried, etc.

shirotech · 2020-12-02T12:02:21Z

@shellscape it's all in the repl, just need to run and it will show the error. Downgrading to v10.0.0 works etc.

import 'systemjs/dist/s.min'; // fails
import 'systemjs/dist/s.min.js'; // works

shellscape · 2020-12-02T12:04:27Z

Always include as much info as possible in the issue. The REPL is there to reproduce the issue, not to document it. The reproduction typically won't survive (and doesn't have to) past resolution of the issue, but the issue will always live in for reference. The same holds true in any repo you contribute issues to.

shirotech · 2020-12-02T12:07:05Z

Cool, no worries, here's the error, thanks!

npx rollup -c

input.js → output/bundle.js...
[!] (plugin Rollup Core) Error: Could not load /home/runner/rollup-plugin-repro/node_modules/systemjs/dist/s.min (imported by /home/runner/rollup-plugin-repro/input.js): ENOENT: no such file or directory, open '/home/runner/rollup-plugin-repro/node_modules/systemjs/dist/s.min'
Error: Could not load /home/runner/rollup-plugin-repro/node_modules/systemjs/dist/s.min (imported by /home/runner/rollup-plugin-repro/input.js): ENOENT: no such file or directory, open '/home/runner/rollup-plugin-repro/node_modules/systemjs/dist/s.min'

exit status 1

However, it's really self explainatory.

tjenkinson · 2020-12-02T13:20:42Z

looking into this atm

tjenkinson · 2020-12-02T13:41:20Z

Hmm I added a test for this which currently passes. tjenkinson@965bce9

Don't have any more time right now but will check this evening

felixapitzsch · 2020-12-02T16:06:30Z

+1 : I just ran into the same issue. Maybe it is helpful to add some more context:

In my current project, I have a complex build chain that has multiple stages of rollup-bundling and only some of them are breaking with node-resolve v11. The first stage bundles all dependencies into a single dependenciesBundle.js which has a stable API and can be updated and shipped independent from the application itself. A later stage bundles the app, treating the dependenciesBundle.js as external. The target platform is web-only. Therefore, the first stage needs to convert CommonJS code into ES-compliant JavaScript by using @rollup/plugin-commonjs. However, the later app bundling stage only faces pure JavaScript and does not need the plugin-commonjs. Interestingly, upgrading node-resolve from v10 to v11 only breakes the first stage of dependency bundling. The app bundling still runs smoothly with v11.

Here is my rollup.config.js for the breaking dependency bundling stage:

/* eslint-disable import/no-extraneous-dependencies */
import commonjs from "@rollup/plugin-commonjs";
import nodeResolve from "@rollup/plugin-node-resolve";
import injectProcessEnv from "rollup-plugin-inject-process-env";

// noinspection JSUnusedGlobalSymbols
export default {
  input: "./src/dependencies.js",
  output: {
    file: "./dependenciesBundle.js",
    dir: undefined,
    format: "es",
    sourcemap: false,
  },
  plugins: [
    nodeResolve(),
    commonjs({
      mainFields: ["module", "main", "jsnext:main", "browser"],
      extensions: [".js", ".mjs", ".json"],
      sourceMap: false,
      browser: true,
    }),
    injectProcessEnv({ NODE_ENV: "production" }),
  ],
};

LarsDenBakker · 2020-12-02T16:12:19Z

We need to tackle this on a package per package basis. It's likely that because node-resolve v11 now respects the exports map in the package.json of packages, there are different rules you need to follow now.

For systemjs, this is according to the node js specs. They have specified an export map: https://github.com/systemjs/systemjs/blob/master/package.json#L5 which means that is used for resolving imports.

When you do an import like this: import 'systemjs/dist/s.min'; it means it will match the rule "./dist/": "./dist/" and thus an explicit file extension is required.

Regarding /@babel/runtime/helpers/esm/assertThisInitialized, this is the same issue as #672 which was already fixed but not yet released.

tjenkinson · 2020-12-02T17:53:48Z

Right, so it sounds like this is not a bug?

It might be possible to handle this with another plugin with which would try adding the .js if it's missing.

Something like

{
  name: 'fallback-js-extension',
  async resolveId(source, importer) {
    return await this.resolve(source + '.js', importer, { skipSelf: true });
  }
}

(untested)

shirotech · 2020-12-02T20:50:02Z

Right so if it is a breaking change should it be mentioned somewhere in the changelog? Just to avoid people opening up further issues on it.

LarsDenBakker · 2020-12-02T20:55:38Z

Technically the support for package entrypoints is mentioned. The behavior should now be the same when using native node js, node-resolve v11 and webpack 5. But I can see that the changelog entry doesn't explain all the details by itself.

lukastaegert · 2020-12-03T05:16:26Z

As the changelog is rather concise, maybe this is more an issue for the readme. By the way, we do not have an opt-out for this feature, do we? Because as this issue shows as package exports can severely restrict/change how certain packages can be imported, this may prevent users from upgrading if there existing code-base heavily depends on old import patterns. Having an opt-out could serve as a means for easing the transition. Also, think about the scenario where a dependency you do not have control over imports from another dependency in a non-compliant way. Would this be difficult to add? I am not thinking of anything fine-grained here, just a brutal on-off switch. Such a feature could also serve as a place in the readme where these restrictions are explained. Thoughts?

LarsDenBakker · 2020-12-03T20:39:26Z

I'm 50/50 on that. The plugin is called node-resolve, and this is exactly how the node resolution logic works. You can't turn it off in node, and from what I can tell you can't turn it off webpack either. Adding the option to disable it makes it easier to fragment the ecosystem like it has with esm/commonjs interop.

A full on/off switch also wouldn't be enough I think, because it will be easy to end up with some packages that have exports that look different from the actual file system For example:

{
  "exports": {
    "./foo": "./dist/foo.js"
  }
}

This kind of export would not work anymore when it's turned off completely.

Adding a switch today would be possible, but the underlying resolve package is also working on adding support for the exports field. They would have to make this configurable too for it to keep working.

* fix runtime node module resolve error According to rollup/plugins#684, it's necessary to use an explicit extension when import if there's an `exports` field in `pacakge.json`. * run changeset

shellscape · 2021-02-01T02:14:35Z

Going to close this as working as expected citing the conversation in the ticket. Thanks for everyone's participation.

tbroyer · 2021-04-21T11:58:54Z

I think there's a bug when using this with commonjs.
I'm trying to move a project from Parcel v1 to Rollup, and facing this with apollo-upload-client, which has require("extract-files/public/extractFiles") causing

Error: Could not load /home/…/node_modules/extract-files/public/extractFiles (imported by node_modules/apollo-upload-client/public/createUploadLink.js): ENOENT: no such file or directory, open '/home/…/node_modules/extract-files/public/extractFiles'

Following https://nodejs.org/api/modules.html#modules_all_together, require("extract-files/public/extractFiles") will end up calling

PACKAGE_EXPORTS_RESOLVE("file:///…/node_modules/extract-files", "./public/extractFiles", { …, "./public/": "./public/", … }, ["node", "require"])

Following https://nodejs.org/api/esm.html#esm_resolver_algorithm_specification, this will call

PACKAGE_IMPORTS_EXPORTS_RESOLVE("./public/extractFiles", { …, "./public/": "./public/", … }, "file:///…/node_modules/extract-files", false, ["node", "require"])

which will then call (from 3.2.3)

PACKAGE_TARGET_RESOLVE("file:///…/node_modules/extract-files", "./public/", "extractFiles", false, false, ["node, "require"])

This will return (from 1.8) file:///…/node_modules/extract-files/public/extractFiles. PACKAGE_EXPORTS_RESOLVE will thus return { resolved: "file:///…/node_modules/extract-files/public/extractFiles", exact: false }.
Back to https://nodejs.org/api/modules.html#modules_all_together, we'll end up in

RESOLVE_ESM_MATCH({ resolved: "file:///…/node_modules/extract-files/public/extractFiles", exact: false })

and because of exact: false, Node would correctly add the .js extension and find file:///…/node_modules/extract-files/public/extractFiles.js.

Webpack 5 had the same issue, and changed behavior last October: webpack/enhanced-resolve#256

So, this is a combination of using node-resolve, commonjs, extensionless requires, and a package using the deprecated subpath folder mappings. Had the extract-files package used subpath patterns, it would have ended up in RESOLVE_ESM_MATCH with exact: true, making the file extension mandatory.

guybedford · 2021-04-21T12:04:55Z

@tbroyer that does in fact sound like a bug to me. Can you please post this as a new issue?

tbroyer · 2021-04-21T16:20:30Z

Done: #865

This comment has been minimized.

Sign in to view

johannes-z mentioned this issue Dec 10, 2020

@babel/plugin-transform-runtime 7.12.10 breaks pnpm monorepo babel/babel#12476

Closed

iFwu mentioned this issue Dec 11, 2020

fix runtime node module resolve error preactjs/prefresh#230

Merged

FredKSchott mentioned this issue Dec 11, 2020

node-resolve cannot resolve exports entry #727

Closed

shellscape added 🔌 plugin-node-resolve x⁸ ⋅ wontfix labels Feb 1, 2021

shellscape closed this as completed Feb 1, 2021

This was referenced Apr 21, 2021

package.json uses deprecated subpath folder mappings in exports jaydenseric/extract-files#17

Closed

Extensionless require() for subpath in package with subpath folder mappings in its exports map fails to resolve #865

Closed

aomarks mentioned this issue Jul 1, 2021

Importing files from lit require the .js extension in TypeScript lit/lit#1986

Closed

kmmbvnr mentioned this issue Aug 3, 2022

Extensionless imports in master-branch no longer supported by node-resolve material-components/material-web#3548

Closed

alexkuchun mentioned this issue Apr 17, 2023

[Feature Request]: Adjust packages exports to align with the nodejs module resolving logics SAP/ui5-webcomponents#6921

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[node-resolve] extensionless import no longer works in v11.0.0 #684

[node-resolve] extensionless import no longer works in v11.0.0 #684

shirotech commented Dec 2, 2020

shellscape commented Dec 2, 2020

shirotech commented Dec 2, 2020 •

edited

Loading

shellscape commented Dec 2, 2020

shirotech commented Dec 2, 2020 •

edited

Loading

This comment has been minimized.

tjenkinson commented Dec 2, 2020

tjenkinson commented Dec 2, 2020 •

edited

Loading

felixapitzsch commented Dec 2, 2020

LarsDenBakker commented Dec 2, 2020 •

edited

Loading

tjenkinson commented Dec 2, 2020

shirotech commented Dec 2, 2020

LarsDenBakker commented Dec 2, 2020

lukastaegert commented Dec 3, 2020 •

edited

Loading

LarsDenBakker commented Dec 3, 2020

shellscape commented Feb 1, 2021

tbroyer commented Apr 21, 2021

guybedford commented Apr 21, 2021

tbroyer commented Apr 21, 2021

[node-resolve] extensionless import no longer works in v11.0.0 #684

[node-resolve] extensionless import no longer works in v11.0.0 #684

Comments

shirotech commented Dec 2, 2020

Expected Behavior

Actual Behavior

Additional Information

shellscape commented Dec 2, 2020

shirotech commented Dec 2, 2020 • edited Loading

shellscape commented Dec 2, 2020

shirotech commented Dec 2, 2020 • edited Loading

This comment has been minimized.

tjenkinson commented Dec 2, 2020

tjenkinson commented Dec 2, 2020 • edited Loading

felixapitzsch commented Dec 2, 2020

LarsDenBakker commented Dec 2, 2020 • edited Loading

tjenkinson commented Dec 2, 2020

shirotech commented Dec 2, 2020

LarsDenBakker commented Dec 2, 2020

lukastaegert commented Dec 3, 2020 • edited Loading

LarsDenBakker commented Dec 3, 2020

shellscape commented Feb 1, 2021

tbroyer commented Apr 21, 2021

guybedford commented Apr 21, 2021

tbroyer commented Apr 21, 2021

shirotech commented Dec 2, 2020 •

edited

Loading

shirotech commented Dec 2, 2020 •

edited

Loading

tjenkinson commented Dec 2, 2020 •

edited

Loading

LarsDenBakker commented Dec 2, 2020 •

edited

Loading

lukastaegert commented Dec 3, 2020 •

edited

Loading