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

Convert to ESM #501

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Convert to ESM #501

wants to merge 1 commit into from

Conversation

fatso83
Copy link
Contributor

@fatso83 fatso83 commented Aug 26, 2024

Purpose (TL;DR) - mandatory

Fix #489 (Skypack import) by converting to ESM. Using top-level await is only allowed in Ecmascript modules, so if we were to use await import(...) in a CJS module, it would change our public interface, as we would have to somehow expose the fact that we need to await a Promise. So to keep the same interface, an ESM conversion seems necessary.

This change should not be handled without forethought ... To be able to do this, we probably need
to convert the main sinon package to ESM as well.

Here is what happens if I pull in the ESM version of this library into Sinon:

 Exception during run: Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/carlerik/dev/fake-timers/src/fake-timers-src.js from /Users/carlerik/dev/sinon/lib/sinon/util/fake-timers.js not supported.
fake-timers-src.js is treated as an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which declares all .js files in that package scope as ES modules.
Instead either rename fake-timers-src.js to end in .cjs, change the requiring code to use dynamic import() which is available in all CommonJS modules, or change "type": "module" to "type": "commonjs" in /Users/carlerik/dev/fake-timers/package.json to treat all .js files as CommonJS (using .mjs for all ES modules instead).

We also need to fix linting issues and I was not successful in figuring out how to override languageOptions.sourceType.

Pretty sure this is just the start in opening up a can of worms ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Skypack bundle cannot be loaded in the browser
1 participant