sinonjs / fake-timers

Fake setTimeout and friends (collectively known as "timers"). Useful in your JavaScript tests. Extracted from Sinon.JS
BSD 3-Clause "New" or "Revised" License
793 stars 103 forks source link

Skypack bundle cannot be loaded in the browser #489

Open jgosmann opened 5 months ago

jgosmann commented 5 months ago

We understand you have a problem and are in a hurry, but please provide us with some info to make it much more likely for your issue to be understood, worked on and resolved quickly.

What did you expect to happen? FakeTimers module is loaded.

What actually happens An error is logged and the module does not load:

Uncaught Error: [Package Error] "timers" does not exist. (Imported by "@sinonjs/fake-timers").
    <anonymous> https://cdn.skypack.dev/error/node:timers?from=@sinonjs/fake-timers:14

How to reproduce

Go to https://output.jsbin.com/xijaquyico and open the JavaScript console to see the error. The module is loaded like this:

  <script type="module">
    import FakeTimers from "https://cdn.skypack.dev/@sinonjs/fake-timers@v11.2.2";
  </script>
jgosmann commented 5 months ago

With version 7.0.1 when the Skypack instructions were added to the readme, it still works.

I suppose the conversion to ESM destroys the conditional import here?

fatso83 commented 5 months ago

oooh, good catch! We have not caught this, nor do we have a test for it. we could still do an async await import(...), though, to fix this, I guess, but that will be a breaking change, as that cannot be done without making the setup face async.

Or ... well.

It can be done, sort of, but it will be a bit hacky:

Which is not great, as I assume most/many consumers will be using this in ESM, in which module will be defined (but in a lot of them, like browsers, there will of course not be a node:timers).

@mroderick I think you would be interested in this.

@SimenB Any ideas? Fixing conditional imports like this would trigger a breaking API change in Jest, right?

I am guessing someone has some clever ideas somewhere.

benjamingr commented 5 months ago

@fatso83 nothing is stopping us from doing top-level await import right?

fatso83 commented 5 months ago

No ... but I am not sure how that works. I usually try to map these newer constructs over to how this would look in ES5, but I cannot see how this works without introducing a Promise that leaks out into the exported module. But maybe there is? Totally not sure, so I will glady be informed 😅

fatso83 commented 4 months ago

Gah, this was hard, Ben. I have tried out top-level await , but getting that and CJS to work together was non-trivial, so it ended up just being a ESM conversion. Did not get anywhere. I think this will have to wait until @mroderick does a full ESM conversion?