Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

esm: Implement package.json "esm": true flag for ES modules in ".js" files #18156

Closed
wants to merge 1 commit into from

Conversation

guybedford
Copy link
Contributor

@guybedford guybedford commented Jan 15, 2018

As previous proposed and discussed at nodejs/node-eps#60, this implements a package.json lookup process for the module format so that adding "esm": true to the package.json enables ".js" files as ES modules.

There has been some discussion as well here as to whether this property should be a string to allow for extensibility in future, which is also still ongoing.

The ideal workflow imagined would be one where the package manager init process itself adds this into the package.json so that it becomes an invisible part of the authoring process, not completely dissimilar to the <!doctype> in html.

The cost of this approach is loading the base-level package.json for all module loads (instead of just for mains). The argument here is that there is already a lot of statting in the module lookup process, and since this shares the package.json cache with the mains, it adds around one extra stat on average for subfolder package lookups per package loaded. (Compare to adding ".mjs", which has added an extra stat per lookup path checked, an order of magnitude more in cost here).

The benefit of this approach is end-users never need to worry about the authoring format of a package they are importing, build tools and bundlers won't need to adapt to a separate linking mechanism for CommonJS, and that NodeJS ES modules wouldn't need to themselves be built for the browser.

The test cases here cover nesting, invalid values and subpaths. A package.json without an "esm": true property, or an invalid "esm" property is treated as if it contained "esm": false (ie ".js" files as CommonJS modules).

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

esmodules

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jan 15, 2018
@targos
Copy link
Member

targos commented Jan 15, 2018

How would you recommend to write a hybrid package with this approach?
I mean: author the package in ESM without using .mjs, transpile and publish so that users can consume it both from ESM code with import and CJS code (in older Node versions) with require().

@guybedford
Copy link
Contributor Author

@targos a package.json file can be created in the dist subfolder to override a subfolder to be interpreted as either CommonJS or es modules.

@targos
Copy link
Member

targos commented Jan 15, 2018

@guybedford Yes, but how would you redirect to the right subfolder? There is only one possible "main" entry.

@guybedford
Copy link
Contributor Author

@targos ah I see what you mean. Hybrid packages effectively rely on conditional mapping applying on only in the ES module case. There are various package.json metadata approaches that could work for this, and I think are a somewhat orthogonal consideration to this technique. That said, they would build on this same principle of using package.json meta for all module loads, so this would lay the same groundwork.

@dnalborczyk
Copy link
Contributor

dnalborczyk commented Jan 15, 2018

@targos you could also tell the user to require said module explicitly from e.g. /dist-cjs, if cjs module is being preferred, or if esm is not an option.

@devsnek
Copy link
Member

devsnek commented Jan 15, 2018

wouldn't this just end up making modules published that a lot of people can't use? IMO setting main to a file without an extension is still a better solution. an fs stat isn't really that evil.

@bmeck
Copy link
Member

bmeck commented Jan 15, 2018

Now that our loaders have had some time to brew and be tested against situations. I'm partial to adding a way to set the format of URLs. This per package isolation seems good to me, though I would prefer a tweak on implementation.

I think with several other modes being talked about:

And some often overlooked:

  • Script
  • REPL

We can move to a MIME based approach. In addition this would prevent the Loader APIs from going out of sync with other APIs that might make sense to come to Node like Blob or File (cc: @jasnell ).

This allows ".js" files to be loaded as es modules whenever the parent
package.json file contains "esm": true. Original proposal and discussion
at nodejs/node-eps/pull/60
@bmeck
Copy link
Member

bmeck commented Jan 18, 2018

CC @jasnell @MylesBorins @devsnek

@bmeck
Copy link
Member

bmeck commented Jan 18, 2018

I have a counter proposal to use MIMEs to achieve this in https://gist.github.com/bmeck/7ee7eb2147e2dafe3167c856d9b4151a ; I think having an alias like "mode"/"type"/etc. for that meaning is fine but would like to use a string and not a boolean in all cases since I am not convinced of it only applying to esm, including the alias.

@jasnell
Copy link
Member

jasnell commented Jan 18, 2018

No specific thoughts on this just yet. Need to work through the cases a bit more. The specific thing I'm struggling with is what the value should be on here.

Btw, the reason @bmeck mentions Blob/File here is because I'm specifically exploring implementing support for both within Node.js core, which would also bring along some basic MIME support.

@arackaf
Copy link

arackaf commented Jan 20, 2018

@bmeck - does your MIME solution do any better with respect to hybrid packages, or would either have to revert to either .mjs or @std/esm?

@bmeck
Copy link
Member

bmeck commented Jan 20, 2018

@arackaf it has no difference than "esm": true for hybrid packages; it is about what Loaders can see and what happens if more parse goals like the multi-module goal land (on TC39 January agenda). Hybrid packages can use nested package.json files to change how .js is treated within directories. However to have the same specifier resolve in both MIMEs usage of .mjs is still the way to go.

@arackaf
Copy link

arackaf commented Jan 20, 2018

As I expected, thank you @bmeck.

So it sounds like your MIME proposal would be as transparent and automatic for end users, but would allow for greater extensibility in userland? Is that right? If so, what sort of use cases would be possible? Something like compiling typescript files when loaded, on startup?

@bmeck
Copy link
Member

bmeck commented Jan 20, 2018 via email

@guybedford guybedford mentioned this pull request Jan 26, 2018
4 tasks
@guybedford
Copy link
Contributor Author

guybedford commented Jan 26, 2018

This approach has been superseded by #18392.

@guybedford guybedford closed this Jan 26, 2018
@guybedford
Copy link
Contributor Author

Correction: #18392

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants