Switch to ESM for internals #2741

novemberborn · 2021-04-26T15:46:53Z

Make sure the entrypoints remain usable in a CJS environment
Convert all other internal files to ESM

novemberborn · 2021-05-02T18:08:15Z

Progress so far:

lib/ is ESM
- I had to load yargs as CJS since it behaved differently
- Line number parsing code is still CJS because it attempts to load some dependencies lazily and I didn't want to make it asynchronous
Shared worker filenames must now be file:// URLs, but you can provide a URL instance
Shared workers are always loaded through a dynamic import
test/ is ESM, except for the tests that would have required top-level await or too much refactoring
The XO config is now a JSON file
I'm enforcing import ordering and formatting beyond what XO does
I'm hoping code coverage isn't decreased
test-tap/ still needs migrating

novemberborn · 2021-05-02T18:09:10Z

plugin.d.ts

@@ -1,3 +1,5 @@
+import {URL} from 'url';


@sindresorhus is this idomatic?

No, but TS forces us to do it: DefinitelyTyped/DefinitelyTyped#34960

novemberborn · 2021-05-02T18:09:55Z

@sindresorhus feedback welcome, though I'm not expecting you to review each file 😉 To be honest neither am I, as long as the tests pass I think we'll be OK.

sindresorhus · 2021-05-02T18:29:18Z

lib/code-excerpt.js

-const codeExcerpt = require('code-excerpt');
-const truncate = require('cli-truncate');
-const chalk = require('./chalk').get();
+import fs from 'fs';


Suggested change

import fs from 'fs';

import fs from 'node:fs';

Any thoughts on switching to this while you're at it? I plan to enforce it in XO soon.

nodejs/node#38343

I'd be happy to let XO fix & enforce that when the time comes 😄

lib/provider-manager.js

sindresorhus · 2021-05-02T18:35:29Z

test-d/macros.ts

@@ -1,5 +1,6 @@
 import {expectType} from 'tsd';
-import test, {ExecutionContext, Macro} from '..'; // eslint-disable-line import/no-unresolved
+
+import test, {ExecutionContext, Macro} from '..';


Suggested change

import test, {ExecutionContext, Macro} from '..';

import test, {ExecutionContext, Macro} from '../index.js';

You should use full name and extension for the TS stuff too.

There's is no index.js file, but there is an exports map. Which the linter rules don't seem to deal with, so I have direct imports of entrypoints/main.mjs elsewhere. But then TypeScript can't find the type definition. Hence .. here.

test/config/loader.js

test/shared-workers/lifecycle/fixtures/teardown.js

sindresorhus · 2021-05-02T19:16:03Z

I had to load yargs as CJS since it behaved differently

New version was just released. Maybe it's better: https://github.com/yargs/yargs/releases/tag/v17.0.0

novemberborn · 2021-05-03T14:12:17Z

I had to load yargs as CJS since it behaved differently

New version was just released. Maybe it's better: yargs/yargs@v17.0.0 (release)

I'm always happy once I figured out how yargs works… not sure I want to deal with its breaking changes in this here PR.

novemberborn · 2021-05-03T16:24:57Z

Test file paths within shared workers are now file URLs.

XO should pass, but the tap tests are not passing yet.

novemberborn · 2021-05-17T14:47:29Z

I think I fixed Magic assert doesn't seem to work with "type": "module" #2719
Also, title prefixes in the reporters now correctly skip all test file extensions, not just js

Refactor and deploy some tricks so that ava/plugin is usable in a CJS environment, even if the rest of AVA's internals become ESM.

* Load yargs as CJS due to strange compatibility issues * Keep line number parsing code as CJS because it attempts to lazy-load dependencies * Shared workers are always loaded through a dynamic import * Shared worker filenames must now be file:// URLs, but you can provide a URL instance * Test file paths within shared workers are now file:// URLs * Error serialization and code excerpts now support ESM * Title prefixes in the reporters now correctly skip all test file extensions, not just `js`

novemberborn · 2021-05-17T16:02:49Z

Tests pass locally… 😉

sindresorhus · 2021-05-18T14:26:26Z

I haven't actually tested this, but the code changes look good to me.

iambumblehead · 2021-05-27T00:19:13Z

@novemberborn I'm looking forward to using this in AVA's next update :)

novemberborn · 2021-06-06T13:38:37Z

I believe the test failures are due to either a Node.js bug or are intermittent. I'm going to merge this.

Addressed or deferred.

novemberborn force-pushed the use-esm branch from a742eed to 729420a Compare May 2, 2021 18:08

novemberborn requested a review from sindresorhus May 2, 2021 18:08

novemberborn commented May 2, 2021

View reviewed changes

sindresorhus previously requested changes May 2, 2021

View reviewed changes

novemberborn mentioned this pull request May 14, 2021

Magic assert doesn't seem to work with "type": "module" #2719

Closed

novemberborn added 3 commits May 17, 2021 16:49

Extract glob helpers that are needed in a CJS environment

6019cc1

Ensure ava/plugin works in a CJS environment

d5a4c1a

Refactor and deploy some tricks so that ava/plugin is usable in a CJS environment, even if the rest of AVA's internals become ESM.

Ensure 'ava' can be imported in a CJS environment

c3b385d

novemberborn force-pushed the use-esm branch from b32ff7a to 6f459e6 Compare May 17, 2021 14:53

novemberborn linked an issue May 17, 2021 that may be closed by this pull request

Magic assert doesn't seem to work with "type": "module" #2719

Closed

novemberborn force-pushed the use-esm branch from 6f459e6 to ae63579 Compare May 17, 2021 15:03

novemberborn force-pushed the use-esm branch from ae63579 to ea21f6a Compare May 17, 2021 15:39

novemberborn requested a review from sindresorhus May 17, 2021 15:44

novemberborn mentioned this pull request May 18, 2021

Implement semaphores avajs/cooperate#5

Merged

novemberborn added 3 commits May 18, 2021 19:30

fixup! Convert to ESM

17dd6c9

fixup! fixup! Convert to ESM

d011dbb

CI: Increase delay on timeout test, mostly for Windows

414588d

novemberborn marked this pull request as ready for review June 6, 2021 13:38

novemberborn merged commit c57067b into main Jun 6, 2021

novemberborn deleted the use-esm branch June 6, 2021 13:39

novemberborn mentioned this pull request Jun 6, 2021

Use ESM whenever possible #2670

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch to ESM for internals #2741

Switch to ESM for internals #2741

novemberborn commented Apr 26, 2021 •

edited

novemberborn commented May 2, 2021 •

edited

novemberborn May 2, 2021

sindresorhus May 2, 2021

novemberborn commented May 2, 2021

sindresorhus May 2, 2021

novemberborn May 3, 2021

sindresorhus May 2, 2021

novemberborn May 3, 2021

sindresorhus commented May 2, 2021

novemberborn commented May 3, 2021

novemberborn commented May 3, 2021

novemberborn commented May 17, 2021

novemberborn commented May 17, 2021

sindresorhus commented May 18, 2021

iambumblehead commented May 27, 2021

novemberborn commented Jun 6, 2021

	import test, {ExecutionContext, Macro} from '..';
	import test, {ExecutionContext, Macro} from '../index.js';

Switch to ESM for internals #2741

Switch to ESM for internals #2741

Conversation

novemberborn commented Apr 26, 2021 • edited

novemberborn commented May 2, 2021 • edited

novemberborn May 2, 2021

Choose a reason for hiding this comment

sindresorhus May 2, 2021

Choose a reason for hiding this comment

novemberborn commented May 2, 2021

sindresorhus May 2, 2021

Choose a reason for hiding this comment

novemberborn May 3, 2021

Choose a reason for hiding this comment

sindresorhus May 2, 2021

Choose a reason for hiding this comment

novemberborn May 3, 2021

Choose a reason for hiding this comment

sindresorhus commented May 2, 2021

novemberborn commented May 3, 2021

novemberborn commented May 3, 2021

novemberborn commented May 17, 2021

novemberborn commented May 17, 2021

sindresorhus commented May 18, 2021

iambumblehead commented May 27, 2021

novemberborn commented Jun 6, 2021

novemberborn commented Apr 26, 2021 •

edited

novemberborn commented May 2, 2021 •

edited