Publish TypeScript declarations from the 4.x packages#402
Conversation
- Introduced a `types/` directory for each package and build to publish TypeScript declaration files. - Updated package manifests to include `types` metadata and `exports` conditions for better type resolution. - Implemented a new build script to generate declarations and clean up generated artifacts. - Enhanced the build process to ensure all public packages and builds have corresponding type definitions. - Added smoke tests to verify type imports from the generated declarations.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughMoves declaration emit to per-workspace ChangesTypeScript Declaration Publishing
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tools/types-smoke/consumer.ts (1)
1-14: ⚡ Quick winConsider expanding package coverage in the smoke test.
The smoke test currently exercises only 3 packages (
@tko/build.knockout,@tko/build.reference,@tko/observable) but the changeset advertises type publishing for 27 packages acrossbinding.*,provider.*,utils.*, and other categories. Consider adding imports from a few more representative packages to increase confidence that the declaration publishing strategy works uniformly across the workspace.🧪 Example expanded coverage
import ko from '`@tko/build.knockout`' import reference from '`@tko/build.reference`' import { observable } from '`@tko/observable`' +import { DataBindProvider } from '`@tko/provider.databind`' +import { options } from '`@tko/utils`' const name = observable('TKO') const knockoutVersion: string = ko.version const referenceVersion: string = reference.version const bindingString: string = ko.expressionRewriting.preProcessBindings('text: name') +const provider = new DataBindProvider() +const defaultOptions = options🤖 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 `@tools/types-smoke/consumer.ts` around lines 1 - 14, Add additional representative package imports and simple usages to the smoke test so types from more of the 27 packages are exercised: extend the current imports (ko, reference, observable) by importing a few packages from other groups (e.g., one or two from binding.*, one or two from provider.*, and one or two from utils.*) and use a small typed value from each (for example assign an exported const or call a small factory) similar to how name, bindingString, and jsxTree are used; keep exports (e.g., add to the exported list or create new exported consts) so those types are included at build time and ensure you reference the actual package names added in the file when editing consumer.ts.
🤖 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 `@packages/builder/src/Builder.ts`:
- Around line 320-323: The JSDoc for the create method is stale: update the
`@return` annotation on the create<T extends object[]>(...additionalProperties: T)
method so it reflects the actual signature by documenting the return type as
KnockoutInstance & AdditionalProperties<T> (or equivalent JSDoc generics),
ensuring the doc comment references both KnockoutInstance and
AdditionalProperties<T> to match the public API and improve generated
docs/editor hints.
In `@tools/build-dts.ts`:
- Around line 22-25: The current logic assumes emitted types live under
`${workspace}/src` (emittedTypesDir, indexDeclaration) and only copies from
`/src`, which misses packages whose entry is `packages/*/index.ts`; update the
code to look for and prefer declaration files in both
`${workspace}/src/index.d.ts` and `${workspace}/index.d.ts` (check both paths
before deciding missing), and copy/type-publish from whichever exists (or both)
into `${workspace}/types`; additionally add `packages/*/index.ts` to the
tsconfig.dts.json "files"/"include" inputs so TypeScript emits root package
entry declarations (adjust the tsconfig update logic to include that glob).
In `@tools/types-smoke/tsconfig.json`:
- Around line 2-11: Add an explicit "noImplicitAny": false under
"compilerOptions" to satisfy the repo guideline while keeping "strict": true;
update the tsconfig.json's compilerOptions block (where "strict", "target",
"module", "moduleResolution", "types", and "lib" are defined) to include
noImplicitAny: false so TypeScript remains strict but allows implicit any per
guideline.
---
Nitpick comments:
In `@tools/types-smoke/consumer.ts`:
- Around line 1-14: Add additional representative package imports and simple
usages to the smoke test so types from more of the 27 packages are exercised:
extend the current imports (ko, reference, observable) by importing a few
packages from other groups (e.g., one or two from binding.*, one or two from
provider.*, and one or two from utils.*) and use a small typed value from each
(for example assign an exported const or call a small factory) similar to how
name, bindingString, and jsxTree are used; keep exports (e.g., add to the
exported list or create new exported consts) so those types are included at
build time and ensure you reference the actual package names added in the file
when editing consumer.ts.
🪄 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: 7ca81b5c-b2fc-4eb5-a270-88ab772ac2d0
📒 Files selected for processing (38)
.changeset/publish-types-layout.md.gitignorebuilds/knockout/package.jsonbuilds/knockout/src/index.tsbuilds/reference/package.jsonbuilds/reference/src/index.tspackage.jsonpackages/bind/package.jsonpackages/binding.component/package.jsonpackages/binding.core/package.jsonpackages/binding.foreach/package.jsonpackages/binding.if/package.jsonpackages/binding.template/package.jsonpackages/builder/package.jsonpackages/builder/src/Builder.tspackages/computed/package.jsonpackages/filter.punches/package.jsonpackages/lifecycle/package.jsonpackages/observable/package.jsonpackages/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.jsonplans/2026-06-04-dts-types-layout.mdtools/build-dts.tstools/types-smoke/consumer.tstools/types-smoke/tsconfig.jsontsconfig.dts.json
|
On Across the full git history, every Suggest collapsing to a single object argument, which deletes both helper types and is far easier to maintain, while still giving this PR exactly what it's after (a typed default-export surface): create<T extends object>(additionalProperties: T): KnockoutInstance & T {
const instance = Object.assign(
{ /* existing getBindingHandler getter/setter */ },
knockout,
this.providedProperties,
additionalProperties
) as KnockoutInstance & T
// ...
}The two call sites already pass one object, so they're unchanged. On compatibility: Generated by Claude Code |
|
Cosmetic, but valuable while this is open: in render(jsx: any): {
node: ChildNode | DocumentFragment | null
dispose: () => void
}The shape is accurate, but it's hand-duplicated. Suggest promoting it to a named type — e.g. Generated by Claude Code |
|
Cosmetic, but valuable: in if (workspaces.length !== 27) {
throw new Error(`Expected 27 publishable workspaces, found ${workspaces.length}`)
}Suggest a named constant so the intent reads at a glance and the number isn't repeated: const WORKSPACES_EXPECTED_COUNT = 27
// ...
if (workspaces.length !== WORKSPACES_EXPECTED_COUNT) {
throw new Error(`Expected ${WORKSPACES_EXPECTED_COUNT} publishable workspaces, found ${workspaces.length}`)
}When package #26 lands and this guard trips, a named const points the next contributor straight at what needs updating. Generated by Claude Code |
|
@brianmhunt everything is changed
|
|
@brianmhunt You know I can't merge without review confirmation? We currently have several branches that have done all feedback notes and are awaiting final approval. |
2 participants
Summary
types/folders for every publicpackages/*workspace and both bundled buildstypesmetadata andexportsconditions, including preserving the extra public type surface on@tko/build.knockoutand@tko/build.referenceCloses #384.
Testing
bun run dtsbun run buildbun run verify:typesbun run tscnpm pack --dry-runinpackages/observablenpm pack --dry-runinbuilds/knockoutnpm pack --dry-runinbuilds/referencebun run verifySummary by CodeRabbit
New Features
Build Infrastructure
Chores