Ship TypeScript source exports (no install build) - #4
Conversation
Point package exports at src/, drop prepare/dist from the published layout, and smoke-test tarball install by importing .ts under Bun. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
Next review available in: 52 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughPackage entry points now target TypeScript source files. Package smoke tests no longer build or expect ChangesSource package resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
package.json (1)
31-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider removing the now-unused build pipeline.
No lifecycle script or check invokes
buildafter theprepareremoval, anddistis no longer published. If you keepbuild, add a comment or CI job that explains its purpose. If you do not need it, remove thebuildscript, thetsupdev dependency, andtsup.react-native.config.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` around lines 31 - 32, Remove the unused build pipeline by deleting the package.json build script, removing the tsup dev dependency, and deleting tsup.react-native.config.ts. Verify that no remaining scripts or checks depend on these build artifacts.tests/package/package-smoke.mjs (1)
183-193: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winResolve the React Native export through the package entry map.
The package only declares the
react-nativecondition under the root.export, so importing./node_modules/bip158/src/react-native.tsdoes not exercise the export contract. Importbip158with thereact-nativecondition applied and assert the resolved file points tosrc/react-native.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/package/package-smoke.mjs` around lines 183 - 193, Update the React Native smoke test around the `rn` import to load `bip158` through its package entry map with the `react-native` condition enabled, rather than importing `src/react-native.ts` directly. Assert that the resolved module points to `src/react-native.ts`, then retain the existing `buildBasicFilter` and output assertions against that resolved module.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Around line 9-17: Update the package entry-point configuration in package.json
so main, module, types, and the "." export resolve to published JavaScript and
declaration files under dist rather than raw src TypeScript files, including the
react-native condition as appropriate. Ensure the build emits those artifacts
and the smoke test and README Node support contract match the published entry
points.
In `@tests/package/runtime-smoke.mjs`:
- Around line 10-12: Remove the unused package smoke fixtures, including
runtime-smoke.mjs and browser-smoke.mjs, unless they are explicitly wired into
the test:package command. In the inline package smoke script, remove the
alternate backslash path check and retain only the forward-slash src/index.ts
resolution assertion.
---
Nitpick comments:
In `@package.json`:
- Around line 31-32: Remove the unused build pipeline by deleting the
package.json build script, removing the tsup dev dependency, and deleting
tsup.react-native.config.ts. Verify that no remaining scripts or checks depend
on these build artifacts.
In `@tests/package/package-smoke.mjs`:
- Around line 183-193: Update the React Native smoke test around the `rn` import
to load `bip158` through its package entry map with the `react-native` condition
enabled, rather than importing `src/react-native.ts` directly. Assert that the
resolved module points to `src/react-native.ts`, then retain the existing
`buildBasicFilter` and output assertions against that resolved module.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: afe33c10-2d65-4954-aba2-6f3c556845fd
📒 Files selected for processing (3)
package.jsontests/package/package-smoke.mjstests/package/runtime-smoke.mjs
| "main": "./src/index.ts", | ||
| "module": "./src/index.ts", | ||
| "types": "./src/index.ts", | ||
| "exports": { | ||
| ".": { | ||
| "types": "./src/index.ts", | ||
| "react-native": "./dist/react-native.js", | ||
| "import": "./dist/index.js", | ||
| "default": "./dist/index.js" | ||
| "react-native": "./src/react-native.ts", | ||
| "import": "./src/index.ts", | ||
| "default": "./src/index.ts" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
TypeScript 5.9 allowImportingTsExtensions error TS5097 importing .ts from node_modules dependency types
💡 Result:
The TS5097 error indicates that you are attempting to import a file using a TypeScript extension (e.g.,.ts,.mts,.tsx) in an environment or configuration where such imports are not permitted [1][2]. Enabling allowImportingTsExtensions is the primary way to resolve this, but there are critical limitations regarding its usage and interactions with node_modules that you should be aware of: 1. Requirements for allowImportingTsExtensions: This flag can only be used when either noEmit or emitDeclarationOnly is enabled in your tsconfig.json [3][4]. This is because the TypeScript compiler itself does not resolve these extensions at runtime; it expects your build tool (bundler, runtime, etc.) to handle the import paths [3]. 2. The node_modules Constraint: TypeScript generally does not support importing.ts files directly from node_modules [5][6]. Even if you enable allowImportingTsExtensions, the compiler often ignores or rejects attempts to resolve TypeScript files inside node_modules folders, as these are typically expected to be pre-compiled into JavaScript declaration files [5][6]. Node.js, for instance, specifically refuses to handle TypeScript files inside node_modules to discourage package authors from distributing raw source code [5][6]. 3. Recommended Approaches: If you are encountering this error while importing your own dependencies, consider the following: * Remove the extension: If the target file is in your own project, removing the.ts extension from your import statement is the standard practice [7]. * Use a bundler: If you require extensionful imports (e.g., for certain ESM-native setups), ensure your bundler (like Rollup, Vite, or Webpack) is configured to handle the resolution, and use allowImportingTsExtensions to satisfy the TypeScript compiler's type-checker [3]. * Check for pre-compilation: If the dependency is in node_modules, you likely need to ensure that the dependency provides valid.js and.d.ts files. You cannot typically "fix" an import error from a third-party node_modules library by changing your own tsconfig settings if the library itself is incorrectly distributing.ts source files [5][6]. For users on TypeScript 5.7 and later, if you are attempting to rewrite imports from.ts to.js during compilation, you may also explore the --rewriteRelativeImportExtensions compiler option, which helps manage these import paths when you need to emit JavaScript [8]. However, this does not bypass the fundamental restriction on importing raw TypeScript files from node_modules [5].
Citations:
- 1: https://typescript.tv/errors/ts5097
- 2: TS incorrectly flags a side effect-only import as TS5097 (An import path can only end with a '.ts' extension...) in
--watchmode microsoft/TypeScript#58791 - 3: https://www.typescriptlang.org/tsconfig/allowImportingTsExtensions.html
- 4: The error for allowImportingTsExtensions does not reference rewriteRelativeImportExtensions microsoft/TypeScript#60397
- 5: https://nodejs.org/api/typescript.html
- 6: https://nodejs.org/docs/latest-v26.x/api/typescript.html
- 7: https://stackoverflow.com/questions/40616272/ts-error-an-import-path-cannot-end-with-ts
- 8: https://devblogs.microsoft.com/typescript/announcing-typescript-5-7/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package.json exports/scripts/files =="
cat -n package.json | sed -n '1,80p'
echo
echo "== candidates containing "allowImportingTsExtensions", ".ts" exports, README docs =="
rg -n '"allowImportingTsExtensions"|allowImportingTsExtensions|README|Node|bun|dist|main":|"module":|"types":|"exports"' -S .
echo
echo "== source entrypoint relevant lines =="
wc -l src/index.ts src/react-native.ts
cat -n src/index.ts | sed -n '1,180p'
echo
echo "== tsconfigs =="
fd -a 'tsconfig.*\.json$' . | sort
for f in $(fd 'tsconfig.*\.json$' . | sort); do
echo "--- $f"
cat -n "$f" | sed -n '1,200p'
doneRepository: Overtorment/bip158
Length of output: 17678
🌐 Web query:
Node.js v20 TypeScript support node_modules stripping types require import .ts
💡 Result:
As of August 8, 2026, native support for running TypeScript files directly in Node.js (via type stripping) is not available in Node.js v20 [1][2]. Type stripping was introduced as an experimental feature in later versions of Node.js [2][3] and has since become stable and enabled by default in newer major releases (v23+) [4][2]. Because Node.js v20 is an older LTS release, it does not include the built-in --experimental-strip-types functionality or the ability to natively execute.ts files [4][2]. If you are using Node.js v20, you must continue to use third-party tools to run TypeScript files [4][5]. Common solutions include: 1. Transpilation tools: Using a tool like tsc (the TypeScript compiler) to output JavaScript files before running them with Node.js [6]. 2. Runtime runners: Using third-party packages such as tsx, ts-node, or vite-node, which intercept the execution process, transpile TypeScript to JavaScript in memory, and then pass it to the Node.js runtime [4][7][5]. To use these tools with Node.js v20, you would typically run them via your package manager or directly: npx tsx your-file.ts If you require native, built-in TypeScript support (where Node.js handles.ts files directly without third-party loaders), you would need to upgrade to a more recent version of Node.js (v22.18.0 or newer) [6][2]. In those newer versions, Node.js automatically strips erasable TypeScript syntax (such as type annotations and interfaces) and ignores tsconfig.json files, focusing purely on runtime execution rather than type checking [6][4][2].
Citations:
- 1: https://nodejs.org/en/blog/announcements/v20-release-announce/
- 2: https://nodejs.org/api/typescript.html
- 3: module: add --experimental-strip-types nodejs/node#53725
- 4: https://nodejs.org/docs/latest/api/typescript.html
- 5: https://nodejs.org/docs/latest-v26.x/api/typescript.html
- 6: https://nodejs.org/learn/typescript/run-natively
- 7: https://www.reddit.com/r/typescript/comments/1aswjg2/node_20_setup/
🌐 Web query:
TypeScript moduleResolution node16 allowImportingTsExtensions importing .ts inside node_modules ts5097
💡 Result:
You’re seeing TS5097 when code contains imports with an explicit “.ts” extension (e.g. import "foo.ts"), but your tsconfig doesn’t enable allowImportingTsExtensions. Key facts from the official TS docs: 1) allowImportingTsExtensions allows TypeScript-specific extensions in import specifiers like.ts/.mts/.tsx. [1] 2) This option is only permitted when noEmit or emitDeclarationOnly is enabled. [1] Important constraint about node_modules: - Node.js documentation explicitly discourages handling TypeScript files under node_modules: “Node.js refuses to handle TypeScript files inside folders under a node_modules path.” [2][3] How to fix (common, type-checking use case): - Enable allowImportingTsExtensions and make sure noEmit (or emitDeclarationOnly) is set. - Ensure moduleResolution is set to one that matches modern Node ESM/CJS behavior if you’re using module=“node16” (or related). Example tsconfig.json: { "compilerOptions": { "module": "node16", "moduleResolution": "node16", "allowImportingTsExtensions": true, "noEmit": true } } Why this resolves TS5097: - TS5097 is exactly the diagnostic raised when an import path ends with “.ts” but allowImportingTsExtensions is not enabled. [4] - The documented conditions explain why you also must use noEmit or emitDeclarationOnly when allowing these extensionful imports. [1] Notes about “importing.ts inside node_modules”: - Even if you make TypeScript type-check succeed via allowImportingTsExtensions, Node.js itself won’t run TypeScript files from inside node_modules by default. [2][3] In practice, packages in node_modules are expected to publish JS (or at least non-TS entrypoints), not raw.ts files. If you paste your current compilerOptions and one example failing import line, I can suggest the smallest compatible tsconfig changes (e.g., whether you should use moduleResolution "node16" vs "nodenext" vs "bundler").
Citations:
- 1: https://www.typescriptlang.org/tsconfig/allowImportingTsExtensions.html
- 2: https://nodejs.org/api/typescript.html
- 3: https://nodejs.org/docs/latest-v26.x/api/typescript.html
- 4: TypeScript starter API templates fail tsc due to .ts import extension microsoft/aspire#17224
Keep Node and TypeScript consumers compatible with the published entry points.
main, module, types, and the . export all point to raw .ts files, but README.md still says this package “Runs in Node” and the smoke test builds dist/index.js. Node does not execute node_modules TypeScript by default, and the current package does not re-export declarations without .ts extensions for clients that do not set allowImportingTsExtensions. Publish JS/.d.ts entry points for Node and TypeScript consumers, or update the Node support contract to require a TypeScript runtime/loader and document that requirement.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` around lines 9 - 17, Update the package entry-point
configuration in package.json so main, module, types, and the "." export resolve
to published JavaScript and declaration files under dist rather than raw src
TypeScript files, including the react-native condition as appropriate. Ensure
the build emits those artifacts and the smoke test and README Node support
contract match the published entry points.
| assert.ok( | ||
| import.meta.resolve("bip158").endsWith("/src/index.ts"), | ||
| "expected bip158 to resolve to src/index.ts", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find every reference to the package smoke fixtures.
rg -n --glob '!node_modules' 'runtime-smoke|browser-smoke|node-smoke|types-smoke|react-native-smoke'Repository: Overtorment/bip158
Length of output: 156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files relevant to tests =="
git ls-files | sed -n '/tests\/package\//p'
echo
echo "== file existence =="
for f in tests/package/runtime-smoke.mjs tests/package/package-smoke.mjs; do
if [ -f "$f" ]; then
echo "FOUND $f"; wc -l "$f"
else
echo "MISSING $f"
fi
done
echo
echo "== refs to runtime-smoke.mjs / package-smoke.mjs / fixtures text =="
rg -n --glob '!node_modules' 'runtime-smoke\.mjs|package-smoke\.mjs|runtime-smoke|package-smoke|fixtures|copyFileSync|mkdtemp|File\(.*runtime|bip158' . || true
echo
echo "== package scripts with smoke terms =="
python3 - <<'PY'
import json, pathlib
p=pathlib.Path('package.json')
if p.exists():
data=json.loads(p.read_text())
print(data.get('scripts',{}))
PY
echo
echo "== read package-smoke around inline bun lines =="
if [ -f tests/package/package-smoke.mjs ]; then
nl -ba tests/package/package-smoke.mjs | sed -n '1,240p'
fi
echo
echo "== read runtime-smoke =="
if [ -f tests/package/runtime-smoke.mjs ]; then
nl -ba tests/package/runtime-smoke.mjs | sed -n '1,80p'
fiRepository: Overtorment/bip158
Length of output: 4546
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== read package-smoke.mjs =="
python3 - <<'PY'
from pathlib import Path
for i, line in enumerate(Path('tests/package/package-smoke.mjs').read_text().splitlines(), 1):
if 55 <= i <= 195:
print(f'{i:4d}\t{line}')
PY
echo
echo "== read runtime-smoke.mjs =="
python3 - <<'PY'
from pathlib import Path
for i, line in enumerate(Path('tests/package/runtime-smoke.mjs').read_text().splitlines(), 1):
print(f'{i:4d}\t{line}')
PY
echo
echo "== read browser-smoke.mjs =="
python3 - <<'PY'
from pathlib import Path
for i, line in enumerate(Path('tests/package/browser-smoke.mjs').read_text().splitlines(), 1):
print(f'{i:4d}\t{line}')
PY
echo
echo "== exact file refs in tracked text files =="
rg -n --glob '!node_modules' --glob '!docs/**' --glob '!README.md' 'tests/package/runtime-smoke\.mjs|tests/package/browser-smoke\.mjs|node_modules/bip158/(src/)?runtime|runtime-smoke\.mjs' . || trueRepository: Overtorment/bip158
Length of output: 6928
Delete the unused smoke fixtures and remove the alternate backslash check.
package.json only runs node tests/package/package-smoke.mjs, and that inline script has no call to tests/package/runtime-smoke.mjs or tests/package/browser-smoke.mjs. Keep these tests under test:package only if they must run; otherwise delete them with the other unused fixtures. Also, the inline script accepts ...\\src\\index.ts, but import.meta.resolve returns a file: URL path, so the forward slash check covers all paths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/package/runtime-smoke.mjs` around lines 10 - 12, Remove the unused
package smoke fixtures, including runtime-smoke.mjs and browser-smoke.mjs,
unless they are explicitly wired into the test:package command. In the inline
package smoke script, remove the alternate backslash path check and retain only
the forward-slash src/index.ts resolution assertion.
Summary
src/(Bun-first)preparebuild — consumers need notsup/dist stepsrc+ README only; update package smoke for source importsTest plan
bun run checkpassesbun installin a consumer does not run a buildimportthe package and resolve tosrc/index.tsMade with Cursor
Summary by CodeRabbit
Chores
Tests