would you be open to a pure es module?

[mreinstein]

I'd be happy to send a PR, maybe something along the lines of https://nodejs.org/api/packages.html#packages_approach_1_use_an_es_module_wrapper

thoughts?

[nzakas]

Sorry, there's not enough info here for us to be able to evaluate what you're suggesting.

What is the change you want to make?

What value does making the change bring to the project?

What problems might be experienced with making the change?

[mreinstein]

Sorry, there's not enough info here for us to be able to evaluate what you're suggesting

Sorry for being overly brief; Happy to add more details. :)

What is the change you want to make?

I would like to have an es module version of espree, so that I can more easily import it directly from a web browser without the need for a build step. This is currently possible via skypack. e.g., I can do:

import espree from "https://cdn.skypack.dev/espree";

However this relies on some magic built into skypack's cdn which takes the commonjs module published on npm and does some transformation on it to expose an importable module.

What value does making the change bring to the project?

I think from my perspective it brings a few things:

• allows me to import espree directly from a web page, and from node
• prepares for a future where commonjs is deprecated in favor of esm, which should reduce the amount of build code that comes along with every module in npm land

What problems might be experienced with making the change?

I think the biggest question to anticipate is "will this impact people using espree from npm/node?". I don't think it should impact those use cases at all. There are 2 strategies that seem obvious to me as possibly viable:

• produce a "dual package" (the url I linked earlier) where we keep main pointing at a commonjs module and publish an es module with the exports field in package.json. If we want to keep supporting node older than v12.17 this is the safest approach. It won't break anything for existing node/npm users.
• drop nodejs support older than v12.17 and just produce an es module. bump the major package version of espree to indicate it's a breaking change. This would be a more dramatic change, but it would also mean less build stuff needing to happen in espree.

Maybe this is something that is more easily demonstrable with a PR. If you'd like to see what I mean I'm happy to submit one.

btw, thanks for a great module! espree has really enabled some great stuff in several of my prototypes!

[nzakas]

Thanks, that’s helpful. If we can ensure backwards compatibility, then I’m in favor.

Note: this would need to be in a major version bump.

@mdjermanovic @btmills thoughts?

[mdjermanovic]

• drop nodejs support older than v12.17 and just produce an es module. bump the major package version of espree to indicate it's a breaking change. This would be a more dramatic change, but it would also mean less build stuff needing to happen in espree.

This looks like a big change for consumers. How would, for example, ESLint load espree after this change?

[mreinstein]

This looks like a big change for consumers. How would, for example, ESLint load espree after this change?

eslint could either:

a) do the same thing, making the next major version of eslint node 12.17+ only
b) provide a dual package (basically the same strategy presented in #458 , this is what acorn does currently too.)

[mreinstein]

c) do nothing; if espree were to go pure es module in 8.x, eslint could stay on espree@7 for as long as it needed to

[mdjermanovic]

a) do the same thing, making the next major version of eslint node 12.17+ only
b) provide a dual package (basically the same strategy presented in #458 , this is what acorn does currently too.)

I don't think b) is possible for ESLint or any other consumer if Espree becomes an ES module only.

That goes for all other dependents and their dependents, they would all have to switch to ES modules if they want to use espree and/or eslint. We can't make such a breakage even in a major version.

c) do nothing; if espree were to go pure es module in 8.x, eslint could stay on espree@7 for as long as it needed to

ESLint can't stay on a fixed version of espree, so we'd have to maintain two espree versions. Seems much easier to provide a dual package?

[mreinstein]

I don't think b) is possible for ESLint or any other consumer if Espree becomes an ES module only.

in order to achieve b) one would have to introduce some build step in eslint@7 that takes espree@8 and converts it to cjs.

Personally given that node is about to stabilize the module interface, and also given that node v10 will move to unmaintained in less than 6 months (April 21') I don't think it's really drastic to do a). the current version of eslint will continue to work in all node versions (including v10.) At some point when a project relying on eslint wants to update to the latest shiny, they can do so when they're able to consume es modules directly.

That said, I realize this is my own personal opinion, and it is less conservative than having dual package support for however long one sees fit. The most inclusive approach (node version wise) is to publish dual packages for both espree and eslint.

Either way though, will be a nice step forward!

[mdjermanovic]

I don't think b) is possible for ESLint or any other consumer if Espree becomes an ES module only.

That goes for all other dependents and their dependents, they would all have to switch to ES modules if they want to use espree and/or eslint. We can't make such a breakage even in a major version.

Actually, dependents could use import(), but still looks like a big change for one major version.

in order to achieve b) one would have to introduce some build step in eslint@7 that takes espree@8 and converts it to cjs

That would be possible, but I think it isn't a common practice for Node tools/libs to bundle dependencies.

[mreinstein]

That would be possible, but I think it isn't a common practice for Node tools/libs to bundle dependencies.

yeahhhh it's a bit icky. making both of them dual package or both of them pure es modules are going to be the 2 most straightforward options.

[nzakas]

I think dual package is really the only viable choice here.

[mdjermanovic]

What is the change you want to make?

I would like to have an es module version of espree, so that I can more easily import it directly from a web browser without the need for a build step.

If I got the #458 proof of concept right, the published es modules are source code:

espree.js
lib/ast-node-types.js
lib/espree.js
lib/features.js
...

Does this make it possible to import espree directly from a web page, in particular what happens on import * as acorn from "acorn" in espree.js?

[mreinstein]

Does this make it possible to import espree directly from a web page

I've been using skypack's cdn, which basically produces a pure es module for every package in npm. They essentially do 2 things:

• convert each package into an es module
• modify it's imports to reference other modules on the cdn

This works today for espree in a browser: import espree from 'https://cdn.skypack.dev/espree'

If you open up that link directly in the address bar you can follow the urls and you'll see the modified version of espree is referencing the skypack cdn for it's dependencies (acorn, etc.)

So the stuff in #458 would basically eliminate 1 of the things skypack needs to do (the es module creation.)

[btmills]

Hi @mreinstein 👋 we agreed in yesterday's TSC meeting to go ahead with this! As a next step, can you put together an RFC? That will outline help the team understand the tradeoffs and impacts of the change. Thanks for working on this!

[mreinstein]

@btmills that's cool that eslint is ready to move forward with an es module. I already have a PR open for some other changes, would prefer to do one thing at a time. Can we start with getting #456 merged?

After this gets merged I'll have time to help with the RFC etc.

[nzakas]

@mreinstein we generally hold off on merging breaking changes until we can do them as a batch, just in case we need to do a minor release in the meantime.

So the goal for #456 should be to get the PR approved with no further changes needed rather than getting it merged.

[mreinstein]

we generally hold off on merging breaking changes until we can do them as a batch,

Thanks for the heads-up re: process. Is #456 essentially approved now? Shall I switch over to the RFC?

[nzakas]

Yup, looks like #456 is all set to go.

[mreinstein]

@ljharb you mentioned an interest in providing the initial package.json changes to ensure maximum compatibility for esm changes we've been discussing. I'm trying to submit a new, clean PR very soon for the esm stuff based on our group discussions in the RFC thread. Would you like to chime in with the snippet you have in mind here? I'll include it in my initial commit.

[ljharb]

@mreinstein sure! it depends on the desired entry points tho - do you have paths in mind?

[mreinstein]

Here's where we left off:

  "main": "dist/espree.cjs",
  "module": "espree.js",
  "exports": {
    "import": "espree.js",
    "require": "./dist/espree.cjs"
  }

And then your comment about possibly using an exports field, and other bits: eslint/rfcs#72 (comment)

so espree.js being the esm entry point, dist/espree.cjs being the commonjs entry point.

[mreinstein]

@mdjermanovic followed up with this comment, which has an updated snippet for package.json: eslint/rfcs#72 (comment)

[ljharb]

The “module” field is not a thing, so it should be omitted. One or two bundlers use it, but espree isn’t really used in web browsers afaict.

package.json is included so tooling doesn’t break; and type:module should not be used, so i fixed the file extensions (which is also needed for full back compat). The array fallback is needed for a few node versions that had an incomplete exports implementation, as is “default” instead of “require”.

"main": "dist/espree.js",
"exports": {
  ".": [
    {
      "import": "espree.mjs",
      "default": "./dist/espree.js"
    },
    "dist/espree.js"
  ],
  "./package.json": "./package.json"
}

[mreinstein]

why is ./package.json an export?

[ljharb]

So that tooling can access it, since otherwise there’s no reliable way to get to it from outside.

[mreinstein]

are json imports/exports actually supported in the esm standard?

In my proof-of-concept tooling I was working around this with a little hackery:

[ljharb]

JSON modules are stage 3, but the exports field restricts require as well, so any tooling using require to get at package.json breaks without that line.

[mreinstein]

While working through the tests, I've run into a snag. tests/lib/ecma-features.js and tests/lib/ecma-version.js take advantage of a require() specific feature, where we dynamically iterate over the the tests/fixtures/ directory and builds a pretty large set of tests.

Unfortunately, mocha doesn't natively support dynamic import(path) statements. The most common workaround seems to be invoking babel on these files, but I really loathe the idea of injecting a massive source code transformation step just for the sake of running some tests.

Some thoughts on other workarounds:

• write a little script to traverse all of the files within fixtures/ and write out all of them to explicit import statements at the top of the test file, and then reference those modules explicitly in the tests
• explore some other testing framework besides mocha which might actually support import() natively

Would be interested in hearing people's thoughts

[mreinstein]

for example, this works out of the box:

import * as espree from "../../espree.js";
import tap from "tap";


async function parseTest () {
    const a = await import("./tester.js");
    const tester = a.default;
    tap.equals(typeof tester.getRaw, "function");
}

parseTest()

[mreinstein]

I've done some more exploration, and tap actually has mocha compatibility too. This causes all of the tests to pass in tests/lib/ecma-features.js:

import tap from "tap";

tap.mochaGlobals();

it(`should parse correctly when ${feature} is ${isPermissive}`, async () => {
    config.ecmaFeatures[feature] = isPermissive;
    const expected = await import(`${path.resolve(__dirname, "../../", FIXTURES_DIR, filename)}.result.js`);
    tester.assertMatches(code, config, expected.default);
});

[mreinstein]

ok, the 2nd PR is ready. It is a monster. :o