Conversation
📝 WalkthroughWalkthroughThis PR systematically adds TypeScript type resolution entries ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/repackage.mjs (1)
17-26:⚠️ Potential issue | 🟠 Major
exports["."].typespoints to a path that will not be included in published packagesLine 17 sets
typesto./index.ts, but thefileswhitelist (Lines 23–26) only includesdist/andhelpers/. When npm publishes the package, only whitelisted files are included in the tarball. The root-levelindex.tswill not be published, causing type resolution failures for consumers even though the file exists locally.Suggested fix
exports: { ".": { - types: "./index.ts", + types: "./dist/index.d.ts", require: "./dist/index.cjs", import: "./dist/index.js" },Change the types target to
./dist/index.d.ts, which is published and generated viamake dtsas noted in the project learnings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/repackage.mjs` around lines 17 - 26, The package exports object currently sets exports["."].types to "./index.ts" which won't be published because the files whitelist only includes "dist/" and "helpers/"; update the exports["."].types entry to point to the published declaration file (e.g., "./dist/index.d.ts") so consumers can resolve types (this aligns with the generated artifacts from the make dts step) and keep the files array unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/builder/package.json`:
- Line 34: The package.json exports entry sets "types": "./index.ts" but your
published files only include dist/ and helpers/, so consumers will not find the
.ts source; either generate and point to built declarations (emit .d.ts into
dist and update "types" to "./dist/index.d.ts"), or include index.ts in the
package files array (not recommended), or remove the "types" export until
declaration generation is implemented; implement declaration generation in the
package build (tsconfig "declaration": true/emitDeclarationOnly) so
dist/index.d.ts exists and update package.json exports["."].types to
"./dist/index.d.ts" (apply same change across packages builder, utils,
builds/knockout, builds/reference).
In `@packages/lifecycle/package.json`:
- Line 38: The package.json currently sets "types": "./index.ts" but the "files"
array only includes "dist/" and "helpers/", so index.ts won't be published;
either add "index.ts" to the "files" array or implement a declaration build
target and point "types" at the generated file (e.g., "./dist/index.d.ts"); to
fix, choose one: (A) add "index.ts" to the package.json "files" whitelist so the
source .ts is published, or (B) add a build target (e.g., make dts / an npm
script that runs tsc --emitDeclarationOnly to produce dist/index.d.ts), ensure
the build pipeline produces that file, and update "types" to "./dist/index.d.ts"
(also add the new build target to project docs/scripts).
In `@packages/provider.databind/package.json`:
- Line 29: The package.json "types" field currently points to "./index.ts" which
is not published; update the "types" entry in package.json (the "types" key) to
"dist/index.d.ts" and verify the package's "files" array includes the dist
folder so the generated declaration file is published; apply this same change to
all packages that currently reference "./index.ts" in packages/*/package.json.
---
Outside diff comments:
In `@tools/repackage.mjs`:
- Around line 17-26: The package exports object currently sets
exports["."].types to "./index.ts" which won't be published because the files
whitelist only includes "dist/" and "helpers/"; update the exports["."].types
entry to point to the published declaration file (e.g., "./dist/index.d.ts") so
consumers can resolve types (this aligns with the generated artifacts from the
make dts step) and keep the files array unchanged.
🪄 Autofix (Beta)
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
Run ID: 3e258c82-2a10-4920-8497-07e1f3a80059
📒 Files selected for processing (34)
builds/knockout/package.jsonbuilds/reference/package.jsonpackages/bind/package.jsonpackages/bind/src/bindingEvent.tspackages/binding.component/package.jsonpackages/binding.core/package.jsonpackages/binding.foreach/package.jsonpackages/binding.if/package.jsonpackages/binding.template/package.jsonpackages/builder/package.jsonpackages/computed/package.jsonpackages/filter.punches/package.jsonpackages/lifecycle/package.jsonpackages/observable/package.jsonpackages/observable/src/Subscription.tspackages/observable/src/index.tspackages/observable/src/subscribable.tspackages/provider.attr/package.jsonpackages/provider.bindingstring/package.jsonpackages/provider.component/package.jsonpackages/provider.databind/package.jsonpackages/provider.multi/package.jsonpackages/provider.mustache/package.jsonpackages/provider.native/package.jsonpackages/provider.virtual/package.jsonpackages/provider/package.jsonpackages/utils.component/package.jsonpackages/utils.functionrewrite/package.jsonpackages/utils.jsx/package.jsonpackages/utils.parser/package.jsonpackages/utils/package.jsontools/repackage.mjstsconfig.dts.jsontsconfig.json
| ], | ||
| "exports": { | ||
| ".": { | ||
| "types": "./index.ts", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
import json, pathlib, sys
targets = [
"packages/builder/package.json",
"packages/utils/package.json",
"builds/knockout/package.json",
"builds/reference/package.json",
]
failed = False
for t in targets:
p = pathlib.Path(t)
data = json.loads(p.read_text())
types_path = (((data.get("exports") or {}).get(".") or {}).get("types"))
files = data.get("files", [])
if not types_path:
print(f"{t}: no exports['.'].types")
continue
rel = types_path[2:] if types_path.startswith("./") else types_path
target_exists = (p.parent / rel).exists()
def norm(x):
return x.rstrip("/").lstrip("./")
rel_norm = norm(rel)
covered = any(rel_norm == norm(f) or rel_norm.startswith(norm(f) + "/") for f in files)
print(f"{t}: types={types_path}, exists={target_exists}, covered_by_files={covered}")
if not target_exists or not covered:
failed = True
if failed:
print("FAIL: at least one package has a types target that is missing or excluded by files[]")
sys.exit(1)
print("OK: all checked types targets exist and are included by files[]")
PYRepository: knockout/tko
Length of output: 477
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking if dist/index.d.ts exists for affected packages ==="
for pkg in "packages/builder" "packages/utils" "builds/knockout" "builds/reference"; do
if [ -f "$pkg/dist/index.d.ts" ]; then
echo "$pkg/dist/index.d.ts: EXISTS"
else
echo "$pkg/dist/index.d.ts: MISSING"
fi
done
echo ""
echo "=== Checking files[] whitelist in each package ==="
for pkg in "packages/builder" "packages/utils" "builds/knockout" "builds/reference"; do
if [ -f "$pkg/package.json" ]; then
files=$(python3 -c "import json; data=json.load(open('$pkg/package.json')); print(json.dumps(data.get('files', []), indent=2))")
echo "$pkg package.json files:"
echo "$files"
echo ""
fi
done
echo "=== Checking index.ts location ==="
for pkg in "packages/builder" "packages/utils" "builds/knockout" "builds/reference"; do
if [ -f "$pkg/index.ts" ]; then
echo "$pkg/index.ts: EXISTS"
else
echo "$pkg/index.ts: MISSING"
fi
doneRepository: knockout/tko
Length of output: 766
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking Makefile for dts/declaration targets ==="
for pkg in "packages/builder" "packages/utils" "builds/knockout" "builds/reference"; do
if [ -f "$pkg/Makefile" ]; then
echo "=== $pkg/Makefile ==="
cat "$pkg/Makefile"
echo ""
fi
done
echo "=== Checking root Makefile for dts target ==="
if [ -f "Makefile" ]; then
grep -A 10 "dts\|\.d\.ts" Makefile || echo "No dts/d.ts targets found in root Makefile"
fi
echo ""
echo "=== Checking tsconfig for declaration generation ==="
if [ -f "tsconfig.json" ]; then
python3 << 'PYEOF'
import json
data = json.load(open("tsconfig.json"))
comp = data.get("compilerOptions", {})
print(f"declaration: {comp.get('declaration', 'NOT SET')}")
print(f"declarationDir: {comp.get('declarationDir', 'NOT SET')}")
print(f"declarationMap: {comp.get('declarationMap', 'NOT SET')}")
PYEOF
fiRepository: knockout/tko
Length of output: 1673
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking tsconfig.dts.json ==="
if [ -f "tsconfig.dts.json" ]; then
cat tsconfig.dts.json
else
echo "tsconfig.dts.json not found"
fi
echo ""
echo "=== Checking tools/build.mk ==="
if [ -f "tools/build.mk" ]; then
head -100 tools/build.mk
else
echo "tools/build.mk not found"
fi
echo ""
echo "=== Listing dist/ contents for packages/builder ==="
if [ -d "packages/builder/dist" ]; then
ls -la packages/builder/dist/ 2>/dev/null | head -20
else
echo "packages/builder/dist/ does not exist"
fiRepository: knockout/tko
Length of output: 2999
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking builds/dts/ structure ==="
if [ -d "builds/dts" ]; then
echo "builds/dts exists:"
find builds/dts -type f | head -20
else
echo "builds/dts does not exist"
fi
echo ""
echo "=== Checking if dist/index.d.ts is ever generated ==="
find . -name "index.d.ts" -type f 2>/dev/null | grep -v node_modules | head -20
echo ""
echo "=== Checking what other similar projects use for types exports ==="
echo "Looking at all exports configurations:"
python3 << 'PYEOF'
import json
import pathlib
for pkg_file in pathlib.Path(".").rglob("package.json"):
if "node_modules" in str(pkg_file):
continue
try:
data = json.load(open(pkg_file))
exports = data.get("exports", {})
if exports and isinstance(exports, dict):
types = exports.get(".")
if types and isinstance(types, dict) and "types" in types:
print(f"{pkg_file}: types={types.get('types')}")
except:
pass
PYEOFRepository: knockout/tko
Length of output: 175
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking README for types/declarations info ==="
if [ -f "README.md" ]; then
grep -A 5 -B 5 -i "type\|\.d\.ts\|declaration" README.md | head -50
else
echo "README.md not found"
fi
echo ""
echo "=== Checking package.json in builds/knockout and builds/reference ==="
for pkg in "builds/knockout" "builds/reference"; do
if [ -f "$pkg/package.json" ]; then
echo "=== $pkg/package.json exports.types ==="
python3 -c "import json; d=json.load(open('$pkg/package.json')); print(json.dumps((d.get('exports',{}).get('.',{})), indent=2))"
echo ""
fi
done
echo "=== Looking at actual git state of these exports ==="
git log --oneline -n 20 -- "packages/builder/package.json" 2>/dev/null || echo "git log not available"
echo ""
echo "=== Full exports configuration from packages/builder ==="
python3 << 'PYEOF'
import json
data = json.load(open("packages/builder/package.json"))
print(json.dumps(data.get("exports", {}), indent=2))
PYEOFRepository: knockout/tko
Length of output: 4303
exports["."].types points to a file that won't be published.
The exports configuration lists "types": "./index.ts", but this package only publishes dist/ and helpers/ via the files array. This will cause type resolution to fail for consumers after the package is published.
This affects all packages: packages/builder, packages/utils, builds/knockout, and builds/reference.
The suggested fix of using ./dist/index.d.ts won't work—those files don't currently exist and aren't generated by the build pipeline. Declaration generation needs to be implemented: either by adding per-package .d.ts emission, updating the files array to include index.ts (which exposes source code), or removing the types export until declarations are properly built.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/builder/package.json` at line 34, The package.json exports entry
sets "types": "./index.ts" but your published files only include dist/ and
helpers/, so consumers will not find the .ts source; either generate and point
to built declarations (emit .d.ts into dist and update "types" to
"./dist/index.d.ts"), or include index.ts in the package files array (not
recommended), or remove the "types" export until declaration generation is
implemented; implement declaration generation in the package build (tsconfig
"declaration": true/emitDeclarationOnly) so dist/index.d.ts exists and update
package.json exports["."].types to "./dist/index.d.ts" (apply same change across
packages builder, utils, builds/knockout, builds/reference).
| ], | ||
| "exports": { | ||
| ".": { | ||
| "types": "./index.ts", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
pkg="packages/lifecycle"
echo "== package metadata =="
jq '{files, exports: .exports["."]}' "$pkg/package.json"
echo "== source/declaration presence on disk =="
[ -f "$pkg/index.ts" ] && echo "has $pkg/index.ts" || echo "missing $pkg/index.ts"
[ -f "$pkg/dist/index.d.ts" ] && echo "has $pkg/dist/index.d.ts" || echo "missing $pkg/dist/index.d.ts"
echo "== files that would be published (dry run) =="
(
cd "$pkg"
npm pack --dry-run --json \
| jq -r '.[0].files[].path' \
| rg -n '^(index\.ts|dist/index\.d\.ts)$' || true
)Repository: knockout/tko
Length of output: 404
🏁 Script executed:
fd -t f "Makefile" packages/lifecycleRepository: knockout/tko
Length of output: 83
🏁 Script executed:
cat packages/lifecycle/MakefileRepository: knockout/tko
Length of output: 87
🏁 Script executed:
rg "dts|declaration" packages/lifecycle --type ts --type js --type json -A 2Repository: knockout/tko
Length of output: 38
🏁 Script executed:
cat ../../tools/build.mk 2>/dev/null || cat tools/build.mkRepository: knockout/tko
Length of output: 3631
🏁 Script executed:
find . -name "tsconfig.json" -type f | head -20Repository: knockout/tko
Length of output: 71
🏁 Script executed:
cat tsconfig.jsonRepository: knockout/tko
Length of output: 1192
🏁 Script executed:
grep -r "dts" tools/ --include="*.mk" --include="*.mjs"Repository: knockout/tko
Length of output: 38
🏁 Script executed:
grep -r "\.d\.ts" packages/lifecycle/Repository: knockout/tko
Length of output: 38
🏁 Script executed:
ls -la packages/lifecycle/dist/ 2>/dev/null || echo "dist/ does not exist"Repository: knockout/tko
Length of output: 76
🏁 Script executed:
find packages/ -name "Makefile" -type f | head -5 | xargs grep -l "dts" 2>/dev/null || echo "No dts targets found in package Makefiles"Repository: knockout/tko
Length of output: 97
🏁 Script executed:
rg "declaration" packages/*/package.json | head -10Repository: knockout/tko
Length of output: 38
🏁 Script executed:
cat packages/lifecycle/package.json | grep -A 10 "exports"Repository: knockout/tko
Length of output: 294
exports["."].types points to unpublished file; suggested fix has missing build dependency
Line 38 sets "types": "./index.ts", but "files" (Lines 12-15) whitelists only "dist/" and "helpers/". The source file index.ts will not be in the published tarball, breaking TypeScript type resolution for consumers.
The suggested fix "./dist/index.d.ts" is incomplete: this file doesn't exist, and the build system has no target to generate it. The learning notes "Generate TypeScript declaration files using make dts", but this target doesn't exist yet.
Recommended solutions:
- Add
"index.ts"to the"files"array if it should be published, OR - Implement the
dtsbuild target first, then pointtypesto"./dist/index.d.ts"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/lifecycle/package.json` at line 38, The package.json currently sets
"types": "./index.ts" but the "files" array only includes "dist/" and
"helpers/", so index.ts won't be published; either add "index.ts" to the "files"
array or implement a declaration build target and point "types" at the generated
file (e.g., "./dist/index.d.ts"); to fix, choose one: (A) add "index.ts" to the
package.json "files" whitelist so the source .ts is published, or (B) add a
build target (e.g., make dts / an npm script that runs tsc --emitDeclarationOnly
to produce dist/index.d.ts), ensure the build pipeline produces that file, and
update "types" to "./dist/index.d.ts" (also add the new build target to project
docs/scripts).
| ], | ||
| "exports": { | ||
| ".": { | ||
| "types": "./index.ts", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify packages where exports["."].types points to ./index.ts while files whitelist omits it.
fd -p 'package.json' packages | while read -r f; do
types_path="$(jq -r '.exports["."].types // empty' "$f")"
[ -z "$types_path" ] && continue
pkg_dir="$(dirname "$f")"
rel="${types_path#./}"
has_file="no"
[ -f "$pkg_dir/$rel" ] && has_file="yes"
includes_root_index_ts="$(jq -r '.files[]? // empty' "$f" | rg -n '^index\.ts$' || true)"
if [ "$types_path" = "./index.ts" ]; then
echo "$f :: types=$types_path :: file_exists=$has_file :: files_mentions_index_ts=$([ -n "$includes_root_index_ts" ] && echo yes || echo no)"
fi
doneRepository: knockout/tko
Length of output: 2679
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Examine the target package file
cat -n packages/provider.databind/package.json
# Check if dist/index.d.ts exists in the target package
ls -la packages/provider.databind/dist/ 2>/dev/null | head -20 || echo "dist/ not found or is empty"
# Check another package as well to understand the pattern
echo "---"
cat -n packages/bind/package.json | head -50Repository: knockout/tko
Length of output: 2728
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if any packages correctly point types to dist/index.d.ts
fd -p 'package.json' packages | while read -r f; do
types_path="$(jq -r '.exports["."].types // empty' "$f")"
if [[ "$types_path" == *"dist"* ]]; then
echo "CORRECT: $f :: types=$types_path"
fi
done
echo "---"
# Check what files exist in a typical source package (not dist)
echo "Source files in packages/provider.databind/:"
find packages/provider.databind -type f -name "*.ts" -o -name "*.tsx" | head -10
echo "---"
# Check if there's a Makefile that builds dist and generates declarations
ls -la packages/provider.databind/ | grep -i makefile
head -20 packages/provider.databind/Makefile 2>/dev/null || echo "Makefile not readable"Repository: knockout/tko
Length of output: 387
types export points to unpublished file; update to dist/index.d.ts
Line 29 points to ./index.ts, which is not in the files array and won't ship in the published package. This breaks downstream type resolution. The root index.ts is compiled to dist/index.d.ts during the build, which is included in the published package.
Proposed fix
".": {
- "types": "./index.ts",
+ "types": "./dist/index.d.ts",
"require": "./dist/index.cjs",
"import": "./dist/index.js"
},This issue affects all 26 packages in the monorepo with the same pattern—each exports types: ./index.ts while excluding the root file from the npm package. Apply the same fix across all packages/*/package.json files.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "types": "./index.ts", | |
| ".": { | |
| "types": "./dist/index.d.ts", | |
| "require": "./dist/index.cjs", | |
| "import": "./dist/index.js" | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/provider.databind/package.json` at line 29, The package.json "types"
field currently points to "./index.ts" which is not published; update the
"types" entry in package.json (the "types" key) to "dist/index.d.ts" and verify
the package's "files" array includes the dist folder so the generated
declaration file is published; apply this same change to all packages that
currently reference "./index.ts" in packages/*/package.json.
Summary by CodeRabbit
New Features
Chores