Skip to content
This repository has been archived by the owner on Sep 2, 2023. It is now read-only.

ESM in .js files proposals #150

Closed
wants to merge 4 commits into from

Conversation

GeoffreyBooth
Copy link
Member

@GeoffreyBooth GeoffreyBooth commented Jul 11, 2018

Potential solutions for #149 and #151. Most take the approach of providing a way for the user to signal to Node that .js files should be parsed as ESM.

Readable view: https://github.com/GeoffreyBooth/modules/blob/esm-in-js-files/proposals/esm-in-js-files.md

To discuss particular proposals, please add comments as PR notes after those proposals. Add general comments to suggest new proposals or broader changes.

@WebReflection
Copy link
Member

FWIW, I am for Do nothing, enabling migration through bundlers and loaders.

Regardless, I think whatever decision will be taken, any new default should consider defaulting to the future of the language, instead of defaulting to the legacy behavior that will otherwise never go away.

michael-ciniawsky

This comment was marked as off-topic.

@michael-ciniawsky
Copy link

michael-ciniawsky commented Jul 11, 2018

Small code examples for each proposal (if appropriated) would be a nice to have (imho)

ljharb

This comment was marked as off-topic.

@GeoffreyBooth
Copy link
Member Author

Until we’ve first agreed on the constraints - on what an implementation should dom - then i find it unproductive to talk about potential solutions to an undefined problem.

I think the problem is pretty well defined in #149 and #151. The request is to find a way for Node to treat .js files as ESM JavaScript (not necessarily instead of treating .mjs files as ESM, but as another alternative). As for what an implementation should do, that’s part of the discussion in this thread.

@GeoffreyBooth GeoffreyBooth changed the title ESM in .js files solutions document initial version ESM in .js files proposals Jul 12, 2018
@ljharb
Copy link
Member

ljharb commented Jul 12, 2018

I mean, the ways that present an alternative are basically:

  • a command line flag, for an entry point or eval or stdin
  • something in-file that prevents parsing as a Script (this would involve dual parsing, however)
  • a manifest (package.json, a separate file, etc)

The first seems a given; the second seems not worth it; the third might work but doesn’t really support cases without package.json.

@michael-ciniawsky
Copy link

The first seems a given;

Yes, but node entry.mjs could/should still be consider

the second seems not worth it;

Yep

the third might work but doesn’t really support cases without package.json.

What do you precisely mean by that ? I mean neither

const bare = require('specifier')

nor

import bare from 'specifier'

would currently work without some kind of resolution algo providing the URL full somehow. Being it a package.json and/or a package-name-map.json or a conventional expanding/fallback by node or a webserver (e.g pre-/postfixes the bare specifier in a certain way) is something to discuss within the scope 'how to handle bare specifiers' aka packages in general.

@ljharb
Copy link
Member

ljharb commented Jul 12, 2018

@michael-ciniawsky I’m talking about one-off scripts in node, which need nothing more than node’s default resolution - having nothing to do with browsers.

@michael-ciniawsky
Copy link

What is a one-off script :) ? Could you to provide an example please ?

@ljharb
Copy link
Member

ljharb commented Jul 12, 2018

@michael-ciniawsky any single javascript file foo.js ran like node foo.

@GeoffreyBooth GeoffreyBooth dismissed ljharb’s stale review July 18, 2018 17:34

Responded in a comment. The review doesn’t request any particular revisions to this PR.

@GeoffreyBooth
Copy link
Member Author

I think today’s agenda is already pretty full, but can we add a mention to this? To invite the group to visit this page and comment if they haven’t already, and then at the next meeting we can discuss this.

ljharb

This comment was marked as off-topic.

@GeoffreyBooth
Copy link
Member Author

This PR isn’t meant to ever be merged in. I’m just using the PR structure to organize comment threads.

@GeoffreyBooth
Copy link
Member Author

FWIW, I am for Do nothing, enabling migration through bundlers and loaders.

I guess “do nothing” needs qualification, as in what it’s relative to. I was assuming it was relative to the --experimental-modules behavior, where “do nothing” means keeping that .mjs-centric implementation as the only option. I assume the above comment treated “do nothing” as relative to current unflagged ESM-less Node? So “do nothing” as in don’t allow CommonJS imports via import statements, and enable CJS-into-ESM import interoperability only via bundlers and loaders? Because that’s already its own option.

I think whatever decision will be taken, any new default should consider defaulting to the future of the language, instead of defaulting to the legacy behavior that will otherwise never go away.

This is a strong argument for why import shouldn’t support CommonJS at all, and people would need to use something else (import.meta.require, createRequireFunction etc.). to bring in CommonJS files. Once we allow import to pull in CommonJS, especially as the default for .js files, we’ll be stuck with it just like we’re stuck with node file.js always running file.js as CommonJS (unless we want to break backward compatibility, which I assume we don’t).

@ljharb
Copy link
Member

ljharb commented Jul 19, 2018

The future of the language is ESM supporting everything, which includes commonJS.

@GeoffreyBooth
Copy link
Member Author

Added createRequireFunction per #150 (comment)

@GeoffreyBooth
Copy link
Member Author

I mean, the ways that present an alternative are basically:

  • a command line flag, for an entry point or eval or stdin
  • something in-file that prevents parsing as a Script (this would involve dual parsing, however)
  • a manifest (package.json, a separate file, etc)

The first seems a given; the second seems not worth it; the third might work but doesn’t really support cases without package.json.

I think this is a good way to frame the options. We could also add “a file extension” as a group of solutions to cover both .mjs and .cjs. Basically it seems to me that no single solution covers all the cases we’re trying to support:

  • command-line flag: the only option for non-file-based input, so we have to have it; but only affects entry points, so we need additional APIs to enable interoperability
  • file extension such as .mjs: covers a lot of cases, but not build chains that don’t know or support the parse goal (e.g. converting something to a .js or .mjs file) or users who need/want to use .js to host ESM for use in browsers (the Web compatibility and ESM in .js files #149 and Feature: ESM in .js files #151 issues that started this thread)
  • a manifest: also covers a lot of cases, but not ESM and CommonJS files side by side in the same folder, or a bare file (node file.js) without a manifest nearby (and without a flag)
  • something in-file: actually, I think maybe this could cover all cases aside from the “I want to know the parse goal just from the filename” case, which could always be achieved informally (e.g. name your files .m.js or something). As @ljharb mentioned, this involves a double parse, so there’s a slight performance hit; and there’s the difficulty in designing an algorithm for this that doesn’t break in the future (e.g. if import() comes to CommonJS). This is discussed so rarely though I feel like there must be use cases it doesn’t support that are escaping my mind at the moment.

So either “something in-file,” whether that’s "use module" or one form of unambiguous syntax, can actually be made to work (or someone please remind me of why it’s not a magic bullet even if it could be made to work)—or we need at least one option from each of the other three categories. So one such implementation could be --experimental-modules with its .mjs support plus the --mode flag plus the mimes field in package.json. There are lots of other variations, but the bottom line seems to be that we need a mix of at least three.

I could group the options into these categories, and then the effort will be finding consensus on which solution(s) within each category we want to pursue.

@GeoffreyBooth
Copy link
Member Author

I see a lot of personal attacks and snide comments on this thread. Frankly it’s making me less inclined to participate, and I’m probably not the only one. Can we please tone it down?

Also can we please also keep the discussion on this thread focused on the PR here? I know a lot of these comments are at least somewhat related because they discuss implementations, but this thread is already over 100 comments and it’s getting impossible to follow the discussion of this actual PR. Could these alternate implementations you all are discussing please get their own threads? This one’s mine! 😉

Getting back to my comment from only 21 hours and 89 comments ago, can someone (like @bmeck) please explain whether unambiguous grammar can or can’t work? If it can work, it seems like the silver bullet solution that solves all (?) use cases. So I’m assuming it can’t work. But I’d like to understand why. Maybe that explanation can get its own thread too, in case there’s any dispute over whether it can or can’t work.

Aside from that, I don’t think there’s much reason to argue: it seems like we can agree that there’s at least desire, perhaps even a need, for Node to ship support for both ESM via .mjs and ESM via .js, somehow. I say we bank that consensus and move on, and build on it. Please don’t try to get the last word against me here and claim that your side or the other is the necessary one or the better one or the one true solution that we all should accept. We’re in agreement, and for the sake of consensus building we should just leave it at that.

What I would like us to discuss on this thread (and please feel free to continue the other discussions in other issues) is which of the proposals in the PR people find most promising and would like to pursue, for how to tell Node to treat .js files as ESM (assuming it’s opt in; please leave for later the question of whether it could or should be opt out or default). They break down into these categories:

  • Alternate API for CommonJS into ESM, where consumer determines module type
    • import.meta.require
    • import { importCommonJS } from 'module'
    • createRequireFunction
  • File extensions
    • .cjs always signals CommonJS
  • Manifests
    • package.json flag to enable ESM for all .js files within a package boundary
    • package.json section to define MIME types for any file extension within a package boundary
  • Source text
    • Unambiguous grammar, where ambiguous cases are assumed to be ESM
    • Unambiguous grammar, where ambiguous cases are assumed to be CommonJS
    • "use module" pragma
  • Loaders
    • Loader is required for an import statement to import CommonJS
    • Loaders are required for import statements to work at all for anything

People have raised general objections about creating an alternate API like import.meta.require or createRequireFunction, so we don’t have consensus to pursue those. I think .cjs is something we might as well add support for because it pairs with .mjs and gives people an option to truly enable unambiguous files at all times if they want to, and doesn’t conflict with anything else. The manifest options seem the most promising to the most people, and the latter one (defining MIME types) is more powerful, covering more use cases including future cases like binary AST etc., so that seems to me like a very promising solution to pursue. Source text-based solutions I’m assuming are too good to be true, so someone please validate or correct me. And loaders will be part of our implementation, but it seems from our discussion that people consider this “how to configure Node’s handling of file extensions” to be important enough and core enough to support directly rather than outsourcing to loaders.

So that leaves us with --experimental-modules plus:

  • A CLI flag
  • .cjs is always CommonJS (mirror of .mjs always being ESM)
  • package.json section where users can configure within a package boundary how various file extensions are parsed, using MIME types as a way of defining Node’s behavior

That’s my take on things, anyone else want to propose a different set of solutions?

@michael-ciniawsky
Copy link

michael-ciniawsky commented Jul 20, 2018

Alternate API for CommonJS into ESM, where consumer determines module type

createRequire (A factory {Function} adds the least baggage in terms of maintenance and future legacy)

Alternate API for CommonJS into ESM, where consumer determines module type

👎 on .cjs
👌 .mjs as an alternative to .js for authoring ESM with the benefits it brings for authors

No transparent interop via import/import() for CJS

Manifests

👍 pkg.module for explicit package entries, analog to pkg.main

👎 pkg.mode (Relying on a package boundary won't support dual-mode packages)
👎 MIME Types (MIME isn't currently solving the main issue about which parsing goal a JS file should be interpreted with and otherwise the file extension is sufficient for node (e.g for WASM <=> JS))

Source text

👎 (Not needed)

Loaders

👎 ("node is not a build tool", fragmentation, high risk to be 'abused')

CLI

👍 --module flag to explicitly set the parse goal for an entrypoint

Resolving

👍 Removal of file extension and directory resolving for ESM

I apologize in advance in case any of my comments attacked someone personally, which was never intended. Since I'm not a member of the node modules group I will try to refrain from further commenting on any threads to reduce the noise

@WebReflection
Copy link
Member

WebReflection commented Jul 20, 2018

Apologies @GeoffreyBooth, I agree this went out of hands but at the same time I couldn't stand reading always the same answers to what should've been a MUST HAVE since day one.

I'll try to not interfere but since it's somehow too late, I'd like at least to clarify one bit that everyone apparently understood, except for @devsnek:

andrea's proposal says that anything imported is ESM...

@devsnek my proposal is all about disambiguation with the .js extension, and it's about loading either .mjs or .js files accordingly to the flow (when it comes to .js).

My proposal was in topic: how to load .js as ESM.

My proposal has nothing to do with .json or .wasm since these don't need any disambiguation (although one might discuss in the future how to signal .wasm and I woldn't personally mind having a --wasm flag).

After all, the reality for JS as language is that ESM is the module system and .wasm is the "binary format" for such module system. There won't be other formats in the future, beside new versions of .wasm, because no other format is needed once you have your main language, and any foreign language as was or transpiled, allowed.

So please let's not force things over my proposal, thanks.

Best Regards


Edit I have updated the proposal explicitly putting .js where appropriate, and I second @robpalme who also perfectly understood (and summarized) my proposal #150 (comment)

@demurgos
Copy link

demurgos commented Jul 20, 2018

GeoffreyBooth:
Getting back to my comment from only 21 hours and 89 comments ago, can someone (like @bmeck) please explain whether unambiguous grammar can or can’t work? If it can work, it seems like the silver bullet solution that solves all (?) use cases. So I’m assuming it can’t work. But I’d like to understand why. Maybe that explanation can get its own thread too, in case there’s any dispute over whether it can or can’t work.

@GeoffreyBooth
You may be interested in this comment by @bmeck. It lists all the cases where a parse error can be used to disambiguate and what are the runtime differences for the ambiguous cases. It does not list it but Module also implies strict mode.
I consider unambiguous grammar and the "use module" pragma to be definitely rejected. I agree that a dedicated issue may be worth opening. I don't have time to lookup all the arguments and references, but here's why:

  • Ambiguous files already exist and are used. A common use case is importing a file to get a side effect at evaluation (ex. polyfill): these files may not use require, import, export or any other disambiguation mechanism.
  • A proposed disambiguation mechanism is to force the addition of the noop export {};. Similarly to "use module" it creates a future where using ESM has a "cost" compared to CJS (you have to add some boilerplate to each file).
  • It's easy to accidentally change the parse goal. A commit adding a single export at the end of a file suddenly changes the parse goal of the whole file. The changes are often subtle and hard to spot.
  • Accidentally changing the parse goal can also happen when using tools unaware of the issue. For example, a concatenation tool may join files with different parse goals, causing the whole file to be treated as Module (even the parts coming from a Script file).
  • Unambiguous grammar involves double parsing which has a perf hit.
  • TC39 rejected both unambiguous grammar and "use module". (1, 2)

Unambiguous grammar solves many cases, but when it does not work it's a dangerous footgun.

@WebReflection
Copy link
Member

WebReflection commented Jul 20, 2018

@demurgos is there any real-world example of a file considered ambiguous that could have side effects if loaded as ESM instead of CJS ?

The only ambiguous thing that comes to my mind is the potentially "use strict" directive but it's straight forward to educate polyfills authors (delivered through modules) to explicitly set that directive.

It's also worth noticing that polyfills are always to patch older environments so they all use "use strict", when needed, to be sure the behavior is preserved.

TL;DR I don't think polyfills are a valid argument 'cause I wrote dozen of them and all of them would work in any environments, strict or not, ESM or CJS or Browser => aka: if there's no need to disambiguate it's because the code would run just fine no matter where.

@demurgos
Copy link

demurgos commented Jul 20, 2018

I avoid polyfills so I am not aware if one of them exhibits this behavior. I assume that most popular polyfills will be written by skilled people and effectively behave the same (regardless of the host or parse goal), just as your polyfills.
The safety principle would still recommend to assume that if the language permits it, someone somewhere will have a fragile script relying on this behavior: there are many less popular or private scripts.

I listed polyfills as an example, but @bmeck's comment had a larger list of use cases where issues may arise:

  • Mutation of Globals
    • Polyfills
  • Usage of Globals
    • Configuration of globals (source-map-support, some config scripts)
    • JSONP-like calling of globals
  • Prefetching (wants to be unordered parallel so uses import())
  • Init scripts (CLI etc.)
  • REPL like text (either for local testing or things like vm.runInContext)
    • Anything really that uses the completion value of the text

If someone has an example of script with differing behavior (not necessarily polyfill), please post it.

Besides, my other points remain valid.

@WebReflection
Copy link
Member

I have summarized my proposal, the history behind, and what it solves and how in this gist:
https://gist.github.com/WebReflection/a4e026e78af45ede8d0f7498cab44f91

Quickly back to @demurgos, my proposal has only a single point of potential ambiguity, the initial file, in case it's not an .mjs one, and in case the --module flag, or --type=module web-like, hasn't been specified.

In a generic node file.js as entry point, we could default to CJS but we could also parse that only file once in search of the usage of the static import which, if found, would automatically set the entry point parse goal as ESM.

An entry point won't ever be a polyfill, it makes no sense, neither any of the other kind of files in that list.

A CLI can use the --module flag, .mjs or again, use the parsing goal resolution only once and only for the entry point which will default to CJS.

At that point, if interop is dropped and there is an initial parsing goal, everything else is covered by my proposal, and better explained in the gist.

  • authors are able to define the parsing goal
  • users are forced to know authors parsing goal (requiring an ESM only module would fail, as well as importing a CJS only one)
  • dual packages are allowed and straight forward
  • tools can easily update (most of them won't need to do anything)
  • everyone can migrate to a better future of ESM only

I hope my point, and my proposal, is now clear.

Please comment in there to avoid keep polluting this thread.

Thank you.

@bmeck
Copy link
Member

bmeck commented Jul 20, 2018 via email

@GeoffreyBooth
Copy link
Member Author

Getting back to this PR, I’m realizing from @michael-ciniawsky’s comment above that the options I’ve listed in this PR take experimental-modules as a baseline. The proposals in this PR are all additions to the code currently in Node behind --experimental-modules. And that’s fine; I think adding one or more of these proposals will make --experimental-modules handle more use cases, and therefore will make it better.

What @michael-ciniawsky seems to be saying, if I understand his comment correctly, is that an alternate implementation could use a different mix of strategies for handling CommonJS and ESM interoperability. Which is also fine; I’m sure that @michael-ciniawsky and @WebReflection would argue that an alternate implementation would be better, and that’s also a fine debate to have (though please, not in this thread! 😄).

Getting back to this PR though, and improvements to --experimental-modules, I’m realizing that I don’t need to call out .cjs as a separate thing; if we add package.json mimes, users can add ".cjs": "application/node" in the mimes block. Node will need to ship with default extension/MIME mappings, and perhaps this .cjs mapping could be part of the default set, but that can be discussed later as part of the implementation of mimes. As could the name itself, in case the group thinks of a better name for mimes; and all the other implementation details. My goal here is to find consensus on what functionality to add, not necessarily exactly what said functionality’s API or naming should be. As long as there’s some way for users to specify parse goals for file extensions, whether that’s using the MIME type as a metaphor or something else, that’s what the mimes proposal means.

So I think we can boil this down to just two additions to --experimental-modules:

  • A command line flag to enable ESM for an entry point
  • A package.json section where users can specify mappings of file extensions to MIME types/parse goals, to apply to all files within that package boundary

Is this something we can find consensus on, at least with regard to --experimental-modules? What I’m thinking is that we can add these to that implementation, and that doesn’t mean anything about any other implementations that anyone else is working on; they can continue to do their own things, and grow and experiment, and down the line we can debate whether one approach is better than another or what parts from each we want to mix and match into new implementations. But for now, with --experimental-modules more or less having privileged status as our baseline implementation, I’d like to improve it as much as we can whether or not we eventually choose something else.

@devsnek
Copy link
Member

devsnek commented Jul 22, 2018

@GeoffreyBooth iirc there's been a pr against core for --mode since before the moratorium and imo hooking resolve from package.json loaders (I think bradley was planning that) is better than just overriding the file type statically.

mode flag pr: nodejs/node#18392

edit: pr to core was for mode in package.json, not a flag sorry for confusion

@GeoffreyBooth
Copy link
Member Author

iirc there’s been a pr against core for --mode since before the moratorium

Great! Can you find a link? I can’t seem to find it.

hooking resolve from package.json loaders (I think bradley was planning that) is better than just overriding the file type statically.

I think this is an implementation discussion. We could open a new thread for “okay, so how should we let users tell Node what parse goals to use for particular file extensions?” and debate the various methods. @bmeck also did some work sketching out a mimes field before I rediscovered the idea here: https://gist.github.com/bmeck/7ee7eb2147e2dafe3167c856d9b4151a

@ljharb
Copy link
Member

ljharb commented Jul 22, 2018

@GeoffreyBooth

So I think we can boil this down to just two additions to --experimental-modules:

  • A command line flag to enable ESM for an entry point
  • A package.json section where users can specify mappings of file extensions to MIME types/parse goals, to apply to all files within that package boundary

My personal preference would be a mapping from extension to extension (ie, from .js to .mjs, so that .js files are interpreted as if they were .mjs files) - and then I'd want the command line flag to mirror that, so that it's something like "pretend this file has .mjs even though it doesn't" or "pretend this file has .wasm even though it doesn't".

Regardless of the specifics, this does seem like something we'll probably be able to come to a consensus on - as long as we don't derail it by trying to discuss changing the defaults. Thanks for the concise summary and attempt to get the thread back on track <3

@GeoffreyBooth GeoffreyBooth added the modules-agenda To be discussed in a meeting label Jul 22, 2018
@WebReflection
Copy link
Member

WebReflection commented Jul 22, 2018

as long as we don't derail it by trying to discuss changing the defaults

without derailing this conversation, the fact an experimental flag has shipped without consensus, and NodeJS is now stuck around that experiment, should hopefully mark a "mistake to learn from" in the history of NodeJS.

I personally wouldn't call a flag "experimental" without the will to change it later on 'cause I already worked on it too much.

Regards.

@ljharb
Copy link
Member

ljharb commented Jul 22, 2018

@WebReflection it absolutely had consensus, from prior to the module group being formed.

@WebReflection
Copy link
Member

@ljharb then at this point I don't even know what's the purpose of this group if nothing behind that experimental flag could change. Anyway, I've written my proposal, I have nothing else to add in here.

@MylesBorins
Copy link
Member

I'm going to remove this from the agenda until we can reach consensus on a minimal kernel / approach for the fork. Please feel free to re add or let me know if this should be on today's agenda

@GeoffreyBooth
Copy link
Member Author

Superseded by #160

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet