-
Notifications
You must be signed in to change notification settings - Fork 44
Proposal: minimal esm implementation #141
Comments
Quick review from user perspective. Not only the binaryYou wrote "must use .mjs as entry point for node binary" but I've noticed Overall working ...Overall everything seems to work as expected:
import fs from 'fs';
import rand from './module.js';
const {require} = import.meta;
const readFile = require('util').promisify(fs.readFile);
Promise.all([
readFile('./package-map.json'),
require('./module.c.js')
]).then(([data, crand]) => {
console.log(rand === crand);
console.log(data.toString());
}); ... but !I am not sure I am doing it right with the {
"path_prefix": "./node_modules",
"packages": {
"flatted": { "main": "esm/index.js" }
}
} and there's no way I can import The Last, but not least
Wishes
import path from 'path';
const {
pathname: __filename,
__dirname = path.dirname(__filename)
} = new URL(import.meta.url);
console.log(__filename);
console.log(__dirname); |
Can you define "transparent" interop here so that it doesn't get confused. I remain strongly opposed to I would like to better understand the reasoning behind all of these choices rather than just seeing something that works. It would be nice to see a write up on what things were chosen not to be supported and why the use cases they provide can be served with this minimal implementation.
|
To clarify, the issue with the package-name maps is that it requires the specifier to have an extension. Unless there's is another way to branch on the module type used by the consumer, this rules out the patterns relying on "mjs + js" (see table in #139). This currently leaves only "Default Export" and its clunky API as a completely safe pattern. |
I really like the direction of going for a more minimal solution. I agree with @bmeck that there's some open questions. But I'd like to address them from a perspective of "what is the use case". So "Loading of polyfill or APM instrumentation that is implemented in CJS" instead of just "ordering concerns". Because that will most likely inform how we talk about it. When refactoring an application with "random" CJS files, the order is not as problematic in the general case. |
@jkrems we have files for configuration and singleton modules at work that need to evaluate prior to others. They configure things like servers and database connection pools. These are outside of polyfill or APM concerns. |
But I assume (hope) that they don't set random globals that other files then just assume are initialized..? If they do, I would treat them as "polyfills" (mutate global state to expose additional APIs). |
For additional color, the following will "just work": import fs from 'fs';
import randomLib from 'esm-lib';
const db = import.meta.require('db');
db.accessTable(); // db init will still happen before this |
for even more color, the following will "just fail": import.meta.require('x_polyfill'); // only available as cjs
import "something_expecting_x"; // only available as esm I really don't like the term "just works" because it leaves out all the things that don't "just work" |
One possible answer for "how would polyfills work in that world" would be "it requires an intermediate module", e.g.: import.meta.require('polyfill-a');
await import('./run-app'); But that definitely reduce the abilities of modules that need to ensure a polyfill (no static import of actual code, pretty awkward exports). |
I hate the term "just work"/"just fail" because it shuts down discussion of what/why things act a specific way under a guise of it being plainly apparent. We should try and explain why things serve or do not serve a use case. I also want to bring up concerns that I have expressed elsewhere about introducing CJS permanently to all ESM modules under I also only see I am unswayed by the claims of web compatibility problems with importing non-ESM, especially in the face of browsers wanting to include non-ESM such as HTML modules. The arguments about compatibility have not made direct claims about how the ability to import CJS directly prevents support for some feature on the browser platform. All claims have been around codebases being unable to run in all platforms, however that claim is easily disproven if the codebase entirely is using ESM. Once browsers encourage non-supported formats such as HTML the same claim would be applied that their module system is no longer compatible with Node, but we can once again easily disprove this by creating applications that do not use HTML modules. What is the claim for compatibility problems in the face of the issues with not supporting importing CJS above? The claim that a codebase does not work on a platform is a codebase compatibility concern, please describe the platform compatibility concern. |
I'm not sure I follow. |
@jkrems |
I really don’t understand why we’re seeing implementation proposals before we’ve finished discussing transparent interop (all of its definitions) and defaults. |
I agree with @ljharb here. It's good to see this work, and @WebReflection 's feedback is valuable, but I think the waters are still pretty muddy. I think we should come back to looking at implementation alternatives after we've done some more work on our framework for evaluating implementations. |
IMHO
import module from 'module'
const url = new URL(import.meta.url)
const require = createRequire(url)
const cjs = require('./main.js')
|
the author of the file should always have first say over what kind of file they authored. node can provide things to override defaults but the default behaviour must always defer to the author of the file. |
I'd be cautious with that line of thinking. Making anything less convenient should never be a goal. Because people are really good to find a path of least resistance. So if you make something less convenient, people will find a more convenient solution. And it's almost never what you actually wanted them to do. |
@MylesBorins In general, I think this is a good direction. How does resolution work when importing a package specifier?
|
There appear to be some bugs around package specifiers; it seems to be possible to import from a directory if the directory is contained within a package path (e.g. Other than that, the general idea seems to be: you can only import from a directory if you are using a root-level package specifier, and in that case the normal Using this approach, an author could dual publish by including both "index.js" and "index.mjs", or by having two similarly named files and using an extensionless entry in I think I would simplify things: make it possible to import from any directory, but only use The other issue here is that dual-publishing still forces authors to use The community has already converged on As @WebReflection mentioned, I would also add an executable flag for specifying the entry point format. Summary:
With those changes, we can easily support dual-mode packages for interoperability and we allow users to keep using the ".js" extension if they choose. |
If anyone would like to push commits on top of this to fix bugs, extend / change implementation, or anything else please do. Follow up with a comment and if I have issues with what's pushed we can talk through it. edit: oh yeah this isn't a PR... lol whooops. If you post a link to a commit sha I'll cherry pick and push first and ask questions later 😄 |
I’m tempted to say the implementation approach is just about right (re package specifiers) - in
that having extensions for the main is ok, or allowing directory lookups
are ok, as long as it was resolved from a plain name to begin with thereby
being package map compatible.
…On Thu, 28 Jun 2018 at 22:36, Myles Borins ***@***.***> wrote:
If anyone would like to push commits on top of this to fix bugs, extend /
change implementation, or anything else please do. Follow up with a comment
and if I have issues with what's pushed we can talk through it.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#141 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAkiyjuTrquyv7F3o91Wres8cq1HCLuHks5uBT5CgaJpZM4U62rR>
.
|
In #137 there was a lot of discussion of initial entry points. I think the consensus was that we need a CLI flag to handle the |
FWIW, since it's actually super trivial to have const {
pathname: filename,
dirname = filename.replace(/\/[^/]*$/, '')
} = new URL(import.meta.url); that's already a cross env KISS approach. |
Absolutely! Perhaps @MylesBorins could merge my commits and provide an updated download link? 😺 |
I don't want guessing in my server. guessing means crawling, mean degraded performance, means complexity. Which should I guess first, This whole thread is about an MVP for ESM in modules and guessing is not part of an MVP, IMO. |
@WebReflection are you referring to the debug binaries for engines? I don't think we should be considering those at all... they just exist to test the engines. |
Migration is a required part of an MVP, and imo that includes guessing. In other words, we disagree on what "minimum" and "viable" mean. As for which you would guess first, that'd be easy - you'd follow the order of |
it's not even an Array and last one I have just checked is I also write ESM daily and I've never had any issue in specifying the bloody extension of a file name. Bundlers solve that, good for them, but if you write ESM for real, you better write that extension or it wont natively work on browsers/server 'cause nobody writes overkilling rewrite rules for a missing |
package maps and bundling fix that. if you can somehow invalidate the existence of those then you have a point. no one expects have code written in node.js can be verbatim copied into a browser and just work beyond toy code and polyfills. idiomatic node lets you drop the file extension because what a package is written in is an implementation detail. we shouldn't give that up for misinformed browser compatibility. |
I'm pretty sure the Implied extensions and a priority list therein are really common among webservers. You may have never reconfigured it yourself, but it's usually there.
|
Also cdnjs |
if you use those, you won't write fully qualified path up to the extension. If you have package maps you Anyway, if you think omitting the extension is not just useless overhead and unnecessary for this MVP go ahead, all I was saying as daily ESM user is that explicit file names are not an issue, and never will be. In JSC and SpiderMonkey these are also mandatory and ESM shipped like a year ago ... that is what I call a MVP: it works, it doesn't have much philosophy or guessing around file paths, it shipped ahead of time. |
You surely do write it, in package-name-map.json {
"path_prefix": "/node_modules",
"packages": {
"moment": {
"main": "src/moment.js"
},
"lodash": {
"path": "lodash-es",
"main": "lodash.js"
},
"wrong": {
"main": "src" // <= No
}
}
} https://github.com/domenic/package-name-maps#basic-url-mapping
👌 |
@michael-ciniawsky it seems like we agree, or at least, I agree with everything written in that quote, and also JSC and SpiderMonkey already work like that (fully qualified names). |
you keep saying this, but what does it mean. are you referring to the cli debuggers they maintain? the APIs built into the engines? I have no idea that that means. |
echo 'print(123)' > module.js
echo 'import "./module.js";' > entry.js
jsc -m entry.js
# prints 123 you can try omitting part of the name (extensions are irrelevant, imports resolve the path though) echo 'import "./module";' > entry.js and see that
Both JSC and SpiderMonkey, and browsers, uses out-of-the-box resolvable names, without any guessing involved, neither folder or extension crawling. This is the meaning of (KISS and) an MVP, imo. edit
meaning, they ask for what you are importing, they don't crawl the server, they don't guess extensions, they ask for a module with that name, end of the story. |
so you are referring to the cli debuggers. those only exist to test functionality of the engines. i would request that you do not base any behaviour in node.js on those, as node isn't a debug utility - its a production runtime with millions of users. i'm also slightly upset about that as i'm currently working on some examples of module loading for V8/D8 and i would dislike if my work was misrepresented like this. |
@devsnek you are ignoring SpiderMonkey, powering GJS and others, where it's the exact same. I am telling you how engines that shipped ESM already work, and also how the browser work. If you don't want to listen, it's not me misrepresenting anything, it's you not wanting to listen, sorry. I think we have done with this discussion. |
@WebReflection how GJS resolves stuff isn't specific to spidermonkey, but it is a better example of your point. i wish you had brought it up earlier :) |
I think we've probably reached the end of this line. I think we can all agree that file-extension searching is a web/node interop issue. On the other hand, we need to balance that against the need to provide a smooth transition path to native ESM modules on node. We can debate how to balance those concerns later when we have more feedback. |
GJS uses js52 here and js52 behaves exactly the same. Swap
TL;DR I've used jsc 'cause I was on mac, not because it was anyhow more or less relevant to my point. Also browsers do the exact same, they don't guess, they don't crawl, they ask one thing only (that might be changed by the map but that's another story, I have nothing against the usage of the map)
AFAIK there is not much transition to smooth out since std esm already does that. This is what I mean when I say most developers here use bundlers or esm resolver and there's nothing to smooth out for them 'cause both bundlers and esm can guide developers to migrate start deprecating or warning in console without breaking. Tools should be there to help migrating, following what Node.js ships. This shouldn't be vice-versa, otherwise Node.js will be always doomed by tooling adoptions. Sure thing though, we need feedbacks, but I am worried people that use tooling will create problems that don't really exists (and they'll keep using tooling anyway). |
@WebReflection they don't use the cli debugger though... they're using their own implementation of resolution and evaluation which you can read through here: https://gitlab.gnome.org/GNOME/gjs/blob/master/gjs/module.cpp edit: i'm not even sure they're using ESM... if anyone is familiar with mozjs's api please let me know |
it was brought up already months ago in the mailing list, which is why I keep saying we should keep it simple. GJS has live bindings but a completely different module system which, AFAIK, is going to be integrated with ESM. Right now though, the import is limited and it will always add a MVP of ESM: explicit extensions, never ambiguous, faster to ship, less to discuss. You have loaders to solve everything else. How cool is that? 🎉 actually: loaders can help smoothing out migration !!! |
super cool if we didn't have 750,000 cjs modules that are still first-class citizens in the node ecosystem. if everyone has to use a loader (and they will have to use a loader) then why not make it the default... |
those can be brought in via However, loaders can solve that too 🎉
Nobody has to use a loader here. I've tested this and I didn't need any loader. Also my code wouldn't need any loader. Package maps, require when needed, pure ESM straight forward for everything else. Who's using ESM today is already behind bundlers or loaders, so they don't have to change anything 🎉 MVP
I see it this way:
That's it. But I also stop now, since like I've said there's not much else to add from me. Regards |
I've updated my MVP branch based on some comments in this thread (updating original post as well). Changes include
Repo: https://github.com/MylesBorins/node/tree/esm-kernel Download: https://nodejs.org/download/test/v11.0.0-test2018070976df5841a1/
|
@MylesBorins it looks like you dropped the need for the extension but you haven't put in the option to enforce ESM so that now we have ambiguity.
If I start from It is also impossible to bootstrap ESM via Considering I don't care about extensions, since I've described why it's a footgun to omit those but I like the moment I don't everything works as expected, would you be so kind to bring in the Thanks. |
Besides the above reasons, we need a CLI flag to enable ESM mode so that code like this works: node --module --eval 'import path from "path"; console.log(path.sep);' |
FWIW, other CLIs use Since other extensions don't really need any enforcement and have no differences between ESM env or not (i.e. BehaviorEverything evaluated as JS, or with a JS oriented extension (both |
Closing as this is quite similar to what we've ended up doing for the minimal kernel |
Hey All,
Like everyone here I've been thinking about this a whole bunch. For a while I've been trying to come up with a minimal implementation that we can iterate on. Something that could offer a fast path to deflagging.
Features
Try it out today
Repo: https://github.com/MylesBorins/node/tree/esm-kernel
Download: https://nodejs.org/download/test/v11.0.0-test2018070976df5841a1/
warning super naive implementation, lots of room for improvement... but it shows off the desired UX and all the tests pass locally on my machine.
Upstream PRs
TODO
Note
I have not opened a PR upstream to remove transparent interoperability as I want to be respectful of the current conversations going on within the group. While import.meta.require and limiting extensions are not guaranteed to land, I do think that they are worth discussing independently on their own merits.
The text was updated successfully, but these errors were encountered: