Skip to content

switch to ESM#207

Open
jogibear9988 wants to merge 6 commits intoioBroker:masterfrom
jogibear9988:master
Open

switch to ESM#207
jogibear9988 wants to merge 6 commits intoioBroker:masterfrom
jogibear9988:master

Conversation

@jogibear9988
Copy link
Copy Markdown

fixes #206

@GermanBluefox
Copy link
Copy Markdown
Contributor

GermanBluefox commented Mar 25, 2026

Will this fix work for front-end and backend?

We have a package from @AlCalzone that creates both packages: https://github.com/ioBroker/plugin-docker/blob/main/package.json#L24

@GermanBluefox
Copy link
Copy Markdown
Contributor

Issues for front-end + back-end usage

  1. Missing "browser" / "module" field -- The exports map only has "import" and "require" conditions. Many front-end bundlers (webpack, Vite,
    Rollup) also look for "browser" or "default" conditions. While modern bundlers generally resolve "import" fine, there's no explicit browser
    entry point. This is probably OK but not bulletproof.

  2. moduleResolution: "node" for ESM is wrong -- The PR changes tsconfig.json from "module": "Node16" / "moduleResolution": "node16" to
    "module": "esnext" / "moduleResolution": "node". This is problematic:

  • "moduleResolution": "node" is the legacy CJS-era resolution. It doesn't enforce .js extensions in imports, doesn't understand exports
    maps, and doesn't match how Node.js actually resolves ESM.
  • The correct setting for ESM targeting Node.js is "module": "Node16" (or "NodeNext") with "moduleResolution": "node16" (or "nodenext"), or
    "module": "ESNext" with "moduleResolution": "bundler" if targeting bundlers.
  • For a library that must work in both Node.js and bundlers, "moduleResolution": "bundler" is actually the most appropriate for the ESM
    build.
  1. CJS build has no moduleResolution -- tsconfig.build.cjs.json sets "module": "commonjs" and "moduleResolution": "node", but it inherits
    "module": "esnext" from the base tsconfig and then overrides it. It works, but the base tsconfig is now ESM-focused, which muddies editor
    support.

  2. The CJS build is a single build/index.cjs file -- The CJS tsconfig outputs to build/ and then renames build/index.js to build/index.cjs
    via mv. But the TypeScript sources have multiple files (ChannelDetector.ts, typePatterns.ts, types.ts, roleEnumUtils.ts). The CJS build will
    produce all of them as .js files in build/, then only rename index.js to index.cjs. The other CJS .js files will clash with or overwrite
    the ESM .js files if they're in the same directory. The ESM build goes to build/esm/ so the CJS files pollute build/ with unrenamed .js
    files that aren't valid ESM. This is a bug.

  3. "type": "module" makes the whole package ESM -- All .js files in the package (including lib/createMd.js, lib/nameOfTypes.js, test files)
    must now use ESM syntax. The PR converts these, but any consumer or tooling that loads .js files from this package will expect ESM. The CJS
    build files in build/ that aren't renamed to .cjs will be treated as ESM by Node.js, but they contain require() calls -- this will crash at
    runtime.

  4. Test file uses require() for JSON -- The current test file uses require(objectDef) to load JSON test fixtures. The PR reportedly converts
    to ESM imports, but dynamic require() for JSON in ESM requires createRequire from node:module or import() with assertions. Need to verify
    this is handled correctly.

  5. No "types" / "typings" field for both entry points -- The exports map should also include "types" conditions so TypeScript resolves .d.ts
    files correctly for both ESM and CJS consumers:
    "exports": {
    ".": {
    "import": { "types": "./build/esm/index.d.ts", "default": "./build/esm/index.js" },
    "require": { "types": "./build/index.d.cts", "default": "./build/index.cjs" }
    }
    }

Verdict

This PR is not safe to merge as-is. The core concern -- front-end and back-end compatibility -- is not reliably addressed due to:

┌────────────────────────────────────────────┬──────────────────────────────────────────────────────┐
│ Problem │ Severity │
├────────────────────────────────────────────┼──────────────────────────────────────────────────────┤
│ CJS build files overwrite/clash in build/ │ Critical -- runtime breakage │
├────────────────────────────────────────────┼──────────────────────────────────────────────────────┤
│ Wrong moduleResolution for ESM │ High -- type-checking won't catch real ESM errors │
├────────────────────────────────────────────┼──────────────────────────────────────────────────────┤
│ No "types" in exports map │ Medium -- TypeScript consumers may not resolve types │
├────────────────────────────────────────────┼──────────────────────────────────────────────────────┤
│ No "default" or "browser" export condition │ Low -- most bundlers handle "import" fine │
└────────────────────────────────────────────┴──────────────────────────────────────────────────────┘

Recommended approach

If the goal is front-end + back-end support, a cleaner approach would be:

  1. Keep "module": "Node16" / "moduleResolution": "node16" (current master settings work fine)
  2. Build ESM to build/esm/ and CJS to build/cjs/ (separate directories, no renaming hacks)
  3. Add proper exports with types conditions
  4. Or: use a tool like tsup or unbuild that handles dual-build correctly out of the box

The current package on master already works in bundlers (webpack/Vite/Rollup can consume CJS just fine), so the real benefit of ESM is
tree-shaking -- which this library is small enough that it barely matters. The risk/reward ratio of this PR is unfavorable.

@jogibear9988
Copy link
Copy Markdown
Author

I don't think I've time to work on those issues, so best would be if we close it.

In my opinion, I would only ship ESM packages, if someone has a project wich uses CJS, the bundler will support the ESM package. I hate that the javascript ecosystem takes forever to switch to ESM... ;-)
And if the lib's do the switch, often they ship invalid ESM packages without file extensions, wich are not usable in the browser directly... (Or as monaco does, ship ESM with special import syntax...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Release as ESM module

2 participants