finos / FDC3

An open standard for the financial desktop.
https://fdc3.finos.org
Other
201 stars 132 forks source link

CommonJs vs ES6 #1351

Open robmoffat opened 2 months ago

robmoffat commented 2 months ago

For Background:

@Lecss @kriswest and I built the "combined" @finos/fdc3 module in the FDC3 repo, combining all the other modules. For now, this uses ES6 module structure, rather than the FDC3 rollup approach used originally.

However, the original FDC3 module was commonJS. I'm not sure of the ins and outs of this, so:

  1. Is it backwards incompatible to move to ES6? Or does Javascript correctly handle both?
  2. ES6 import seems like the way we want to go forward is this the right time to change?
  3. If we have an old, legacy CJS module, should we expect people to import the monorepo's submodules e.g. fdc3-context? This seems unlikely to work given the amount of literature around the old version.

I want to open this up for further discussion as I'm not an expert on this.

robmoffat commented 2 months ago

@novavi

robmoffat commented 2 months ago

@julianna-ciq

kriswest commented 2 months ago

My limited understanding is that the current module contains both: https://unpkg.com/browse/@finos/fdc3@2.1.1/dist/ and I would assume its not backwards compatible to move to ESM only... but again I'm no expert on that topic. @julianna-ciq or @thorsent might know more.

thorsent commented 2 months ago

TLDR; either configure rollup to produce both commonjs and esm (if you want to be a good citizen), or just go with esm (you'll be fine)

The approach taken by most libraries these days is to produce both commonjs and esm. Rollup can do this pretty easily (the "main" entry in package.json points to the cjs version, while the "module" entry points to the esm version).

Some libraries have chosen to only support esm (an effort to strong-arm the world into moving forward), so people are learning to deal with it.

In general, commonjs is only necessary for node (and usually only in legacy situations). On the web, browsers already support esm as do all the major bundlers. Node however is very persnickety about esm , requiring either that all modules are esm (via a switch) or that modules specifically identify themselves using the .cjs and .mjs notation to indicate their module type.

We most often see this issue rear its head when building unit tests, since mocha and JEST run in node. Anyone in the world who has incorporated the FDC3 library, or needs to incorporate the FDC3 library, in a mocha suite that isn't configured for esm will have issues (as they will with many other public libraries - the solution usually being to use an older version...)

Typically, if you don't have a commonjs option then you have to go "all in" on esm in node in your unit test pipeline, which means including extensions in your imports:

import "../mymodule.js" instead of import "../mymodule"

This syntax change gives a lot of people heart palpitations, because "mymodule.js" may actually refer to a file named "mymodule.cjs"! I think this is the major reason why we're not all running esm already. Personally, I've found that it's just something to get used to.

robmoffat commented 2 months ago

That's brilliant, @thorsent. Also it agrees with @Lecss 's view that we should "move to ES6 and wait until someone complains".

kriswest commented 2 months ago

If doing so, I think we should do so in the open by getting consensus from the SWG. That'll give anyone with a CJS issue a chance to come forward before the release and for us to work out a solution if one is needed (such as an optional legacy build).

I too doubt most will have issues and few will complain about a smaller module!

Roaders commented 1 month ago

I am no great expert on ES6 by any means. My main experience is being frustrated when things don't work when a package is only available as an ES6 package!! 😄

In theory it should all just work. As mentioned above most / all bundlers support ES6. It is usually jest or other dev tools that don't work (again as mentioned above in the very helpful comment by @thorsent )

In my opinion to reduce friction of adoption and to give developers an easier life it would be better to publish both commonJs and ES6 in the same package. A lot of popular libraries do this and as we're hoping to become a popular library as well I think we should do the same!

kriswest commented 1 month ago

We currently publish a combined package containing both CommonJS and ES6 modules - so this is a proposal to stop doing so.

As discussed at today's SWG meeting, we'll drop this from the 2.2 scope for the time being, while the maintainers review the options and work to create an FDC3 mono repo (which alters the build system and is what prompted this proposal to be raised).

FYI The current rollup config that creates both module types can be found at: https://github.com/finos/FDC3/blob/main/rollup.config.js