Publish dual ESM and CommonJS outputs by Fryuni · Pull Request #128 · croct-tech/cache-js

Fryuni · 2026-07-01T18:35:40Z

What Changed

Migrated the package to ESM with type: "module", tsup builds, and dual import/require entry points for ESM and CommonJS consumers.
Added export mappings for the root package and existing subpath imports so package consumers can continue resolving generated JS and type declarations.
Updated Jest, ESLint, README, and package-contract verification to match the new ESM build and test setup.

Risk Assessment

⚠️ Medium: The review found no substantiated material issues, but the change is a packaging/build-system and ESM/CJS migration that affects published entry points and release behavior.

Testing

The first Jest baseline run failed because the ESM test environment did not expose the global jest; after adding a test-only setup file, the full Jest suite passed, the package build/export contract passed, and a manual consumer-style ESM/CJS import check produced evidence showing the expected cache exports and methods are usable.

Evidence: ESM and CJS package consumer import transcript

{ "esm": { "inMemory": {"exportType": "function", "hasGet": true, "hasSet": true, "hasDelete": true}, "noop": {"exportType": "function", "hasGet": true, "hasSet": true, "hasDelete": true} }, "cjs": { "inMemory": {"exportType": "function", "hasGet": true, "hasSet": true, "hasDelete": true}, "noop": {"exportType": "function", "hasGet": true, "hasSet": true, "hasDelete": true} } }

{
  "esm": {
    "inMemory": {
      "exportType": "function",
      "hasGet": true,
      "hasSet": true,
      "hasDelete": true
    },
    "noop": {
      "exportType": "function",
      "hasGet": true,
      "hasSet": true,
      "hasDelete": true
    }
  },
  "cjs": {
    "inMemory": {
      "exportType": "function",
      "hasGet": true,
      "hasSet": true,
      "hasDelete": true
    },
    "noop": {
      "exportType": "function",
      "hasGet": true,
      "hasSet": true,
      "hasDelete": true
    }
  }
}

Pipeline

Updates from git push no-mistakes

⏭️ **intent** - skipped

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 3 issues found → auto-fixed ✅

🚨 tsup.config.ts:19 - The old build used tsc -p tsconfig.build.json to emit declarations, but the new tsup config only builds JS and never enables declaration output while package.json points types/export type conditions at build/index.d.ts and build/index.d.cts; published TypeScript consumers will resolve to files that are not produced.
⚠️ package.json:26 - Adding an exports map with only "." removes previously publishable subpath entry points such as @croct/cache/inMemory that existed because the old tsc build emitted every source module and published without an exports map; existing deep-import consumers will now fail with package subpath export errors unless this is an intentional breaking release.
⚠️ README.md:223 - The README now tells maintainers to run npm run test:package, but package.json does not define that script, so the documented package-contract verification command fails immediately.

🔧 Fix: Restore package contract exports
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

npm test -- --runInBand (initial run exposed ESM Jest global setup failure)
Added test/setup-jest.mjs and referenced it from jest.config.mjs so existing tests can use jest.fn/jest.spyOn in ESM mode
npm test -- --runInBand
npm run test:package
node --input-type=module -e "import {createRequire} from 'node:module'; import {InMemoryCache as EsmInMemoryCache} from '@croct/cache/inMemory'; import {NoopCache as EsmNoopCache} from '@croct/cache'; const require = createRequire(import.meta.url); const {InMemoryCache: CjsInMemoryCache} = require('@croct/cache/inMemory'); const {NoopCache: CjsNoopCache} = require('@croct/cache'); const describeCache = Cache => ({exportType: typeof Cache, hasGet: typeof new Cache().get === 'function', hasSet: typeof new Cache().set === 'function', hasDelete: typeof new Cache().delete === 'function'}); console.log(JSON.stringify({esm:{inMemory:describeCache(EsmInMemoryCache), noop:describeCache(EsmNoopCache)}, cjs:{inMemory:describeCache(CjsInMemoryCache), noop:describeCache(CjsNoopCache)}}, null, 2));" > /tmp/no-mistakes-evidence/01KWFDTZK94WTS2XEYQ8S5JR56/package-consumer-transcript.json
Removed generated build/ and coverage/ directories after verification

✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

github-actions · 2026-07-01T18:35:52Z

👋 @Fryuni

Thanks for your contribution!

The approval and merge process is almost fully automated 🧙

Here's how it works:

You open a new pull request
Automated tests check the code
Maintainers review the code
Once approved, the PR is ready to merge.

👉 Omit the extended description
Please remove the commit body before merging the pull request.
Instead, include the pull request number in the title to provide the full context
about the change.

☝️ Lastly, the title for the commit will come from the pull request title. So please provide a descriptive title that summarizes the changes in 50 characters or less using the imperative mood.

Happy coding! 🎉

This reverts commit 842b8c8. Revert "no-mistakes: apply CI fixes" This reverts commit d7349a1. Revert "no-mistakes: apply CI fixes" This reverts commit e3ead78.

renan628

Review of the ESM/CJS migration. The runtime side holds up well — I verified that @croct/time, @croct/logging, @croct/json and node-object-hash are all CJS-only and that cjs-module-lexer correctly detects every named export used, that the release workflow order (npm ci → npm run build → prepare → publish from .) is sound with files taking precedence over .gitignore, and that tsup 8.5.1 does support CJS code splitting (so class identity is preserved across entry points).

The issues below are in the packaging contract, the lint configuration, and the package-contract test. Two of them are blocking.

renan628 · 2026-08-07T14:28:51Z

+      },
+      "default": "./build/index.js"
+    },
+    "./*": {


@croct/cache/package.json becomes unresolvable

The ./* pattern captures package.json and maps it to build/package.json.js, which will never exist. Verified:

import.meta.resolve('@croct/cache/package.json') -> file:///.../build/package.json.js

This breaks bundlers and any tooling that reads the package manifest.

Add before ./*:

"./package.json": "./package.json",

renan628 · 2026-08-07T14:28:51Z

-  "homepage": "https://github.com/croct-tech/cache-js",
+  "sideEffects": false,
+  "type": "module",
+  "exports": {


Deep imports stop resolving types under moduleResolution: node

The package used to be published with a flat layout, so @croct/cache/inMemory resolved physically to inMemory.d.ts. It now only exists through exports, which node10 resolvers ignore — runtime still works, but types break. Sibling repos (cache-ioredis-js, admin-backend, cli) are still on "moduleResolution": "Node".

Add a fallback:

"typesVersions": {"*": {"*": ["./build/*.d.ts"]}}

renan628 · 2026-08-07T14:28:52Z


 export default defineConfig(
-    configs.typescript,
+    configs.typescript.map(config => ({


This .map() silently disables every rule on .mjs files

In @croct/eslint-plugin@0.8.3, the @croct/javascript block (which carries all base rules) has no files. The ?? ['**/*.ts','**/*.tsx'] scopes it to TS, so the '*.mjs' 'test/*.mjs' globs added in package.json:61 are inert.

Proof: line 1 of this file uses import { defineConfig } with spaces, violating @stylistic/object-curly-spacing, and lint still passes.

Rewrite using extends — it already propagates files to the extended configs — instead of rewriting the preset's structure:

{files: ['**/*.ts', '**/*.tsx'], extends: [configs.typescript], languageOptions: {parserOptions: {projectService: {allowDefaultProject: ['*.config.ts']}, tsconfigRootDir: import.meta.dirname}}}, {files: ['**/*.mjs'], extends: [configs.javascript], languageOptions: {ecmaVersion: 2022}, rules: {'import-x/no-default-export': 'off'}},

ecmaVersion: 2022 is required because the preset sets 2018 and test/package-contract.mjs uses top-level await.

renan628 · 2026-08-07T14:28:52Z

    {
        files: ['src/**/*.ts'],
        rules: {
+            'import-x/extensions': 'off',


Turning this off for all of src hides the inconsistency

The rule was disabled to accommodate @croct/time/defaultClockProvider.js, but three conventions now coexist unenforced: package subpath with .js in src/holdWhileRevalidate.ts:3, without .js in test/holdWhileRevalidate.test.ts:2 and test/staleWhileRevalidate.test.ts:2, and extensionless relative imports.

eslint-plugin-import-x@4.16.1 supports a targeted override — keep the rule and carve out only where the extension is mandatory:

'import-x/extensions': ['error', 'never', { d: 'always', json: 'always', pathGroupOverrides: [{pattern: '@croct/time/**', action: 'ignore'}], }],

And add .js to both @croct/time/clock/fixedClock imports in the tests.

renan628 · 2026-08-07T14:28:52Z

+];
+
+// Dependencies that are not visible to dependents and must be bundled.
+const bundledDependencies = Object.keys(packageJson.devDependencies ?? {});


noExternal is fed every devDependency (used on line 28)

bundledDependencies resolves to typescript, eslint, jest, tsup, @types/*. The tsup docs are explicit: skipNodeModulesBundle "will still bundle modules matching the noExternal option". It's harmless today, but the moment a file in src/ imports a devDependency, the published bundle inlines the whole TypeScript compiler. The @types/* entries have no runtime code at all.

Drop bundledDependencies and the noExternal option; external + skipNodeModulesBundle already cover the case.

renan628 · 2026-08-07T14:28:52Z

+    format: ['esm', 'cjs'],
+    dts: true,
+    minify: true,
+    metafile: true,


metafile: true leaks into the published tarball

tsup writes metafile-esm.json and metafile-cjs.json into outDir (src/esbuild/index.ts:287-294), and files: ["build"] publishes the whole directory.

Remove metafile: true, or exclude it with "!build/metafile-*.json" in files.

renan628 · 2026-08-07T14:28:52Z

+        {cwd: workspace}
+    );
+
+    await exec('npm', ['pack', '--dry-run'], {cwd: root});


npm pack --dry-run verifies nothing

The output is discarded, so this only asserts an exit code that is always 0. README.md:222 claims the script validates "dry-run package contents".

Use npm pack --dry-run --json, parse it, and assert that build/index.js, build/index.cjs, build/index.d.ts and build/index.d.cts are present.

renan628 · 2026-08-07T14:28:52Z

+        "import '@croct/cache';\nimport '@croct/cache/inMemory';\n"
+    );
+
+    await exec(


The require path is never validated

The type-check only runs in a type: module workspace, so it exercises exports.import.types → .d.ts only. The require → ./build/*.d.cts mapping has zero coverage — and a missing .d.cts was one of the original review findings. It also only covers . and ./inMemory, out of 14 subpaths.

Add a second CJS workspace with an index.cts, and iterate over every module in src/*.ts.

renan628 · 2026-08-07T14:28:52Z

+    );
+
+    await exec(
+        join(root, 'node_modules', '.bin', 'tsc'),


Not portable to Windows (same for npm on line 44)

node_modules/.bin/tsc and npm need the .cmd variant on Windows.

execFile(process.execPath, [require.resolve('typescript/bin/tsc'), ...]) const npmCommand = process.platform === 'win32' ? 'npm.cmd' : 'npm';

renan628 · 2026-08-07T14:28:52Z

+await import('@croct/cache');
+await import('@croct/cache/inMemory');
+require('@croct/cache');
+require('@croct/cache/inMemory');


These assert nothing

A bare await import(...) only confirms the module loads; if an export disappears, the test stays green.

Assert the expected symbols (e.g. InMemoryCache, NoopCache) on each entry point.

Fryuni and others added 4 commits July 1, 2026 18:07

Migrate to ESM

d210d10

no-mistakes(review): Restore package contract exports

74ae526

no-mistakes(document): Document ESM package outputs

5d2eea6

no-mistakes(lint): Fix ESLint project-service config

052ef3c

Fryuni requested review from a team as code owners July 1, 2026 18:35

Fryuni added 3 commits July 1, 2026 18:39

no-mistakes: apply CI fixes

e3ead78

no-mistakes: apply CI fixes

d7349a1

no-mistakes: apply CI fixes

842b8c8

Fryuni added the maintenance Housekeeping label Jul 1, 2026

Revert "no-mistakes: apply CI fixes"

f4a9e92

This reverts commit 842b8c8. Revert "no-mistakes: apply CI fixes" This reverts commit d7349a1. Revert "no-mistakes: apply CI fixes" This reverts commit e3ead78.

Fryuni changed the title ~~feat: publish dual ESM and CommonJS outputs~~ Publish dual ESM and CommonJS outputs Jul 1, 2026

amorimjuliana assigned Fryuni Jul 31, 2026

renan628 reviewed Aug 7, 2026

View reviewed changes

Navigation Menu

Uh oh!

Publish dual ESM and CommonJS outputs - #128

Publish dual ESM and CommonJS outputs#128
Fryuni wants to merge 8 commits into
masterfrom
esm-migration

Fryuni commented Jul 1, 2026

Uh oh!

github-actions Bot commented Jul 1, 2026

Uh oh!

renan628 left a comment

Uh oh!

renan628 Aug 7, 2026

Uh oh!

renan628 Aug 7, 2026

Uh oh!

renan628 Aug 7, 2026

Uh oh!

renan628 Aug 7, 2026

Uh oh!

renan628 Aug 7, 2026

Uh oh!

renan628 Aug 7, 2026

Uh oh!

renan628 Aug 7, 2026

Uh oh!

renan628 Aug 7, 2026

Uh oh!

renan628 Aug 7, 2026

Uh oh!

renan628 Aug 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Fryuni commented Jul 1, 2026

What Changed

Risk Assessment

Testing

Pipeline

Uh oh!

github-actions Bot commented Jul 1, 2026

Uh oh!

renan628 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants