fix: dual package

[mauriciolauffer]

Fix issue #778

[marianfoo]

LLM review below:

Thanks for putting this together. I reproduced #778 independently and agree with the root-cause analysis and core fix.

Node determines the format of dist/cjs/index.js from its nearest package.json. Because the nearest package currently declares "type": "module", the CommonJS output is parsed as ESM. The require export condition selects the file but does not change its module format.

I reproduced the exact ReferenceError: exports is not defined in ES module scope with the published 3.0.10 package and a real .cjs consumer on Node 20.20.2, 22.18/22.21, and 24.11/24.15. Adding dist/cjs/package.json with {"type":"commonjs"} fixes all tested versions while ESM imports continue to work.

So the proposed package-boundary fix is correct. The ESM marker is technically redundant because it inherits the root "type": "module", but keeping it explicit and symmetrical is reasonable.

Before resubmitting, I recommend addressing these gaps:

1. Handle build:watch.
   The existing watch command cleans dist and then only runs tsc -w, so the marker can disappear. A safe structure would be:

   "build:watch": "run-s clean _finalize:dist _build:watch:all",
   "_build:watch:all": "run-p _build:watch:cjs _build:watch:esm"
   

2. Add a regression test using the package specifier.
   It should verify both:

const { default: Service } = require("wdio-ui5-service")
new Service()

and:

const { default: Service } = await import("wdio-ui5-service")
new Service()

This tests the actual conditional exports instead of loading dist/* directly.

3. Verify the packed artifact.
   npm pack should contain:

dist/cjs/package.json
dist/esm/package.json

Installing that tarball into a clean temporary project and running the two loading tests above would cover the real consumer path.

4. Update the core workflow path filters.
   The current workflow is primarily triggered by examples/** and src/. A packaging-only PR changing package.json and scripts/ may therefore skip the relevant CI. Consider adding package*.json, scripts/, test/, and tsconfig*.json.

Also, renaming only the consuming wdio.conf.js to .cjs is not sufficient for this bug. That can fix the caller’s own module format, but it does not reclassify wdio-ui5-service/dist/cjs/*.js.

With those additions, I believe this is the correct and appropriately minimal package-side solution.

[mauriciolauffer]

Thanks for reviewing. Follow my response to the recommendations:

1. The new tsdown already takes care of that see changes in package.json and tsdown.config.ts
2. The tests were already added. If you run the new tests with the old build, it fails. Run with the new build, success. Same for the tests in https://github.com/mauriciolauffer/wdio-ui5-test-apps
3. No need to have 2 package.json files. All CJS content is now .cjs. See tsdown.config.ts.
4. That's a good one, current publish workflow ignores changes in package.json. I'll add a small change to something in .github/** just to trigger the publish. Not adding package.json to paths as any change there could trigger a new publish.

[marianfoo]

Thanks for taking the time to respond. You’re right that some of my earlier recommendations were based on the previous package.json marker approach.

After testing the current branch again, I agree that:

• tsdown --watch rebuilds both outputs correctly.
• Emitting .cjs and .d.cts removes the need for nested package.json files.
• The packed package works with both require("wdio-ui5-service") and import("wdio-ui5-service") on Node 20.20.2 and 22.18.0.
• All four example scripts pass locally for me on Node 22.

I did notice a few remaining points that may be worth checking:

1. Node 20 build compatibility

tsdown@0.22.14 requires Node ^22.18.0 || >=24.11.0. The current Node 20 CI jobs therefore fail during npm run build with:

Failed to import module "unrun"

The Node 22 jobs were subsequently cancelled, so the new tests have not yet completed successfully in PR CI.

According to the tsdown documentation, the intended approach for libraries supporting Node 20 is to build on Node 22.18+ and then test the built output or tarball on Node 20. Perhaps the workflows could be adjusted accordingly.

2. Bundled @wdio/types declarations

The current build emits 30 files under:

• dist/cjs/node_modules/@wdio/types/**
• dist/esm/node_modules/@wdio/types/**

This also causes declarations such as dist/cjs/service.d.cts to reference a relative bundled copy of @wdio/types. In a clean TypeScript consumer with skipLibCheck: false, I received additional duplicate declaration errors including TS2374, TS2320, and TS2310.

The build itself reports:

Detected dependencies in bundle: @wdio/types

I tested adding:

deps: { neverBundle: ["@wdio/types"] }

to the shared tsdown configuration. That removed the vendored declarations and the additional duplicate errors. This appears consistent with the tsdown dependency documentation.

3. CI coverage for the TypeScript examples

Unless I overlooked another workflow, npm run test:cjs:ts and npm run test:mjs:ts are not currently invoked by CI. The JS variants are covered by wdi5-tests_js-app.yml, but the new TS variants appear to be local-only.

It might also be helpful to add a small npm pack smoke test that installs the tarball into a clean temporary project and checks both loading modes. That would verify the actual published artifact without requiring another browser suite.

4. Workflow path filters

Apologies if my earlier wording was unclear: I was referring primarily to the PR test workflows, not only npm-publish.yml.

The example changes happen to trigger CI for this PR, but a future packaging-only change to package.json or tsdown.config.ts would still skip the core checks.

Using an unrelated .github/** change would trigger this particular publication, but it would not cover future packaging changes. Perhaps the relevant package/build paths could be added to CI, while publishing could use an explicit or tag-based trigger if every package.json edit should not create a release.

5. main fallback

One smaller compatibility consideration: would it make sense to keep:

"main": "./dist/cjs/index.cjs"

The exports field already sends ESM imports to dist/esm/index.js. With the proposed ESM main, package-specifier loading works, but directory loading through require("./node_modules/wdio-ui5-service") still produces ERR_REQUIRE_ESM on Node 20.0.0.

Overall, I agree that the .cjs approach addresses the original runtime problem. I would just appreciate another look at the Node 20 build setup and generated declarations before merging.

[mauriciolauffer]

My comments:

1. Yes, I'm aware, that's why I've changed the workflows. I'm intentionally ignoring node v20 now because I'm working on a new test structure. So, I don't want to spend a lot of time to make it work here, then change on the next PR. I'm not making other changes in tests now, just what is required to cover these changes. Let me know whether this is a blocker.
2. I expected unbundle mode to take care of that. My latest commit address it: 3b1ac05
3. Same as said in item 1...
4. Same as said in item 1...
5. Changed as requested.