[CDX-398] Backwards Compatibility Fix for CJS users

[Mudaafi]

When we released the ESM bundled version earlier, we unintentionally made it the default export. This resulted in CJS environments breaking when they upgraded to the newer minor version since require now returns { default: xxx }.

This patch fix removes the typical ESM-structure in package.json in favor of exports which allows us to retain the typical require pattern while also supporting the usual import pattern for ESM. There are limitations however, namely:

1. Extension-less deep imports are no longer supported

   1. require('@constructor-io/constructorio-client-javascript/lib/src/modules/search') → MODULE_NOT_FOUND.
   2. Deep imports must also include the extension, i.e. .js

2. Shared types declaration is ambiguous and might fail for node16 ESM consumers

   1. Since the JS uses module.exports but the types use export default, TypeScript under moduleResolution: "node16" might think .default is required when it isn't.
   2. Might need to split types into .ts and .mts

3. Older bundlers that do not support exports will not work for ESM environments since package.json no longer has the module field

[constructor-claude-bedrock]

Code Review

This PR fixes a backwards-compatibility regression for CJS consumers by replacing the module field in package.json with a proper exports map and renaming the ESM output to .mjs. The approach is technically sound, but has a few issues worth addressing before merge.

Inline comments: 4 discussions added

Overall Assessment: ⚠️ Needs Work

[constructor-claude-bedrock]

Important Issue: The "types" condition inside the exports map points to ./lib/types/index.d.ts, which contains export default ConstructorIO (ESM-style). When a TypeScript consumer uses moduleResolution: "node16" or "bundler" and resolves through the "import" condition, TS will correctly find this declaration and expect a .default import. However, when the same map resolves through the "require" condition (which points to a CJS file using module.exports = …), TypeScript still uses the same index.d.ts. This mismatch between the runtime shape (module.exports) and the type declaration (export default) is exactly the ambiguity the author calls out in limitation #2 of the PR description.

The safest fix is to provide a separate types condition per entry-point kind:

"." : {
  "import": {
    "types": "./lib/types/index.d.mts",
    "default": "./lib/esm/constructorio.mjs"
  },
  "require": {
    "types": "./lib/types/index.d.ts",
    "default": "./lib/constructorio.js"
  }
}

Where index.d.mts uses export default and index.d.ts uses export =. This is the pattern recommended in the TypeScript 4.7 docs for dual CJS/ESM packages. The limitation is already noted in the PR description, but shipping it in a known-broken state for node16 ESM consumers without a follow-up ticket creates silent TS errors for those users.

[constructor-claude-bedrock]

Important Issue: This catch-all pattern restores access to deep paths like ./lib/src/modules/search, but the PR description states (limitation #1) that extension-less deep imports are no longer supported. Because this wildcard maps ./lib/* → ./lib/* literally (no extension appended), consumers who import without an extension (e.g. require('@constructor-io/…/lib/src/modules/search')) will still get MODULE_NOT_FOUND at runtime. The wildcard gives the appearance of support while the limitation remains.

This is worth either:

1. Documenting explicitly in a CHANGELOG or migration guide for this release, or
2. Enumerating the most common deep-import paths explicitly so the error is obvious at publish time rather than at consumer run-time.

At minimum, a code comment would help future maintainers understand why the wildcard is there and what it does/does not support.

[constructor-claude-bedrock]

Suggestion: The comment above this line is clear and well-written. One small addition would be to also note that any consumers who previously referenced lib/esm/constructorio.js (e.g. direct CDN links, test helpers, or legacy module field users) will now get a 404/MODULE_NOT_FOUND. Since lib/ is included in the published files array this is a published path change; a brief note here (or in CHANGELOG) would help teams bisect breakage. Low severity since the exports map already routes bundlers correctly.

[Copilot]

Pull request overview

Adjusts the published entrypoints to restore CommonJS require() behavior after an earlier ESM packaging change inadvertently made CJS consumers receive { default: ... }. The PR does this by switching to conditional exports in package.json and emitting the ESM bundle as .mjs so ESM can be supported without flipping the entire package to "type": "module".

Changes:

• Emit the ESM build as lib/esm/constructorio.mjs so Node treats it as ESM without changing package-wide module classification.
• Replace the module field with conditional exports to route import to the ESM bundle and require to the CJS entrypoint.
• Add an export pattern for ./lib/* and explicitly export ./package.json.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
scripts/build-esm.js	Changes ESM build output from .js to .mjs to ensure correct ESM detection without "type": "module".
package.json	Introduces conditional exports to resolve CJS vs ESM entrypoints correctly and removes the module field.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Alexey-Pavlov]

@Mudaafi Thanks for working on this! Left a few comments, please take a look when you get a chance
Analyzing this PR is quite challenging because there are a lot of different cases, but I think we can fix limitations 1 and 3

[Alexey-Pavlov]

There's no node condition here. So import in Node, and also webpack/Next.js server builds now get the browser bundle (platform: 'browser', global replaced with window). Before this PR they got the CJS build, because module was only read by bundlers.
https://github.com/Constructor-io/constructorio-client-javascript/blob/cdx-498-client-js-fix-require-resolution-broken-by-the-2900-esm/src/constructorio.js#L108
typeof window !== "undefined" && window.CLIENT_VERSION

Suggested change

	"import": "./lib/esm/constructorio.mjs",
	"node": "./lib/constructorio.js",
	"import": "./lib/esm/constructorio.mjs",

[Mudaafi]

interesting. I would've thought that Node environments would use the Node SDK, but no harm keeping it consistent post-change unless it requires something weird. Good catch

[Alexey-Pavlov]

You already listed this as limitation number 1, but I think we can just fix it

Suggested change

	"./lib/*": "./lib/*",
	"./lib/*": "./lib/*.js",
	"./lib/*.js": "./lib/*.js",
	"./lib/*.mjs": "./lib/*.mjs",
	"./lib/*.d.ts": "./lib/*.d.ts",
	"./lib/*.map": "./lib/*.map",

[Mudaafi]

huh. we'll have to consider other extensions as well like .css but I'll dig into it

[Alexey-Pavlov]

Modern bundlers ignore the module when exports match, so keeping it changes nothing for them. It only helps old tooling, which just reads the fields and now falls back to CJS

Can we just put it back as
"module": "lib/esm/constructorio.mjs"?
Then limitation #3 is gone then

[Alexey-Pavlov]

Maybe, adding something likenpx @arethetypeswrong/cli --pack . to CI would have caught the 2.90.0 bug too 🤔

https://www.npmjs.com/package/@arethetypeswrong/cli

[Mudaafi]

I considered that actually. I discarded it though, but I'll need to dig up the reason