chore: prep 4.1.0 — fix peer-dep cascade, shim removed utils API#371
Conversation
Five fixed-group packages declared `peerDependencies` on other fixed-group
packages whose only usages were in spec files (or, for `@tko/lifecycle`, a
type-only import that's erased at compile). With Changesets' fixed-version
group plus its minor→major peer promotion, any minor in the group cascaded
to a major version bump for everything — that's the v5.0 trigger from the
previous release attempt, not the actual change content.
Verified each by grepping for runtime imports in `src/`:
| Depender | Peer(s) | Real use |
|-----------------------|------------------------------------------------------------|------------------|
| @tko/bind | @tko/binding.foreach | spec only |
| @tko/binding.template | @tko/binding.if | spec only |
| @tko/lifecycle | @tko/observable | `import type` |
| @tko/utils.component | bind, binding.core, computed, provider.multi, .virtual | spec only |
| @tko/utils.parser | bind, binding.core, provider.databind | spec only |
Workspace-resolved specs do not need devDep declarations in the Bun
monorepo. No `src/` imports are affected. Lifecycle's `import type
{ Observable }` is erased post-compilation; consumers who use the type
already have `@tko/observable` via their own deps.
Verification:
- `bun run build` — all packages build clean
- `bun run tsc` — type-check passes
- `bunx @changesets/cli status --verbose` — reports 4.1.0 minor for all
27 fixed-group packages with current changesets, no majors
https://claude.ai/code/session_01Jnv99bwWtFWFmsyL7A2cJv
The polyfill modernization (commits 6f4b5f2, 517ddc1) removed four public identifiers from `@tko/utils` and one from `ko.utils`. Each was a trivial wrapper around a now-universal native API; restoring them as inline passthroughs preserves the public surface at zero behavior cost. New `packages/utils/src/compat.ts`: - `createSymbolOrString(id) => Symbol(id)` - `stringTrim(v) => String(v ?? '').trim()` - `stringStartsWith(s, p) => s.startsWith(p)` - `overwriteLengthPropertyIfSupported(fn, d) => Object.defineProperty(fn, 'length', d)` Wired through: - `packages/utils/src/index.ts` — added `export * from './compat'` - `packages/builder/src/Builder.ts` — restored `createSymbolOrString` in import list, `Utils` type, and `utils` object literal so `ko.utils.createSymbolOrString` continues to resolve Changeset updates: - `modernize-utils-dead-polyfills`: demoted from minor → patch for `@tko/utils` and `@tko/builder`. Description updated to call out the passthrough preservation. Internal probes (`useSymbols`, `functionSupportsLengthOverwrite`) had no monorepo callers and stay removed. - `fix-jsx-clean-teardown-race`: promoted from patch → minor for `@tko/utils.jsx`. Adding `options.jsxCleanBatchSize` is a public option — minor by strict semver. Cascades the fixed group to 4.1.0. `bunx @changesets/cli status --verbose` now reports all 27 fixed-group packages bumping to 4.1.0 minor, zero majors. Verification: - `bun run build` clean - `bun run tsc` clean - `createSymbolOrString` confirmed in compiled `dist/compat.js`, bundled `builds/reference/dist/browser.js`, and the `ko.utils` object literal at the same site https://claude.ai/code/session_01Jnv99bwWtFWFmsyL7A2cJv
JSDoc on each passthrough so IDEs strike through usage and signal the native equivalent: - createSymbolOrString → Symbol(identifier) - stringTrim → String(value ?? '').trim() - stringStartsWith → String.prototype.startsWith - overwriteLengthPropertyIfSupported → Object.defineProperty(fn, 'length', d) All four scheduled for removal in the next major. https://claude.ai/code/session_01Jnv99bwWtFWFmsyL7A2cJv
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces a new compatibility module in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52b008392a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export function stringStartsWith(value: string, prefix: string): boolean { | ||
| return value.startsWith(prefix) |
There was a problem hiding this comment.
The compatibility shim changes stringStartsWith semantics from the legacy helper: the previous implementation coerced falsy input to '' and returned false, while the new code calls value.startsWith(prefix) directly and throws when value is null/undefined. Because this commit is explicitly restoring removed public APIs for backward compatibility, this runtime throw can still break existing consumer code that used nullable inputs with ko.utils.stringStartsWith.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Real bug, thanks. The original stringStartsWith did string = string || '' at the top so stringStartsWith(null, 'x') returned false instead of throwing. The shim was calling .startsWith on the raw value, breaking the passthrough promise.
Fixed in 8c35203f with (value ?? '').startsWith(prefix) — same null-safety as the original, just expressed via the modern coalesce. stringTrim already does String(value ?? '').trim() and was unaffected.
Generated by Claude Code
| "@tko/computed": "^4.0.1", | ||
| "@tko/utils": "^4.0.1" | ||
| }, |
There was a problem hiding this comment.
Removing @tko/observable from peerDependencies leaves @tko/lifecycle with a public type surface that still references that package (import type { Observable } in LifeCycle.subscribe), so emitted declarations require consumers to resolve @tko/observable. Under non-hoisting/isolated installs (for example strict pnpm-style layouts), relying on it as only a transitive dependency through @tko/computed can make @tko/lifecycle type resolution fail with TS2307.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. LifeCycle.subscribe(observable: Observable, ...) is a public method, so Observable lands in the emitted .d.ts — strict-isolation installs (pnpm, yarn pnp) need @tko/observable resolvable directly, and the transitive path through @tko/computed only works under flat hoisting.
Fixed in 8c35203f by adding @tko/observable back to @tko/lifecycle's dependencies (its proper home for type-imports in public surface — peerDependencies was overkill, but full removal was too much).
I audited the other four packages where peerDeps were dropped — none of their src/ files reference the removed peers (the imports were all in spec/ only), so no public type surface depends on them. Lifecycle was the only one with this concern.
Generated by Claude Code
There was a problem hiding this comment.
Pull request overview
Prepares the 4.1.0 release line by removing intra-fixed-group peer dependency links that were forcing a major-version cascade, while restoring removed @tko/utils identifiers as deprecated compatibility shims (including re-surfacing ko.utils.createSymbolOrString via @tko/builder).
Changes:
- Remove several intra-monorepo
peerDependenciesthat are spec-only or type-only to prevent fixed-group version cascade. - Add
@tko/utilsdeprecated compat passthrough exports (newcompat.ts) and re-wirecreateSymbolOrStringback ontoko.utilsvia@tko/builder. - Adjust Changeset bump levels to match the intended semver impact (e.g., demote polyfill removal to patch; promote JSX option addition to minor).
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/utils/src/index.ts | Re-exports the new compat module from @tko/utils entrypoint. |
| packages/utils/src/compat.ts | Introduces deprecated passthrough shims for previously removed public utils identifiers. |
| packages/builder/src/Builder.ts | Re-adds createSymbolOrString to the Utils type and ko.utils object surface. |
| packages/utils.parser/package.json | Drops peer deps deemed spec-only for release-cascade prevention. |
| packages/utils.component/package.json | Drops peer deps deemed spec-only for release-cascade prevention. |
| packages/lifecycle/package.json | Drops type-only peer dep to avoid fixed-group peer cascade. |
| packages/binding.template/package.json | Drops spec-only peer dep to avoid fixed-group peer cascade. |
| packages/bind/package.json | Drops spec-only peer dep to avoid fixed-group peer cascade. |
| bun.lock | Updates lockfile metadata to reflect removed peer dependency edges. |
| .changeset/modernize-utils-dead-polyfills.md | Demotes bumps to patch and documents compat shims preserving surface area. |
| .changeset/fix-jsx-clean-teardown-race.md | Promotes @tko/utils.jsx bump to minor due to new public option. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/utils/src/compat.ts (1)
24-26: Optional: null-safety parity forstringStartsWith.
value.startsWith(prefix)throws onnull/undefined, whereas the historicalko.utils.stringStartsWithwas generally tolerant of nullish input (matching the oldstringTrimstyle). Since this is an explicit compat shim destined for removal, low-effort defensiveness keeps existing consumers from regressing:♻️ Suggested adjustment
-export function stringStartsWith(value: string, prefix: string): boolean { - return value.startsWith(prefix) -} +export function stringStartsWith(value: string, prefix: string): boolean { + return String(value ?? '').startsWith(prefix) +}Skip if you've already audited that all known callers pass strings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/utils/src/compat.ts` around lines 24 - 26, The compat shim stringStartsWith should be null-safe like the old ko.utils counterpart: in stringStartsWith(value, prefix) guard for null/undefined inputs (e.g., if value == null or prefix == null) and return false for those cases, otherwise call value.startsWith(prefix); update the function body accordingly so callers that pass nullish values don't throw at runtime.
🤖 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/utils/src/compat.ts`:
- Around line 32-35: Remove the trailing comma from the parameter list of the
function overwriteLengthPropertyIfSupported so the signature reads with two
parameters only (fn: Function, descriptor: PropertyDescriptor) and conforms to
Biome formatting rules (no trailing commas); keep the existing multiline layout
or collapse to one line as long as it stays under 120 chars.
---
Nitpick comments:
In `@packages/utils/src/compat.ts`:
- Around line 24-26: The compat shim stringStartsWith should be null-safe like
the old ko.utils counterpart: in stringStartsWith(value, prefix) guard for
null/undefined inputs (e.g., if value == null or prefix == null) and return
false for those cases, otherwise call value.startsWith(prefix); update the
function body accordingly so callers that pass nullish values don't throw at
runtime.
🪄 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: 3c0607d9-c6d3-4e35-8642-97502053e76a
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
.changeset/fix-jsx-clean-teardown-race.md.changeset/modernize-utils-dead-polyfills.mdpackages/bind/package.jsonpackages/binding.template/package.jsonpackages/builder/src/Builder.tspackages/lifecycle/package.jsonpackages/utils.component/package.jsonpackages/utils.parser/package.jsonpackages/utils/src/compat.tspackages/utils/src/index.ts
💤 Files with no reviewable changes (5)
- packages/lifecycle/package.json
- packages/bind/package.json
- packages/binding.template/package.json
- packages/utils.parser/package.json
- packages/utils.component/package.json
Three fixes from review on #371: 1. **stringStartsWith null safety (P1, codex)** — original `stringStartsWith(string, startsWith)` did `string = string || ''`, coercing falsy/null/undefined to empty so the call returned `false` without throwing. The shim was calling `.startsWith` on the raw value and would throw on `null`/`undefined`. Restored with `(value ?? '') .startsWith(prefix)`. `stringTrim` already handled null via `String(value ?? '').trim()` and stays as-is. 2. **Lifecycle's @tko/observable (P2, codex)** — `LifeCycle.subscribe` has signature `subscribe(observable: Observable, ...)`, where `Observable` is imported from `@tko/observable`. The type appears in the emitted `.d.ts`, so consumers under strict-isolation installs (pnpm-style, yarn pnp) need `@tko/observable` resolvable directly rather than transitively through `@tko/computed`. Added to `dependencies` (correct home for type-imports in public surface; was overkill as a peerDependency). 3. **Trailing-comma on overwriteLengthPropertyIfSupported (P2, coderabbitai)** — Biome's actual complaint was that the param list fits on one line under the 120-col limit and so should be collapsed, not the comma per se (Biome enforces trailing commas in genuinely multi-line lists by default). Collapsed to single line. Verification: - `bun run build` clean - `bun run tsc` clean - `bunx @biomejs/biome check` clean - `bunx @changesets/cli status` still 4.1.0 minor across the fixed group https://claude.ai/code/session_01Jnv99bwWtFWFmsyL7A2cJv
Summary
A previous release attempt minted v5.0.0 because Changesets promoted minor bumps to major across the 27-package fixed group. The trigger wasn't the change content — it was a chain of intra-fixed-group
peerDependencies(mostly spec-only or type-only) that caused any minor in the group to cascade to a major for every dependent.This PR lands the next release as 4.1.0 minor by fixing the cascade and restoring the one removed
ko.utilsidentifier as a@deprecatedpassthrough.What changed
1. Drop spurious intra-fixed-group
peerDependencies(55360445)Verified each by grepping the depender's
src/for runtime imports of the peer:@tko/bind@tko/binding.foreach@tko/binding.template@tko/binding.if@tko/lifecycle@tko/observableimport type(erased)@tko/utils.component@tko/utils.parserWorkspace-resolved specs don't need a devDep declaration in this Bun monorepo. Lifecycle's
import type { Observable }is erased post-compilation; consumers using the type already have@tko/observablevia their own deps.2. Restore removed
@tko/utilsAPI as@deprecatedcompat shims (cc5ddf19,52b00839)Commits
6f4b5f2band517ddc14removed four public identifiers. Each was a trivial wrapper around a now-universal native API; restoring them as one-line passthroughs preserves the public surface at zero behavior cost.New
packages/utils/src/compat.ts(each@deprecatedwith the standardized replacement called out for IDE hover):createSymbolOrString(id) => Symbol(id)stringTrim(v) => String(v ?? '').trim()stringStartsWith(s, p) => s.startsWith(p)overwriteLengthPropertyIfSupported(fn, d) => Object.defineProperty(fn, 'length', d)@tko/builderre-wirescreateSymbolOrStringinto theUtilstype, theutilsobject literal, and the import list soko.utils.createSymbolOrStringcontinues to resolve. Removal is scheduled for the next major.Internal probes (
useSymbols,functionSupportsLengthOverwrite) had no monorepo callers and stay removed.3. Changeset adjustments
modernize-utils-dead-polyfills:@tko/utilsand@tko/builderdemoted from minor → patch. Description updated to call out the passthrough preservation.fix-jsx-clean-teardown-race:@tko/utils.jsxpromoted from patch → minor. Addingoptions.jsxCleanBatchSizeis a public option — minor by strict semver. Cascades the fixed group to 4.1.0.Verification
bun run buildclean across all 27 packagesbun run tsccleanbunx @changesets/cli status --verbosereports all 27 fixed-group packages → 4.1.0 minor, zero majorscreateSymbolOrStringconfirmed end-to-end:packages/utils/src/compat.tspackages/utils/dist/compat.jsbuilds/reference/dist/browser.jsandbuilds/knockout/dist/browser.jsko.utilsobject literal at the same siteRelease path
After merge:
Test plan
bun run buildbun run tscbunx @changesets/cli status --verboseshows 4.1.0 minor cascadebun run verify(browser tests) — please run on CI before mergehttps://claude.ai/code/session_01Jnv99bwWtFWFmsyL7A2cJv
Generated by Claude Code
Summary by CodeRabbit
Release Notes
New Features
Chores