-
Notifications
You must be signed in to change notification settings - Fork 6
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
ESM support #83
Comments
Thanks for the suggestion! |
Components.js currently only supports CommonJS modules indeed. This is not part of my current roadmap at the moment though (since my projects are all on CJS atm). But I'm happy to give guidance for anyone that wants to submit a PR. (Placing a bounty via the Comunica Association might also be a possibility) An easier solution might however be to just expose both the CJS and ESM versions of the sai-js module in its package.json file, which could be done like this: https://github.com/RubenVerborgh/AsyncIterator/blob/main/package.json#L8-L14 |
For reference, adding support for loading ESM modules should just be a matter of implementing a new IConstructionStrategy. Currently, we have a ConstructionStrategyCommonJs.ts, where we would require a ConstructionStrategyEsm in the future. Most likely, only the implementation of However, if the desire is to also use Components-Generator.js on ESM projects, that may also require some tweaks in its ResolutionContext |
We were trying to add Conditional exports to sai-js janeirodigital/sai-js#32 but that doesn't seem to solve the problem. @woutermont based on the comments above, do you think you would be able to put together a PR with basic support for ESM? @angelaraya will you need to use components-generator on sai-js or it is just a matter of |
@elf-pavlik Does it produce the same error as before? |
@elf-pavlik Digita has some packages which use conditional exports succesfully to work both with ComponentsJS and in the browser, so i.m.o. that should work for sai-js as well. Do you have an open PR for that somewhere? I will try to look into ESM support next week. Did not think about the generator implications though, so it might be more complex than I expected. |
@woutermont @rubensworks the error with the conditional exports was fixed in sai-js. The only open item would be adding ESM support for componentsjs and the generator. |
Okay, great! So we're temporarily good for sai-ja, as long as we don't need esm-only dependencies, right? |
Has anyone had a chance to look further into adding ESM support? |
No, not that I'm aware of. |
I was updating the testing setup in https://github.com/o-development/solid-notification-client I think supporting ESM could prevent needing to sandwich projects like CSS as CJS between otherwise standard ESM code. |
Issue type:
Description:
With the move to ESM on the server side, there are packages that will stop supporting (or right out removed) CommonJS support. This means that any dependency that only exports as an ES module won't be usable with Components.js.
Some more context
I ran into this issue recently when trying to use sai-js in the Interop Authorization Agent implementation and would appreciate not having to change the codebase to use dynamic imports for no other reason than having to comply with CommonJS.
The text was updated successfully, but these errors were encountered: