ava icon indicating copy to clipboard operation
ava copied to clipboard

The ESM Plan

Open novemberborn opened this issue 5 years ago • 22 comments

Here's what we're doing to support ESM in AVA. The work has already been broken up into individual issues that you can work on. See the ESM support project for details.

See https://nodejs.org/docs/latest/api/esm.html for the latest on what Node.js is implementing (this issue was opened when that documentation was for Node.js 13.1.0). I'm expecting this to become available without a flag at some point.

  • [x] Firstly, we're moving AVA's Babel support into a separate package that you can install alongside AVA. This will reduce confusion as to what module format AVA supports out of the box. It also allows us to evolve that package independently, which is important since loader hooks may still take some while to land.

  • [x] Secondly, we'll be detecting test files with cjs and mjs extensions. Currently, by default, we only look for files with js extensions.

  • [x] When loading test files, we'll always load files with cjs extensions using require(). Files with mjs extensions will be loaded using import(). https://github.com/avajs/ava/issues/2344

  • [x] Note that import() itself is currently behind an experimental flag. https://github.com/avajs/ava/pull/2272 will let you configure that flag. We'll print a useful error message if import() is not available and AVA is trying to load an mjs test file.

  • [x] When it comes to js files we'll follow the package.json "type" field. So by default we'll treat them as CJS.

  • [x] We'll also let you configure how js files (and other files, but not cjs or mjs files) should be loaded. This way you could override the "type" field, or load test files as ESM even if you're publishing CJS. https://github.com/avajs/ava/issues/2345

  • [x] The Babel provider will detect when you've configured files to be loaded as ESM even though it cannot load them as such, and print an error.

  • [x] We'll support ava.config.cjs and ava.config.mjs configuration files. However deciding what to do with ava.config.js files is more difficult. We shouldn't change how it's loaded based on "type" fields, nor can we use the built-in import() since it needs to be enabled. And finally, right now we need to be able to load it synchronously for use with our ESLint plugin. https://github.com/avajs/ava/issues/2346

  • [x] I'm proposing we remove support for import statements and require() calls from ava.config.js files. In other words, they must only contain export default {} or export default () => ({}) style configurations. This is a breaking change, as at the moment we load ava.config.js using the esm package. However it gives us the best upgrade path.

  • [x] We do need to consider what syntax we use in our documentation. I think we should use CJS, until ESM is available without a flag. It's not as hip, but at least it'll work out of the box.

  • [x] When loading require modules, if they are loaded from the project itself we can use the above logic to load the files based on their extension. If they're packages and import() is available then that should work. Else, for now, we'll fall back to using require(). https://github.com/avajs/ava/issues/2347

  • [ ] Dependency tracking for our watch mode also needs work. https://github.com/avajs/ava/issues/2388

novemberborn avatar Nov 10 '19 16:11 novemberborn

We shouldn't change how it's loaded based on "type" fields

Why not?

sindresorhus avatar Nov 10 '19 16:11 sindresorhus

We shouldn't change how it's loaded based on "type" fields

Why not?

First of, Node.js defaults to CJS for js files. Following this default breaks all existing configuration files. With my suggestion to only support export default most simple files will still work.

Changing the "type" field would have the consequence of breaking your AVA configuration, which I'd find surprising.

Finally, if we say you can override how js files are loaded, through configuration, it's confusing that you can't change how the ava.config.js file itself is loaded.

novemberborn avatar Nov 11 '19 08:11 novemberborn

I've also just realized our dependency tracking (for the watcher) won't work for ESM. Perhaps we can use the loader plugins, or else we should try and do static analysis. The latter is complicated by new JavaScript & TypeScript syntaxes.

novemberborn avatar Nov 11 '19 08:11 novemberborn

IMO, why don't we support Node experimental modules under a flag. For example:

ava --experimental-modules

So that it won't break the current behavior and we can switch to the new modules system when the interoperability and the loader hooks are stable enough.

clitetailor avatar Dec 08 '19 05:12 clitetailor

IMO, why don't we support Node experimental modules under a flag. For example:

ava --experimental-modules

So that it won't break the current behavior and we can switch to the new modules system when the interoperability and the loader hooks are stable enough.

#2272 will add a nicer way of defining the Node.js flags applied to the worker processes than what is currently possible. However, ESM is available already without a flag in Node.js 13. We just need to make a bunch of changes to make effective use of it. And yes it would be opt-in, but that's largely due to how the feature is designed in Node.js itself.

novemberborn avatar Dec 08 '19 16:12 novemberborn

It appears that AVA can't be used on Node 13 with type: module in package.json.

Any suggestions? index.js has an export default in it so it's definitely ESM.

Should I open a new issue?

❯ node -v
v13.2.0

Without any ava config

❯ ava

⠦ (node:64001) Warning: require() of ES modules is not supported.
require() of ./index.js from ./test/index.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename ./index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from ./package.json.

  ✖ No tests found in test/index.js

  1 uncaught exception

  Uncaught exception in test/index.js

  ./test/index.js:4

   3: import {compressToEncodedURIComponent as compress} from 'lz-string';
   4: import OptionsSync from '..';                                       
   5:                                                                     

  Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: ./index.js

With require: esm

This configuration worked up to v12

  "ava": {
    "require": [
      "esm"
    ]
  }
❯ ava

⠧ (node:63214) Warning: require() of ES modules is not supported.
require() of ./index.js from ./test/index.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename ./index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from ./package.json.

  ✖ No tests found in test/index.js

  1 uncaught exception

  Uncaught exception in test/index.js


  Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: ./index.js

fregante avatar Dec 11 '19 07:12 fregante

@fregante,

It appears that AVA can't be used on Node 13 with type: module in package.json.

Yup. This issue sets out our approach to making it work.

novemberborn avatar Dec 11 '19 09:12 novemberborn

I have made all the breaking changes needed for this as part of our v3 release. However we haven't yet added any actual support for ESM files. Please see https://github.com/orgs/avajs/projects/2 for the issues where you can help out.

novemberborn avatar Jan 05 '20 15:01 novemberborn

Have you considered using the esm package ? Native ESM module with node is fine, but when you start mixing with old dependencies or use an older version of node, you will quickly bump into problems.

I have been using esm with my test runner for some time now and I am very happy.

You don't even need to ship it as a dependency, if ava allows require hooks like node does you can pass it as a command argument.

ava -r esm some/test.js

lorenzofox3 avatar Jan 25 '20 17:01 lorenzofox3

Yes that’s been supported for a long time. You can configure the required modules in AVA’s configuration and well then use it to load all subsequent files.

It’s only a stop gap solution though.

novemberborn avatar Jan 25 '20 20:01 novemberborn

esm is no longer enough in Node 13, at least until this is fixed https://github.com/standard-things/esm/issues/855

fregante avatar Jan 25 '20 22:01 fregante

All the necessary breaking changes have already gone out in AVA 3.0. The remaining work has been broken up into individual issues that can be worked on. See the ESM support project for details.

novemberborn avatar Jan 26 '20 14:01 novemberborn

Any current recipes? My entire codebase uses import statements etc., but I cannot seem to quickly figure out how it's suppose to work (running Node 14.6). My package.json has "type": "module", but module resolution doesn't seem to work inside tests.

haywirez avatar Aug 25 '20 16:08 haywirez

@haywirez I think that should work fine. Perhaps you could open a new issue and include additional details, like your AVA version, and a reproduction?

novemberborn avatar Aug 26 '20 08:08 novemberborn

@novemberborn would you consider adding support for a --loader option to the new ava?

In order to use this esmock package, for example, one uses something like this inside package.json

  "scripts" : {
    "unit-test": "ava --node-arguments=\"--loader=esmock\""
  }

If a --loader option were supported something like this might work

  "scripts" : {
    "unit-test": "ava --loader=esmock"
  }

iambumblehead avatar Apr 11 '21 19:04 iambumblehead

@iambumblehead as far as I know, loaders are still experimental: https://nodejs.org/api/esm.html#esm_loaders

When that changes we can consider how best to expose it in AVA.

novemberborn avatar Apr 12 '21 08:04 novemberborn

Using type: module or .mjs prevents agents like newrelic from instrumenting code or sinon from mocking and jest doesn't support modules fully either, see their ecmascript-modules page. The esm library does not yet support optional chaining (elvis operator). The esm-wallaby library which is a fork of esm does. It's disappointing that module support hasn't addressed these issues and we can't move to type:module, .mjs or ava 4.x until these are resolved. For that reason I'm not updating libraries I maintain to type: module or .mjs until these issues are resolved. Actually I did update them to type:module and .mjs, but backed all the changes out after discovering the issues and no workarounds. Consumer of my libs can use type:module, .mjs, or use esm, esm-wallaby, or some other shim (lots of options). They aren't blocked from updating like ava 4.x currently.

tcollinsworth avatar Jan 22 '22 16:01 tcollinsworth

@tcollinsworth AVA 4 works just fine with CJS. Being a Node.js test runner, for ESM support we rely on Node.js' standard behavior.

novemberborn avatar Jan 22 '22 16:01 novemberborn

@tcollinsworth stop using non-native esm and stop using tools that don't support esm. The only tool you listed that doesn't have a native-esm replacement is newrelic https://github.com/newrelic/node-newrelic/issues/553

If you have control of tools you are using, I would recommending dropping newrelic from your setup because using native esm simplifies so many other things within a package -- smaller download/install size, better stack traces, less configuration and dot files etc. Using native esm makes everything easier.

iambumblehead avatar Jan 22 '22 19:01 iambumblehead

@iambumblehead any suggestions for an esm alternative to newrelic?

drmrbrewer avatar Jan 26 '22 21:01 drmrbrewer

@drmrbrewer I'm not a newrelic user and don't know of any suggestions (sorry)

iambumblehead avatar Jan 26 '22 22:01 iambumblehead

@drmrbrewer check out https://github.com/getsentry

Primajin avatar Jan 27 '22 05:01 Primajin

With https://github.com/avajs/ava/pull/3218 watch mode now does dependency tracking of ES modules.

novemberborn avatar Jul 03 '23 14:07 novemberborn