WRT PR #3 - ES module path resolution

[bmeck]

This is the place to discuss specifics of how the EcmaScript (ES) module imports will resolve paths. The proposal itself can be seen in PR #3.

Discussions here should regard:

• What differences module resolution in ES module import syntax will have from require's module resolution algorithm.

It should not discuss import/export conversion, evaluation ordering, or detecting file mode.

[caridy]

To be clear, this is about import and export statements, whether they are declarative or imperative calls, it doesn't really matter because they are reflective somehow.

Additionally, we should, somehow, try to weight-in the resolution process used in browsers, to facilitate portability, I think @domenic can give us a quick overview of what we have so far in HTML.

Questions to be ask:

1. should the importer always include the file extension?
2. should we disallow the ambitious mechanism we do today in node to resolve index.js? specifically, the fact that require('./something') could resolve in ./something.js or ./something/index.js.
3. should we allow introspection into the module's internals? e.g.: import foo from "mod/lib/util"; or should we consider this a legacy behavior, and if you want to access the internals of the module, you should use require(), while an npm pkg will have a single es6 module that represents its api? (this somehow leaks into the discussion about .jsm vs package.json out of band signal)

[domenic]

What browsers are implementing is documented at https://html.spec.whatwg.org/#resolve-a-module-specifier. The answer to your questions for browsers are:

1. Yes, file extension is always necessary; import "./foo" looks for a file named foo, not one named foo.js.
2. Yes, index.js automagic is disallowed. import "./something" looks for a file named something, not one named something/index.js.
3. Not applicable yet, as any non-absolute-URL import specifiers not prefixed with /, ./, or ../ fail. Both seem possible in the future; when a mapping table API is created (manifest etc.) it could either be based on prefix-matching (allowing inspection of "internals") or just string equality (only allow inspection of "internals" if explicitly exposed in the manifest).

[bmeck]

The proposal currently has these specified. Currently the behavior is:

1. yes, importer must include file extension
2. this is disallowed if not looking for a package, but when searching for package main we use the current require algorithm.
3. currently this is allowed, it is used by various packages on npm and would be a hard sell. Currently the proposal does require file extensions for inner files, that would only affect file extension (keep that discussion contained to WRT PR #3 - ES module detection #13).

[caridy]

@domenic yes, 3 does not apply to the browser, although you could enforce that in the future via loader hooks.

@bmeck excellent, 1 and 2 seems to be very straight forward. As for 3, I think the various packages on npm today that are doing so are fat packages, and the transpiled version of their source are doing require() anyways. Once node supports ES modules, they will have to rework their pkgs a little bit, and should be fine to introduce this restriction. For years, we have been battling this, it is a refactor hazard for module authors, and signaling that to consumers of the modules by forcing them to use an alternative api to import those internals, seems like a good tradeoff here.

[bmeck]

@caridy banning inner files will cause some issues, but can probably smooth that over. Going to say we ban inner imports for packages.

[domenic]

I think it would be good to give people an opt-in way to expose their internals. I agree it's a hazard that they're automatically exposed, but for e.g. packages like especially or IIUC lodash it's an important part of their API.

[caridy]

@caridy banning inner files will cause some issues, but can probably smooth that over. Going to say we ban inner imports for packages.

@bmeck I'm not proposing banning them, obivously, inside your pkg you can do whatever you want since you can do ./path/to/relative/files.js. My proposal is to ban it when using import "packageName/fileName.js";

packages like especially or IIUC lodash it's an important part of their API.

@domenic Yes, we have few popular pkgs using that technique, and those will continue to work for people who are using require() to import them, but if you want to use an import statement, you will have to do something like: import {Math} from "especially"; which is itself a namespace produce by: export * as Math from './math.js'; in your index.js which will describe the API of the pkg.

[jkrems]

@caridy We have packages that include service clients and stubs to use for testing. We really don't want to load the service stubbing code into our production apps. Disallowing separate entry points sounds like a pretty big restriction. "You can keep using require" is another way of saying "if you want consistency in your code base and you're doing anything beyond toy examples, ignore import". Which would be a rather unfortunate outcome.

[bmeck]

@domenic would the reexport work if the loader does not evaluate any dependency that only has reexported bindings being imported (words @_@)?

// especially
export * as Math from './math.js';

// consumer
import {Math} from 'especially';

If we specify that especially does not evaluate since it has no direct bindings requested by consumer. Would that work? (me is rereading 262 since this would be a change it looks like, feel free to correct me).

[domenic]

I don't think there's any need to change the execution strategy. Reexporting would work I guess. Just kind of ugly.

[bmeck]

Forget what I said, side effects could mean the execution order needs preservation. We can't skip.

[caridy]

@caridy We have packages that include service clients and stubs to use for testing. We really don't want to load the service stubbing code into our production apps. Disallowing separate entry points sounds like a pretty big restriction.

@jkrems this is precisely what this discussion is for, to surface use cases, like yours. Can you elaborate more about the structure of your pkg? maybe a link that we can look at?

"You can keep using require" is another way of saying "if you want consistency in your code base and you're doing anything what toy examples, ignore import". Which would be a rather unfortunate outcome.

to reiterate, it is the consumer or existing pkgs using require() that will continue using require() for their convenience. If, as a consumer, you really want to use import, then you follow the new API provided by the author of the pkg.

[timoxley]

We have packages that include service clients and stubs to use for testing. We really don't want to load the service stubbing code into our production apps.

This is a use case I've hit and pondered with ES6 modules a few times. While it's possible to control what gets exported, there doesn't seem to be any way to conditionally prevent unneeded code from being loaded at all.

[bmeck]

@timoxley you will need to use an imperative function to load things conditionally:

// declare what can be exported:
export if_debug;

// conditionally load using require
let if_debug = IS_DEBUG ? require('foo') : null;

// conditionally load using System.loader.import
let if_debug = null;

// block completion using `await` , this would not affect require
// after this point if_debug is defined
await System.loader.import('foo').then(ns => if_debug = ns;)

[caridy]

additionally, once we settle on the api for the import() imperative api, you should be able to do something like:

export let if_debug = null;

if (something === true) {
    if_debug = await import('./something');
}

and that is not that far from what you do today in node.

[ljharb]

fwiw, "optional file extensions" and "index.js automagic" is something that's pretty easy to implement with rewrite rules on webservers, and I suspect that many will choose to do this so that require('./foo') in the browser works the same as node.

[bmeck]

@ljharb the requirement for file extension is to remove magic / cause less stat() calls in node. But yes, it is pretty easy to have rewrite rules for the server/browser layer.

[domenic]

Doesn't work on gh-pages.

[timoxley]

@bmeck dropping down to require is how I've handled conditional loading currently (via babel), but this unfortunately exposes ES6/commonjs interop complexities.

I support ES6 modules but that you can't do conditional loading without using require could be seen as an argument that we're upgrading to something worse/unnecessarily complicated – now we have import & export syntaxes, require and System.loader.import, when before we only had require and module.exports.

Also like to add I like the idea of decoupling the external interface from the internal file structure… currently internal file structure becomes a part of the public interface, so making any sort of file structure changes is potential API breakage, which is annoying and error prone for both producers and consumers.

[timoxley]

@bmeck will there be any require.extensions equivalent for hooking extra logic into the loader? or is this handled by interfering with System.loader?

[bmeck]

@timoxley we do not endorse require.extensions but under the current proposal it is honored. The WHATWG Loader does have hooks, but it is not specified that we will be using those since we have a different pipeline.

[caridy]

@timoxley read above to see how to load something conditionally without too much hazard. And no, you will have import, import() and export that should be more than enough.

[ljharb]

@domenic it wouldn't work as-is, but Github could easily add the simple rewrite rules required (or statically duplicate the relevant JS files in advance), so it's not outside the realm of possibility that it could work fine in the future.

[caridy]

@ljharb if we disallow the ability to do import "./foo" because it is ambiguous (it could be ./foo/index.js or ./foo.js), what's the point of omitting the extension?

As for the rewrite rules: just because every server out there can add a rewrite rule to align with node resolution logic doesn't mean they will do so. If we want to bring the npm resolution process to the front, small changes will have to be made for the greater good.

[bmeck]

@caridy we cannot place the burden completely on node to support all existing workflows (such as servers that will not change), we must balance the risks and best path for node here.

[jokeyrhyme]

One good thing about having such strict resolution is that we could start writing our code this way now, and it'll work in both new and old Nodes. We could also start writing new code (or porting old code) to avoid the need for deep imports (e.g. mymodule/something/within.js'. If this is the best advice, then we should begin to circulate it as soon as possible.

[caridy]

@jokeyrhyme 👍

[bmeck]

Update, this is on tract to be accepted as Draft. I have received renewed argument about the removal of search paths via IRC.

In the PR @domenic was requesting the file extensions be mandatory. There was a concern over urls mapping to actual files.

In this thread @ljharb raised the point of the migration not being seamless because of removing the path searching. Talking on IRC, he is suggesting index.js and file extension searches be included in module implementation. Removal of package.json search outside of node module would still be excluded from import.

As this is a Draft, and as it is not Accepted, the community and ecosystem can still weigh in. Since path searching can be added and would not be conflicting with the current proposal, arguments can continue without preventing the existing proposal.

[bmeck]

@mariusGundersen please use something like npm link or linklocal to accomplish this. We do a similar action to this when explaining non-local dependencies, but the resolution algorithm already supports such behavior via symlinks.

Simply place a symlink in node_modules/name_of_root then perform

import "name_of_root/inside_root";

[mariusGundersen]

Sorry, I may not have been entirely clear. What I mean is that I want to import a module in the same package/project without using a path relative to the current file/module. If the project/package has several folders, then it would be nice to import a file/module in a subdirectory. Think of it like how C#, Java, Python, etc do namespaces/packages.

Currently you need to know where the module you want to import is relative to the module you want to import it into. For example, in controllers/myController.js I want to import the model in models/myModel.js. Today I have to use import myModel from '../models/myModel.js'. Most other languages let you do this without having to travers backwards. In other words, there is a way to reference the module/file from the project/namespace/package root: import myModel from '#models/myModel.js (I'm using # here as the standard symbol for proposed-functionality).

[bmeck]

@mariusGundersen create a symlink node_modules/models that points to ../models, use require or import to get models/myModel.js

A console dump of doing this in a clean directory:

LMDV-BRADLEY:link-example bfarias$ mkdir models
LMDV-BRADLEY:link-example bfarias$ echo 'console.log("model!")' > models/index.js
LMDV-BRADLEY:link-example bfarias$ mkdir node_modules
LMDV-BRADLEY:link-example bfarias$ ln -s ../models node_modules/models
LMDV-BRADLEY:link-example bfarias$ node
> require('models')
model!
{}
>
(To exit, press ^C again or type .exit)
>
LMDV-BRADLEY:link-example bfarias$ ls */
models/:
index.js

node_modules/:
models
LMDV-BRADLEY:link-example bfarias$ ls -l */
models/:
total 8
-rw-r--r--  1 bfarias  386085923  22 Apr 25 07:08 index.js

node_modules/:
total 8
lrwxr-xr-x  1 bfarias  386085923  9 Apr 25 07:09 models -> ../models

[mariusGundersen]

Right, now I get you :)

That works, but doesn't it feel quite hacky to you? Wouldn't it be better if this had built in support in the path resolving algorithm? The fact that we have to resolve to something like this indicates that there is a need for what I described above that is simpler and works with published packages too (Your method with symlink won't work if I publish that project on npm, right?).

[bmeck]

@mariusGundersen it is supported in the path resolution algorithm, I am unsure what needs to be added?

linklocal has the tooling to reconstruct the same symlinks upon installation as the example I showed you if you want to publish your package to npm (on the other side if npm was able to package up symlinks like a normal tar that wouldn't be an issue [windows has strange behavior preventing this]). I don't think we need to have multiple resolution mechanisms for bare import/require paths to do this since it is already simple to explain/some tools exist.

New resolution mechanics would need to bring more than what currently exists to the table. We will not be doing network based loading at this time for sure.

[mariusGundersen]

I guess we disagree on whether or not something that I'd a common problem needs a simple, built inn solution or if it is OK that it has a workaround only. Sure, using symlinks works now, but wouldn't it be better to support this without symlinks? I see a few problems with it, for example having a directory with the same name as an installed package (they would have the same name inside node_modules).

Wasn't it also discussed not to be able to import other files from a package using import? As in import x from 'lodash/core' wouldn't work anymore. Wouldn't that break the symlink workaround?

Isn't this a good opportunity to add a resolution mechanism that people have wanted for a long time? It could probably be added later, but since we have breaking changes anyway, this feels like a good time to introduce it.

[jkrems]

@mariusGundersen I actually would argue that such a resolution mechanism ("relative to root of project") is a net-negative because it encourages having spiderweb dependency graphs (everything depends on something else in a completely different part of the project) instead of a clean dependency tree. Adding a cost to reaching into different parts of the code base is a good thing imo. That's highly subjective of course, but it definitely isn't as simple as "this is clearly a missing feature that everyone is waiting for". :)

Can you elaborate more about the structure of your pkg? maybe a link that we can look at?

I don't think we have anything using that pattern on public Github. But the general idea is that a package contains both the actual library code and "test support code". The two are a) relatively small and b) tightly coupled. Which makes it awkward to split them into separate packages. Pseudo-code:

// In "production"/app code:
var MyClient = require('the-client-library');
new myClient(config.myClient);

// Somewhere in the test setup:
var fakeApi = express();
fakeApi.use(require('the-client-library/mocks'));
fakeApi.listen(config.myClient.port);

[bmeck]

@mariusGundersen the proposal still allows inner path resolution. The idea of removing it was discussed, but is not in the proposal. Please see:

Symlinks of [...] node_modules/foo/ -> $HOME/.node_modules/foo/, etc. will continue to be supported. [1]

import x from 'lodash/core' is one of the reasons the proposal is this way as noted in [2]

The solution is not a workaround, it is how path resolution works. Having a single flat namespace for "bare" path resolution is important. You can suggest tooling or proposals to encourage this but it should be using a different syntax that does not break backwards compatibility and is not related to this proposal.

[gulbanana]

Static file servers such as http-server being able to serve <script type=module ... with minimal effort

It's not going to be possible for static file servers to serve node .mjs files for <script, since they won't have the right mime type. So there will have to be some sort of transformation step anyway.

[bmeck]

@gulbanana browsers don't check for file mode via extension, also see note about mime, we can register the file extension to match the MIME if we start implementation for sure.

[ljharb]

@bmeck fwiw, import x from 'no_package_json_but_in_node_modules/foo' won't work with the current proposal, and does work with require :-(

Specifically, this is how many rails apps symlink app/assets/javascripts into node_modules so that they can avoid relative requires.

[gulbanana]

If IANA accepts .mjs as an alternative extension for application/javascript then it would (eventually) solve the problem, yeah.

[pakastin]

I don't like or understand the use of .mjs.
Why not do the following for the backwards-compatibility:

package.json

{
  "main": "cjs/bundle.js",
  "module": "src/index.js"
  ...
}

You could bundle backwards-compatible version with rollup for example:
rollup -f cjs src/index.js -o cjs/bundle.js like you do in the client-side as well..?

[ljharb]

@pakastin because that only works for things with a package.json.

[pakastin]

@ljharb: You mean client-side? What's the issue there then?

[ljharb]

@pakastin no, i mean requiring any file from any folder. Only npm-installed packages have a package.json, but any file can be required.

[pakastin]

I don't understand. If you use import don't you expect the file being an ES6-module? Wouldn't it be better to be other way around: if you want to import legacy commonjs file it would have the suffix '.cjs' for example?

[ljharb]

@pakastin no. you should be able to import a CJS module and require an ES6 module without having to know the module format that was used.

[pakastin]

Why need for .mjs then? Don't you know the format if you define it to be .mjs?

[mariusGundersen]

@pakastin please read this comment (and the rest of the thread): #3 (comment)

[pakastin]

ok, thanks! 👍

[bmeck]

@ljharb we discussed changing the resolution mode when using [bare paths], that apparently did not get written down into spec, bug, will fix when I get a chance. Def needed.

[ljharb]

@bmeck thanks. does the same problem exist for import x from 'no_package_json_but_in_node_modules'?

[bmeck]

@ljharb nope, see examples under https://github.com/nodejs/node-eps/blob/master/002-es6-modules.md#es-import-path-resolution

[matthewp]

Sorry to chime in so late, I don't think the file extension issue is fixable for web servers with rewrite rules, a rewrite rule is just a rewrite rule, this is an algorithm:

1. try FILENAME
2. try FILENAME.js
3. is FILENAME === "package", if so try "package.json"
4. TRY FILENAME/index.js

Maybe I'm just super-ignorant about what nginx/apache can do with rewrite rules, but I'm thinking a web server plugin is likely needed to achieve this.

[ljharb]

@matthewp a rewrite rule (certainly an apache one, i'm not sure about others) should trivially, given the request path /foo, be able to check for the existence of foo.mjs, foo.js, foo/index.mjs, and foo/index.js, selecting the first one that is found, without the browser having to know either the extension or whether the path is a directory or not. To me that seems sufficient to address any issues. What seems problematic to you?

[matthewp]

Ok, I'll take your word for it, I assuming you would use things like RewriteCond, I didn't realize rewriting was that sophisticated. So the remaining issue would be that the server needs to conditionally apply this rewrite only to module requests, and I don't see a signal of that from the browser as currently specced.

[ljharb]

It would apply it to everything without an extension explicitly included, with the JS mime type. The browser spec has nothing to do with that.