Skip to content

v3: dual ESM/CJS + TypeScript rewrite and full toolchain modernization#29

Open
Roman991 wants to merge 7 commits into
inventaire:mainfrom
Roman991:pr-28
Open

v3: dual ESM/CJS + TypeScript rewrite and full toolchain modernization#29
Roman991 wants to merge 7 commits into
inventaire:mainfrom
Roman991:pr-28

Conversation

@Roman991

Copy link
Copy Markdown

I spent a few tokens to modernize this library, which I've been using for many years, to make it easier to maintain over time.
I've already tested it on four projects in CJS and ESM without having to make any changes to the src.

Summary

This PR modernizes isbn3 to a v3 release: the source is rewritten in TypeScript, the package now ships a dual ESM + CommonJS build with generated type declarations, and the legacy toolchain (babel + browserify + uglify + mocha + should + standard + node-fetch) is replaced with a single modern stack.

No runtime behavior change: same functions, same output shape. The existing test suite was ported 1:1 and all 66 tests pass.

What changed

Packaging & modules

  • Dual ESM + CommonJS output via an exports map, so both import and require work. Named exports (parse, audit, …) and the default aggregate object are available in both module systems.
  • Generated, accurate type declarations (dist/index.d.ts / dist/index.d.cts) replace the hand-written isbn.d.ts. Fixes an incorrect type: the audit clue candidate was typed as string but is an object at runtime.
  • "engines": { "node": ">=18" } (was >=6.4.0), "sideEffects": false for tree-shaking, no sourcemaps in the published tarball (package is ~37 kB packed / 13 files).

Language & source layout

  • lib/.js (CommonJS) → src/.ts (TypeScript), centralized types in src/types.ts.
  • Bins (bin/) → src/bin/.ts, still shipped as runnable ESM with shebang preserved.

Toolchain

  • Build: tsdown (https://tsdown.dev) (Rolldown) → ESM + CJS + .d.ts/.d.cts.
  • Tests: built-in node:test + node:assert/strict, run directly on the TS sources via Node's native type stripping (no tsx, no build step for dev).
  • Lint/format: Biome (https://biomejs.dev) (replaces standard).
  • HTTP (groups updater): native global fetch (drops node-fetch).
  • Cross-platform Node scripts replace the .sh build scripts.

CI & infra

  • New CI workflow on push/PR: lint + typecheck + build + test on Node 22/24, plus a compat job that builds and smoke-tests the bundled output on Node 18/20.
  • npm publish --provenance in the auto-update workflow (OIDC already configured); the committed generated file moved from lib/groups.js to src/groups.ts.

Code cleanups (behavior-preserving)

  • Removed dead suggestCorrectChecksum.
  • Replaced deprecated String.prototype.substr.
  • parse/complete_isbn_data refactored to build the result immutably (no mutable draft + cast).
  • The five near-identical audit guess helpers collapsed into one pushCandidate helper.

Removed

  • The browserified UMD build (dist/isbn.js, dist/isbn.min.js), the dist/gh-pages branches and the window.ISBN global. Browser usage is now a CDN ESM
    import (https://cdn.jsdelivr.net/npm/isbn3@3/+esm); index.html and README updated accordingly.

Pros of merging

  • Backward-compatible consumption: dual build keeps both require('isbn3') and import working — no migration needed for CJS consumers.
  • Correct, first-class types shipped with the package (fixes the candidate typing bug).
  • Far simpler, faster toolchain: 7 dev tools collapsed into 3 (tsdown, Biome, node:test); no babel/browserify/uglify pipeline.
  • Cross-platform development (the old .sh scripts didn't run on Windows).
  • Smaller, cleaner package: tree-shakeable, no sourcemaps, ~37 kB.
  • Real CI on PRs (previously tests only ran in the monthly cron job) + npm provenance.
  • No API or output changes; full test suite ported and green.

Cons / risks of merging

  • Large diff / hard review: full source rewrite plus lib/ → src/ moves.
  • Major version bump (3.0.0) with real breaking changes: drops Node < 18, and removes the UMD browser build + window.ISBN global and the dist/gh-pages
    branches (mitigated by the CDN ESM path).
  • New toolchain to adopt: tsdown/Rolldown and Biome are newer than the previous battle-tested stack; the maintainer must be comfortable owning them. (If preferred, the build could be swapped for tsup/unbuild/plain tsc without touching the source.)
  • TypeScript 6 peer-dep friction: tsdown's dts plugin (rolldown-plugin-dts) currently declares a peer of typescript@^5, so install needs
    --legacy-peer-deps. Verified that .d.ts generation works correctly with TS 6.
  • Dev requires Node >= 22.6 (native TS type stripping) even though the published package targets Node >= 18. Contributors on older Node need to upgrade.
  • Publish flow change: the auto-update workflow now commits src/groups.ts and publishes with provenance — worth a sanity check against the maintainer's npm/OIDC setup.

Verification

  • npm run lint ✓ · npm run typecheck ✓ · npm run build ✓
  • npm test → 66/66 ✓ · npm run coverage → 100% functions
  • Smoke-tested built output: require('./dist/index.cjs'), import of ./dist/index.js, and all three CLIs.

Z User and others added 7 commits June 13, 2026 16:11
This PR improves the internal structure of isbn3 without changing any
public API behavior. All existing tests pass (66/66) and a regression
test suite (Regrets) confirms zero behavioral changes.

## Changes

### Decomposition
- **audit.js** (118 → 30 lines): Extracted clue-generating helpers to
  `lib/audit_clues.js`. Each function now has a single responsibility
  and JSDoc documentation.

### Cohesion — Inline functions → dedicated modules
- **hyphenate**: Extracted from inline arrow function in isbn.js to
  `lib/hyphenate.js`
- **asIsbn13**: Extracted from inline arrow function in isbn.js to
  `lib/as_isbn13.js`
- **asIsbn10**: Extracted from inline arrow function in isbn.js to
  `lib/as_isbn10.js`

### Naming — Honest names
- **fill.js → complete_isbn_data.js**: The old name was vague. The new
  name clearly describes that this module completes ISBN data objects
  with computed check digits and formatted versions.

### Single Responsibility
- **normalize**: Extracted from audit.js private function to
  `lib/normalize.js` — useful as a standalone utility.

### Bug Fix (Typo)
- `lookForPossibleInvalityCauses` →
  `lookForPossibleInvalidityCauses`

## Verification

All changes were verified with three independent methods:

1. **Regrets regression testing**: 5 clusters, all GREEN, zero drift
2. **Direct output comparison**: All 15 function outputs identical to
   pre-refactor baseline
3. **Fingerprint cross-match**: All 5 fingerprints match pre-refactor
   contract
- Introduced TypeScript definitions in `src/types.ts` for better type safety.
- Replaced JavaScript test files with TypeScript test files for `asIsbn10`, `asIsbn13`, `audit`, `getGroup`, `groups`, `hyphenate`, and `parse`.
- Removed outdated JavaScript test files to streamline the codebase.
- Added `tsconfig.json` for TypeScript configuration.
- Created `tsdown.config.ts` for building the package with TypeScript.
…18/20

The build toolchain (tsdown) requires Node >= 22.18, so the previous compat
job failed trying to build on Node 18/20. Split CI into: source lint/typecheck/
test on Node 22/24, a single build job that uploads dist/ as an artifact, and a
compat job that smoke-tests the prebuilt ESM/CJS output on Node 18/20 (the
emitted bundles target Node 18).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

1 participant