Change over to ESM

[NancyMcCallB]

Deliver ESM instead of CJS.

### Tasks
- [ ] Drop CJS
- [ ] Switch testing over from CJS to ESM

[raplemie]

@NancyMcCallB @aruniverse We currently deliver both for all packages, what is the intent of this issue ? Can we document it ? I'm not sure removing CJS would be a good idea but I'm not sure that it is what was meant here.

[aruniverse]

The main goal of the issue was to drop CJS support, which is currently only used for testing As a frontend ui library, it doesnt make sense for us to deliver cjs as no one should be using these pkgs in a node env. All browsers iTwinjs support uses ESM by default anyway

[raplemie]

Ok, I wish I had a clearer idea of the impact of this on our consumers before going further on this path.

[mayank99]

The main goal of the issue was to drop CJS support, which is currently only used for testing As a frontend ui library, it doesnt make sense for us to deliver cjs as no one should be using these pkgs in a node env. All browsers iTwinjs support uses ESM by default anyway

Besides testing in node environment, SSR (which also runs in Node) is another reason CJS might be needed. Also I believe that older versions of webpack do not correctly support ESM.

Fwiw iTwinUI recently switched from CJS-first to ESM-first via https://github.com/iTwin/iTwinUI/pull/1246 and https://github.com/iTwin/iTwinUI/pull/1389.

---

Ok, I wish I had a clearer idea of the impact of this on our consumers before going further on this path.

I would like a clearer idea as well. If the impact is minimal, then we would like to drop CJS in iTwinUI as well. It's a bit of a pain to deliver both.

[ImVeryLost]

Moving discussion from #271 as per @aruniverse request

Context: When a package that is using Select dropdown component from `"@itwin/itwinui-react" is build as a JS module, and the itwinjs package that consumes it is build as a ES module, theming does not work.

@ImVeryLost can you elaborate? and possibily in the #232 to not clutter this issues

I'm pretty sure the core build tools, bentley/cra defaults to using esm if present. i know you guys are using vite, which should also be preferring esm over cjs

(Note - this wasnt seen is iTwin Studio, but in ISM analytical tools for Design Review. The sample app and Design Review both use CRA)

Using: "@itwin/build-tools": "4.0.0",

The frontend package in the ISM tool tsconfig extends "./node_modules/@itwin/build-tools/tsconfig-base.json", which sets"module": "commonjs",. Since previously the module type did not matter much, i left the default.

Design Review frontend uses "module": "esnext",.

Theming issues went away after switching to esnext in the frontend package. I dont mind moving to esnext, just thought it a) Needs to be documented because it was not an issue in previous itwin UI versions b) Ideally fixed on iTwin UI side c) Maybe add a tsconfig-base for frontend packages in the @itwin/build-tools that uses esnext by default?

[mayank99]

Ideally fixed on iTwin UI side

What needs to be fixed from iTwinUI? It sounds like a usage problem rather than a bug in iTwinUI.

Agree with documenting it and changing all official templates to use ESM.

[ImVeryLost]

What needs to be fixed from iTwinUI? It sounds like a usage problem rather than a bug in iTwinUI.

Merely the point that theming worked "out of the box" in the previous versions

[mayank99]

Merely the point that theming worked "out of the box" in the previous versions

It "worked" but it was incredibly hacky and had global side effects which often broke other things. In iTwinUI v2 we started relying on context for theming, falling back to the old approach if context is not found. This had the effect of never breaking build, but visuals would break if there is a mismatch between some parts using ESM with a certain theme and other parts using CJS with another theme (dual package hazard).

In iTwinUI v3, we will be using context for even more features (like scoped sizing, which AppUI will benefit from). We also removed the fallback, throwing an error if context is not found. So context will need to work; there is no way around it. And for context to work, the application is responsible for loading only a single instance of the iTwinUI module (either ESM or CJS, not both).

While writing all this, I'm realizing it would be so much simpler if ESM was always used, avoiding such issues and preventing confusion. So in iTwinUI v4, we should strongly consider removing CJS.

[aruniverse]

SSR (which also runs in Node) is another reason CJS might be needed

tbh i dont think iTwin.js is compatible with SSR at all and if that even makes sense for us we will need someone to experiment and test this out

Also I believe that older versions of webpack do not correctly support ESM

the default webpack behavior should be preferring esm over cjs, see: https://webpack.js.org/configuration/resolve/#resolvemainfields

Using: "@itwin/build-tools": "4.0.0", The frontend package in the ISM tool tsconfig extends "./node_modules/@itwin/build-tools/tsconfig-base.json", which sets "module": "commonjs",.

the reason build tool's default output is cjs is because its used in both frontend and backend packages and node esm support & tooling has been spotty. the way the community decided to implement esm across web and node has been a disaster; webpack does it one way, node another

it should be the responsibility of the pkg publisher to build an esm variant of their package

all of the core frontend and common pkgs deliver both esm and cjs, and our backend pkgs deliver only cjs its an additional one line change in your package scripts to deliver esm if you're using itwin/build-tools

[ImVeryLost]

it should be the responsibility of the pkg publisher to build an esm variant of their package

all of the core frontend and common pkgs deliver both esm and cjs, and our backend pkgs deliver only cjs its an additional one line change in your package scripts to deliver esm if you're using itwin/build-tools

No argument there, just throwing out some suggestions to make it easier for other packages not to stumble on the same issue. Until itwinUI documents this behaviour, its a "easy to miss & hard to diagnose" mistake.

[mayank99]

tbh i dont think iTwin.js is compatible with SSR at all and if that even makes sense for us we will need someone to experiment and test this out

iTwinUI is meant to be used for more than just iTwin.js. For example, developer portal uses it. Also, you can have some parts of the app be SSR'd while others stay client-only. For AppUI this might not be the concern though.

Also I believe that older versions of webpack do not correctly support ESM

the default webpack behavior should be preferring esm over cjs, see: webpack.js.org/configuration/resolve/#resolvemainfields

Note: I'm talking specifically about older versions of webpack where there were tons of issues with ESM support, especially when it comes to interop. You can search "ESM" in the webpack repo and look at issues from 2018-2019.

Using: "@itwin/build-tools": "4.0.0", The frontend package in the ISM tool tsconfig extends "./node_modules/@itwin/build-tools/tsconfig-base.json", which sets "module": "commonjs",.

the reason build tool's default output is cjs is because its used in both frontend and backend packages and node esm support & tooling has been spotty. the way the community decided to implement esm across web and node has been a disaster; webpack does it one way, node another

i believe that in 2023, all packages should be ESM-first or ESM-only ("type": "module"). Even the backend packages should deliver ESM (alongside CJS) because Node ESM support is pretty good these days and has been stable for many years now.

---

No argument there, just throwing out some suggestions to make it easier for other packages not to stumble on the same issue. Until itwinUI documents this behaviour, its a "easy to miss & hard to diagnose" mistake.

I agree with both your suggestions - documenting this behavior and also changing the build-tool default target to ESM.