The ESM resolver does not respect package `main`s for packages imported via relative imports

[weswigham]

Version

17.1.0

Platform

Microsoft Windows NT 10.0.19042.0 x64

Subsystem

No response

What steps will reproduce the bug?

It's possible this is intentional (or at least an unintentional effect of how the esm resolver spec was written/implemented), but it seems like a major departure, so here goes. While I was working on fixing this issue in TS, I couldn't shake the feeling that something was off about node's behavior.

Suppose I have a (cjs) package named "pkg":

// @filename: /node_modules/pkg/package.json
{
  "name": "pkg",
  "version": "0.0.1",
  "main": "./entrypoint.js"
}
// @filename: /node_modules/entrypoint.js
module.exports = "entrypoint loaded!";

which I import from my esm consumer package:

// @filename: /package.json
{
  "type": "module",
  "private": true
}
// @filename: /index.js
import str from "pkg";

everything works fine. Likewise, from my cjs consumer package

// @filename: /package.json
{
  "type": "commonjs",
  "private": true
}
// @filename: /index.js
const str = require("pkg");

everything works fine. In addition, in my cjs package, I can do a relative import of that "pkg" directory:

// @filename: /package.json
{
  "type": "commonjs",
  "private": true
}
// @filename: /index.js
const str = require("./node_modules/pkg");

which works fine, however if I try to do the same in an esm package:

// @filename: /package.json
{
  "type": "module",
  "private": true
}
// @filename: /index.js
import str from "./node_modules/pkg";

you get an import path error - the package's main is ignored entirely! This means that while in cjs, you can happily move packages out of your node_modules folder (likely to vendor them) and refer to them by relative paths and they keep working just dandy, to do so breaks any esm consumers (even though the package in question is cjs formatted, and so shouldn't care about esm consumers!).

This seems like a fairly major oversight - esm relative imports being unable to load packages entrypoints outside of node_modules which declare themselves to be cjs seems wrong and inconsistent - it's ignoring the declared (or implicit) format of the package the path points to. The package file is still respected for the format for loading any files you write out the whole path to, but is ignored when it comes to calculating the entrypoint.

cc @nodejs/modules

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior?

Importing a folder (import("./a/b")) that refers to a package ("./a/b/package.json" exists) should load that package's entrypoint, as with require.

What do you see instead?

Importing a folder that is a package (import("./a/b") where "./a/b/package.json" exists) ignored the package's entrypoint, unlike require, which respected it.

Additional information

It's possible this is expected, but it actually makes it really hard to work with packages outside node_modules (as when vendor'ing small dependencies), which pre-esm-resolver was actually pretty easy.

[ljharb]

I suspect this was expected/intended, but it's also likely that the (quite rare, I'd think, but certainly long and widely discouraged) use case of vendoring dependencies wasn't considered.

[weswigham]

Yeah, it's just odd that you can move around an old cjs package and vendor it and as long as you only try to use it from cjs, it works fine, but once you try to pull it into a newer esm mode file, it fails. :(

[ljharb]

I agree; there's a ton of things the CJS resolver can do that it's odd that the ESM resolver can't do.

[guybedford]

Yes this was discussed in detail, on the topic of directory imports, and was an explicit decision alongside extension searching. Vendoring of depenencies is still handled by monorepo symlinking or the package.json local dependencies pattern:

{
  "dependencies": {
    "vendored": "file:../package-path"
  }
}

where npm will symlink into node_modules in the above scenario, enabling the local package import with deduplication via realpathing.

[weswigham]

But, like, the resolver still pulls other stuff from the package.json (like its format), it clearly knows it's there; it just doesn't respect the entrypoint. 😦

It makes it actually hard to develop (esm) packages outside a node_modules structure, and requires the complicated monorepo-tool-symlinking-nonsense. 😅

[guybedford]

The simplification was that relative resolution can always be done without needing any lookups, it's just bare specifier resolution that runs through package resolution rules. This was part of removing the "magic" of resolution. I'm sorry if you feel it's unmagical now!

Just like extension searching, what seems very simple can lead to cascading statting:

/package.json

{
  "main": "./app"
}

/app/package.json

{
  "main": "./subapp"
}

/app/subapp/package.json

{
  "main": "./main-finally"
}

I'm hopeful for Windows symlink dev workflows going forward... perhaps other ways of defining custom package mappings for Node.js could be useful too (this being a separate thing to import maps / module mappings).

[guybedford]

@weswigham for users that this causes upgrade issues for, note that --experimental-specifier-resolution=node should still provide the old lookup behaviour as well.

[weswigham]

I mean, sure, but it's more like the esm resolver isn't respecting the intent of the package as written? Like, that's great and all, but if I'm vendor'ing someone else's package, I'm not looking to be in the business of manually resolving their entrypoints, y'know?

[hybrist]

Is there more info on this kind of vendoring? I assume it only works if the package happens to have no dependencies because a non-node_modules vendor directory would mean that cross-package imports using bare specifiers break?

[weswigham]

Normally this kind of vendor'ing is used for older browser umd-type packages which don't normally have external dependencies, yeah (and brings up anther reason why people may vendor in the first place: to host some part of their js files on something like gh pages without hosting/comitting the rest of their node_modules files used in their build).

[targos]

It's an issue I encountered here in core, where I needed to import from test/doctool packages installed in tools/doc/node_modules. The only workaround I found was to re-export them from a file in tools/doc:

node/test/doctool/test-doctool-json.mjs

Lines 8 to 11 in 863d13c

	import {
	remarkParse,
	unified,
	} from '../../tools/doc/deps.mjs';

https://github.com/nodejs/node/blob/master/tools/doc/deps.mjs

[bmeck]

i remember this from ye olden modules WG meetings where only during bare specifiers would searching potentially occur, this lets the resolver be in step with WHATWG's resolve a module specifier step in https://html.spec.whatwg.org/multipage/webappapis.html#hostresolveimportedmodule(referencingscriptormodule,-modulerequest):resolve-a-module-specifier

I'd also think another workaround is to use bundledDependencies for vendoring as well : https://docs.npmjs.com/cli/v8/configuring-npm/package-json#bundleddependencies

Per changing the behavior / changing so this is expected to work I think it would be complex since now every specifier might require main and exports to be processed instead of just bare specifiers. I'm open to discussion on this, but it was previously unable to be agreed that it would be a desired behavior. In particular, I'm a bit concerned about:

/package.json
/foo/entrypoint.mjs # imports ../bar/...
/bar/package.json

If this skips over /package.json's exports and only used /bar/package.json's fields. Right now exports also doesn't work ever to my knowledge using relative specifiers and it would be my main concern as people start adopting things. I'm not saying any of this is fatal, just a more than trivial discussion has already occurred and would be required I'd imagine to change things again.

[weswigham]

I mean, we're doing that here, right? I, at least, came here to file this because I found the current behavior surprising, given my experience with node packages in the past - beyond the 'imports usually require extensions' thing which can be seen as a simple choice of style enforcement from a user perspective.

It seems like a few others have run into similar issues since shipping the modules mvp; as-is, the es module implementation in node feels kinda... Tacked on? There's a bunch of rough spots (almost anywhere the existing module ecosystem is acknowledged), and I won't claim this is the worst one, but it is one that seems like something that could be fixed so long as the philosophical "relative imports shouldn't be redirectable" is just that - philosophical. And, like, I'm positive someone will pop in and say "well, a user loader could respect package.json files on relative imports if that's important", and my point is moreso that it's surprising that the default loader behavior doesn't. They're node packages, after all (often written in cjs for older versions of node), and node isn't recognizing them (sometimes, in some ways). The author of the package, and their intent, is being ignored, despite being known.