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

Usage of ESM imports instead of CommonJS #251

Closed
4 tasks done
theoludwig opened this issue Aug 17, 2021 · 15 comments
Closed
4 tasks done

Usage of ESM imports instead of CommonJS #251

theoludwig opened this issue Aug 17, 2021 · 15 comments
Labels
enhancement maintenance Issues that covers general maintenance chores
Milestone

Comments

@theoludwig
Copy link
Contributor

theoludwig commented Aug 17, 2021

We should use ESM imports instead of CommonJS.
See: https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c
We'll be able to merge #238, #247, and #262 (dependencies updates that only work with ESM) after this change.

Note: It is a BREAKING CHANGE as standard-engine will only work with ESM.

To offer a smoother transition for packages that depends on standard-engine, we should first migrate them to ESM.

Status

@voxpelli
Copy link
Member

I'm not so sure on this one. I would maybe push this one major version forward.

@theoludwig
Copy link
Contributor Author

I'm not so sure on this one. I would maybe push this one major version forward.

Could you elaborate a little bit more?
Do you think it is too early ?
What are the downsides to do this change ?

@voxpelli
Copy link
Member

voxpelli commented Aug 18, 2021

The downsides is that we, in addition to other changes, need to change how we import this module in all the projects that uses this module + ideally also convert those to ESM then.

As ESM can easily import CommonJS in Node I think it would be better to start converting the individual users to ESM before converting this module.

Also: I would ideally have the ESM change in a version where there are no other breaking changes as it's enough of a breaking change to migrate to, also having to deal with other breaking changes at the same time becomes a bit too much, so I would rather avoid it if possible :)

@theoludwig
Copy link
Contributor Author

start converting the individual users to ESM before converting this module

Sounds good to me! 👍
We will do it for v16, so we can slowly migrate the whole organization to ESM.

@theoludwig theoludwig modified the milestones: 15.0.0, 16.0.0 Aug 18, 2021
@jimmywarting
Copy link

pkg-conf was also converted to ESM recently - it's much smaller in size now
guess we don't have much of a choise to start using ESM if we want to update any of our dependencies now.

@voxpelli
Copy link
Member

voxpelli commented Sep 2, 2021

@jimmywarting We can use many ESM modules in CJS by importing them like await import('pkg-conf'), so we can hold out a while longer

@jimmywarting
Copy link

oh great, not many package have the luxury of using lazy import

@theoludwig
Copy link
Contributor Author

@jimmywarting We can use many ESM modules in CJS by importing them like await import('pkg-conf'), so we can hold out a while longer

Currently, there are #238, #247 and #262 PRs to fix, if it is possible.
It is failing because the packages have been converted to ESM, would appreciate a PR if it is possible to fix with dynamic import (await import). @voxpelli

@jimmywarting
Copy link

jimmywarting commented Sep 5, 2021

all doe i would have prefer if it got switched to ESM-only, but understand if you are not ready yet to go full ESM-only

if you missed it node-fetch@3 (the largest http lib) just went ESM-only :P

@theoludwig
Copy link
Contributor Author

all doe i would have prefer if it got switched to ESM-only, but understand if you are not ready yet to go full ESM-only

if you missed it node-fetch@3 (the largest http lib) just went ESM-only :P

I agree I would love to switch to ESM-only, but there are so many things breaking with this change that we still need to wait a little bit before doing that.
And it would offer a smoother transition to migrate to ESM only for packages like standard, ts-standard etc, before migrating this one (standard-engine).

@jimmywarting
Copy link

for some i think the transition could be easy as many are just executed from cli like so: npx standard so it would not mater what version you use... but then there is half of which that actually use standard scripting-wise

@voxpelli
Copy link
Member

@divlo Okay with you if we close this as completed?

@theoludwig
Copy link
Contributor Author

@divlo Okay with you if we close this as completed?

@voxpelli Why? This is not completed?
Honestly, I think, we can start the migration in standard-engine if anyone wants to do so even if ts-standard is currently blocked. Of course, the best would be to solve the issue of the ts-standard rewrite first and then do this migration here.

@voxpelli
Copy link
Member

@divlo Remembered wrongly that we had already done the move here, that this was only open due to ts-standard not having adopted it.

Then let’s get the ESM:ing started rather than waiting for ts-standard 👍

@theoludwig
Copy link
Contributor Author

Unlikely to be done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement maintenance Issues that covers general maintenance chores
Projects
Status: Done
Development

No branches or pull requests

3 participants