Open angelaraya opened 2 years ago
Thanks for the suggestion!
Components.js currently only supports CommonJS modules indeed. The architecture is however ready for ESM support, so it's just a matter of implementing it.
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 createInstance
will be different in ConstructionStrategyEsm compared to ConstructionStrategyCommonJs. So the implementation should be fairly straightforward.
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 https://github.com/janeirodigital/sai-js/pull/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 IConstructionStrategy
?
We were trying to add Conditional exports to sai-js https://github.com/janeirodigital/sai-js/pull/32 but that doesn't seem to solve the problem.
@elf-pavlik Does it produce the same error as before? Have you tried adding an export entry for package.json, as is done here?: "./package.json": "./package.json", (Some background on why that is needed: https://github.com/RubenVerborgh/AsyncIterator/pull/34)
@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 To start the latest CSS, everything went well until execution reached the point where CSS dynamically imports oidc-provider, which is an ESM package, where it began throwing errors, complaining about import statements.
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.