MithrilJS / ospec

Noiseless testing framework
MIT License
48 stars 13 forks source link

Provide an ES6 build of ospec #25

Closed charlag closed 2 years ago

charlag commented 4 years ago

Hi Thanks a lot for making and supporting ospec. I would like to ask you to provide ES6 version of ospec. We are currently in the process of migrating to Rollup and we try to use native ES6 modules for development, in both web and node versions. To use ospec we use @rollup/plugin-commonjs which transforms commonjs module into ES6 module (or tries its best to do so). Unfortunately it cannot do much with dynamic require() for the util which is not allowed when the file is treated as ES module. What could be used for ES there is dynamic import().

dead-claudia commented 4 years ago

For now, I'd recommend using esm, but as it stands currently, we would not be able to support ES modules without natively using dynamic from within ospec's CLI itself. This may be possible in the future, but such a feature would require Node 14 or later.

charlag commented 4 years ago

I think node uses esm internally so I don't see a difference. I thought that one cannot use import() in commonjs modules but turns out that it's possible and even in node 12: https://nodejs.org/docs/latest-v12.x/api/esm.html#esm_import_expressions if that works, separate es build might not be necessary (but is still nice IMO)

dead-claudia commented 4 years ago

I think node uses esm internally so I don't see a difference.

It does everything natively.

I thought that one cannot use import() in commonjs modules but turns out that it's possible and even in node 12: https://nodejs.org/docs/latest-v12.x/api/esm.html#esm_import_expressions if that works, separate es build might not be necessary (but is still nice IMO)

You still have to pass --experimental-modules and there's been a few semantic changes since then, including with the way package.json is read.

charlag commented 4 years ago

Yeah, sorry, I got confused, I thought the flag is only needed for static imports. Then it makes sense to provide conditional export I think? https://nodejs.org/api/esm.html#esm_writing_dual_packages_while_avoiding_or_minimizing_hazards

I could also try to do it if you agree but it would need some trick to pass a way to import things dynamically.

If I understand your proposal to use esm correctly, you say I could dynamically use it (rather than with node -r esm). This wouldn't really work with browser I believe?

dead-claudia commented 4 years ago

Now that I take a closer look, it appears v12.x has it enabled by default. So it does appear possible. I would still need to modify tests first to ensure it still works.

If I understand your proposal to use esm correctly, you say I could dynamically use it (rather than with node -r esm). This wouldn't really work with browser I believe?

And for using esm, you'd use ospec --require esm IIRC (I'm going off memory, so might be misremembering the name of the flag).

charlag commented 4 years ago

Node 14 has it by default at least. We would need to do it in each place we import ospec (and we also only invoke it programmatically). I just wasn't sure if you meant wrapping require with esm in code or using -r.

charlag commented 4 years ago

I quickly tried to use import() in ospec instead of require but it doesn't seem to be simple to implement. I would still propose separate exports for different versions

pygy commented 2 years ago

This can be worked around without producing an ES Module version, by calling require in a try/catch block.

Fixed in master :-)