ARTEMIS-5999 - Switch from CJS to ESM build

[GChuf]

No description provided.

[grgrzybek]

@GChuf thanks.

I tried moving Hawtio (application built with webpack and the library built with tsup/esbuild), but there were some issues and I had to remove "type":"module" from Hawtio's package.json.

Here are my notes from last year when I tried this:

tsup, webpack and cjs/esm modules

I wanted to change @hawtio/react package to "type":"module" and have tsup/esbuild to produce two outputs (esm, cjs).

But then webpack kind of do double conversion:

// ../../node_modules/@patternfly/react-styles/css/components/ExpandableSection/expandable-section.js
var require_expandable_section2 = __commonJS({
  "../../node_modules/@patternfly/react-styles/css/components/ExpandableSection/expandable-section.js"(exports) {
    "use strict";
    exports.__esModule = true;
...

// src/ui/util/ExpandableText.tsx
var import_expandable_section2 = __toESM(require_expandable_section2(), 1);
...

    const expandableToggle = !isDetached && /* @__PURE__ */ jsxs53(
      "button",
      {
        className: css(import_expandable_section2.default.expandableSectionToggle),

The problems:

• import_expandable_section2.default.expandableSectionToggle fails, import_expandable_section2.default.default.expandableSectionToggle would work
• in theory allowSyntheticDefaultImports: false would work, but it causes problems with react importing

I had problems with double default importing and various issues related have webpack/tsup and jest working (jest still marks ESM support as experimental).

I still don't fully understand when you need the extension in the import/require statements but looks like I use extensionless imports. AFAIR, extensionless imports work well when tsconfig.json uses "moduleResolution": "bundler" which is the recommended setting for libraries and for applications.

Also about the icons - I didn't have problems with extensionless imports. If you check @patternfly/react-icons package.json, there's no "type":"module" even if there's a "module":"dist/esm/index.js" option.
When I tried subpath imports for individual icons with esm variant, I had issues with React - this famous:

Uncaught Error: Element type is invalid: expected a string (for built-in components) or a class/function

so I stayed with importing (c)js variants...

And finally about removing deprecated components - I've migrated the Artemis console (my local, not-yet-pushed branch) to Patternfly 6, but didn't change the styles yet - and yes, it involves the Select/Dropdown component.

I'd eventually suggest migrating to PF6 - especially because Hawtio 5 will be using Patternfly 6 (Patternfly 5 is no longer maintained).

WDYT?

[GChuf]

@grgrzybek, thanks -
first, regarding extensionless imports: I never came across "moduleResolution": "bundler" - or at least I don't remember looking at that.
It's already included in the tsconfig.json in artemis console though, so I first re-run the build after removing the extension on one random import. Build works ...
Then, I ran "npm run start" after removing all extensions and to my surprise, everything seems to work. I should prepare a commit so others can test as well.

About cjs/esm:
If you remember my PR here, which is more or less just an example of a working build - hawtio/hawtio-react#1693 -
I also did not include the type: module inside any package.json - but I did add exports to backend-middleware package.json file.

I cannot remember all the errors I've encountered along the way, and I do not have any notes regarding the process anymore :)
You certainly went deeper into debugging than I ever did so I'm not sure if I'm the right person to comment.
But just as a thought, is switching from CJS to ESM only a valid option for hawtio? For artemis console, it doesn't seem like a big deal to drop CJS since there's no? other projects that use it as a dependency ...

I agree that moving to PF6 makes sense, especialy if hawtio moves to PF6, even though sometimes it feels like updating everything all the time is done for its own sake.
If we move to PF6, removing the deprecated imports needs to be done (and should be done first) and it's just a question of which implementation we take (mine or yours). I don't mind dropping my commits, but it's a shame about the double work :)
I'm not sure what to do with ESM - move to esm build now, or later after PF6? Maybe you could try applying the changes from this PR to your local branch, see if everything still works.
I can remove all the .js extension additions from the "Switch from CJS to ESM build" (7bae20e) commit and you can cherry-pick?

[grgrzybek]

@GChuf hi - getting back to this oldish issue.
Just FYI, recently I did a lot of tests related to what the actual JS is published. I created several combinations of exporters and importers:

• pure tsconfig.json + tsc that produce ESM and (with additional config) CJS
• tsup config that produces both ESM and CJS (in a package with type=module and type=commonjs)
• commonjs consumers, ESM consumers
• manually written CJS and ESM (no transpiling by tsup or tsc)

there are some quirks which mostly revolve around double default problem: https://www.typescriptlang.org/docs/handbook/modules/appendices/esm-cjs-interop.html#no-__esmodule-detection-the-double-default-problem - oh my, it was a painful journey.

Ideally I'd really like to have @hawtio/react provide both ESM and CJS modules, but I'm not there yet - I remember the problems were mostly ... icons. But even recently I found some nasty issue with SVG not being properly transpiled by tsup. See:

• Problems with data:image/svg+xml URLs not displaying correctly hawtio/hawtio-react#2088

Now I'm trying to update to new Module Federation 2.0 (see: hawtio/hawtio-react#2079) and after that I'll try to provide newer/more flexible configuration of ESM/CJS and will add more comments.

[GChuf]

I laughed at "icons" :) Godspeed!
As for me, I'm standing by ...