-
-
Notifications
You must be signed in to change notification settings - Fork 233
Esmification #268
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
Esmification #268
Conversation
Hi @P0lip, hi @philsturgeon I've fixed the ESLint, but seems tests will fail because of Can you consult me, how to correctly update project config to enable es-modules for test environment? |
This comment was marked as outdated.
This comment was marked as outdated.
@aerialist7 I understand this situation a lot better now, and I see that there's a way to go ahead with this pull request if you're still interested! https://dev.to/bcoe/esm-doesn-t-need-to-break-the-ecosystem-4p8b They talk about conditional imports:
Can we set that up so things will continue to work for CJS and ESM? Then we'll avoid the problems discussed in the node-fetch issue. |
Hi @philsturgeon, thank you for your respond. |
I'll admit I'm not caught up with all this, but I have blundered my way through it with help on other projects. https://github.com/stoplightio/spectral-owasp-ruleset/blob/main/package.json We got that repo building from TS to CJS and ESM so perhaps something can be nicked from there. Do you think you could take a swing at it? |
I've just merged two large changes which will have caused some conflicts here, sorry about that. I'm done merging big changes now though so you've got a clear runway to land this change. |
any updates on this? cheers |
@deepakthankachan you know if this PR has updates because it will show them. Currently there are conflicts and failing tests so we're just waiting for that to get fixed. |
The best way to proceed with this is first add in building the library with a tool that can output both esm and cjs and then do the esm changes if we want. There shouldn't really be a reason we need to introduce esm though, since esm can already import cjs files which the library currently is. If we see the value in converting to esm in the long run then making those changes piecemeal in follow up PRs is the best way to go. |
@acunniffe any chance you can pick this PR up and get it over the line? |
I've made progress over here if anyone is interested in continuing this effort. #288 |
More progress is being made in #288 and this one isn't being worked on so I'll close this for that. |
Hi Team,
I've prepare the ESM version of the library.
Can you please review the changes and merge them if OK?