Conversation
Projects using moduleResolution: "node" cannot resolve package exports maps, requiring manual tsconfig paths workarounds. Adding CJS format to tsup and require/main/types fields to package.json lets these projects resolve imports natively. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces CommonJS build output alongside ESM to improve compatibility with projects using legacy module resolution. The changes include updating the build configuration and expanding the package.json exports and entry points. Feedback suggests addressing subpath resolution issues for legacy environments using typesVersions or proxy directories and warns about the 'Dual Package Hazard' which could result in multiple singleton instances if both module formats are loaded.
| "exports": { | ||
| ".": { | ||
| "types": "./dist/index.d.ts", | ||
| "import": "./dist/index.js", | ||
| "types": "./dist/index.d.ts" | ||
| "require": "./dist/index.cjs" | ||
| }, | ||
| "./nestjs": { | ||
| "types": "./dist/integrations/nestjs/index.d.ts", | ||
| "import": "./dist/integrations/nestjs/index.js", | ||
| "types": "./dist/integrations/nestjs/index.d.ts" | ||
| "require": "./dist/integrations/nestjs/index.cjs" | ||
| }, | ||
| "./nextjs": { | ||
| "node": { | ||
| "types": "./dist/integrations/nextjs/index.d.ts", | ||
| "import": "./dist/integrations/nextjs/index.js", | ||
| "types": "./dist/integrations/nextjs/index.d.ts" | ||
| "require": "./dist/integrations/nextjs/index.cjs" | ||
| }, | ||
| "default": null | ||
| } | ||
| }, |
There was a problem hiding this comment.
The PR description mentions that this change enables projects using moduleResolution: "node" to resolve imports. While the addition of the top-level main field fixes the root import, subpaths like @iqai/alert-logger/nestjs will still fail to resolve under the legacy node resolution strategy because it does not support the exports field.
To support subpath resolution in legacy environments, you should add a typesVersions mapping for TypeScript and consider using the "proxy directory" pattern (placing a package.json in a nestjs/ folder at the root) for Node.js resolution.
| "main": "./dist/index.cjs", | ||
| "module": "./dist/index.js", | ||
| "types": "./dist/index.d.ts", |
There was a problem hiding this comment.
Adding CommonJS support alongside ESM introduces the Dual Package Hazard. Since AlertLogger appears to be a singleton (based on getInstance() calls in the Next.js integration), a project that accidentally loads both the ESM and CJS versions (e.g., through different dependencies) will end up with two separate instances of the logger. This means configuration applied to one instance will not be reflected in the other.
Consider ensuring the singleton state is shared across both formats by using a Symbol.for() on the global object within the AlertLogger implementation, or by making the CJS build a thin wrapper around the ESM build (though the latter is difficult in pure Node.js).
Summary
require,main,module,typesfields to package.json exportsmoduleResolution: "node"to resolve imports without manual tsconfig paths workaroundsTest plan
pnpm buildproduces both.js(ESM) and.cjs(CJS) output🤖 Generated with Claude Code
Closes #15