Repository navigation
fix(pkg): resolve conditional types for CJS and remove install-time scripts - #166
Merged
Merged
Conversation
…cripts Validate the packed artifact with publint and attw; both surfaced consumer-facing problems. • Split `exports` types per condition (`import` → `.d.ts`, `require` → `.d.cts`) for the root, `./stream`, and `./cli`. TypeScript with `moduleResolution: node16` previously reported the package as "Masquerading as ESM" for CommonJS consumers. • Ship declaration maps (`dts.sourcemap`) so the `sourceMappingURL` comments in the emitted declarations resolve instead of pointing at missing files. • Remove the `prepare` script: npm 11.20 warns on every consumer install about an unapproved install script, and the referenced file is not published. Hooks stay available through `npm run hooks:install`. • Add `devEngines` requiring Node >=22.18 for contributors; the published `engines` floor stays `>=20.19.0` because the built artifacts still run there. • Add `publint` as `check:pkg` and run it in CI.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A second-pass audit of the merged toolchain changes — packed artifact vs. the published 1.5.0, Node 20.19 and 26 runtimes, TypeScript 5.9 under
node16/bundler, andpublint/attw— found four consumer-facing problems. This fixes them.What turned up
exportshad a single top-leveltypespointing atindex.d.ts. Because the package is"type": "module", TypeScript undermoduleResolution: node16treats that file as ESM, soimportfrom a CommonJS file failed with TS1479;attwreported "Masquerading as ESM" for the root,./stream, and./cli.preparescript added in chore: replace husky and lint-staged with a Git hook #165 made npm 11.20 printinstall-scripts … unreviewed (prepare: node scripts/install-hooks.js)when installing the published package. The file it references is not in the tarball (files: ["dist/"]), so auto-installation was also a latent failure..d.tsfiles ended with//# sourceMappingURL=index.d.ts.map, butdts.sourcemapwas disabled, so the referenced maps did not exist (the old tsup artifact had neither).enginessays>=20.19.0, buttsdownneeds^22.18,vitest@5needs^22.12, and the hook tooling assumes a modern Node — someone on Node 20 followingenginescould install and then fail atnpm test.Changes
• Per-condition types:
import→.d.ts,require→.d.ctsfor the root,./stream, and./cli(browser branches included).attwis now green fornode16(CJS and ESM) andbundler.• Emit declaration maps (
dts: { sourcemap: true }); the tarball grows from 29 to 33 files, and thesourceMappingURLcomments resolve.• Remove
prepare; keepnpm run hooks:install. Installs no longer warn and no unpublished script is referenced. Trade-off: hooks are opt-in for contributors.• Add
devEngines(node >=22.18,onFail: warn).enginesstays>=20.19.0— the runtime artifacts still work there (verified below).• Add
publintasnpm run check:pkgand run it in CI, so export-map regressions fail the build.Verification
require,import, and the CLI work on Node 20.19.0 and 26.13.0.import/requireunder bothmoduleResolution: node16andbundler, includingCompiledQuery,ValidationResult,ExpressionNode, and the./streamsubpath.install-scriptswarning and no script execution.lint:ci,typecheck,build, 1,380 tests, artifact tests (21), coverage thresholds,publint,npm audit(0), andosv-scanner(0) all pass.Remaining, deliberately not changed
pkg.browserstill points at the IIFE bundle, andexports["."].browser.requireresolves to the same IIFE.publintlists this as a suggestion, not an error. Pointing it atindex.esm.js/index.cjswould be more correct for bundlers but changes a long-standing public mapping; worth a separate change with a bundler test.attwreports resolution failures for the./streamand./clisubpaths under node10 resolution. Those subpaths requireexportssupport (TypeScript ≥4.7); Node 10 is long EOL, so this is expected rather than actionable.