Skip to content

docs: detail build.knockout vs build.reference deltas#370

Open
phillipc wants to merge 3 commits intomainfrom
docs/build-knockout-vs-reference-deltas
Open

docs: detail build.knockout vs build.reference deltas#370
phillipc wants to merge 3 commits intomainfrom
docs/build-knockout-vs-reference-deltas

Conversation

@phillipc
Copy link
Copy Markdown
Member

Summary

Documents the concrete differences between @tko/build.knockout and @tko/build.reference so contributors and agents do not have to diff builds/knockout/src/index.ts against builds/reference/src/index.ts to answer the question.

tko.io/src/content/docs/3to4.md

  • Sharpens Start with the right package with the actual behavioral differences (strict equality, globals in bindings, functionRewrite preparser, expressionRewriting shim, ko.jsx).
  • Keeps the existing Feature comparison table as the at-a-glance summary.
  • Adds a Build deltas in detail subsection with three tables: providers wired in, binding-expression behavior, surface APIs.

tko.io/public/agents/soul.md

  • Expands the Stability and Migration section with the same provider / equality / globals / function-rewrite / surface-API deltas, so the agent guide is self-contained.

Adversarial review

Each row was verified against builds/knockout/src/index.ts and builds/reference/src/index.ts on this branch. No behavior change; docs only.

Document the concrete differences between the two builds in builds/:

- 3to4.md: add Build deltas in detail subsection with provider, expression, and surface-API tables. Keep the existing Feature comparison table as an at-a-glance summary.

- soul.md: expand the Stability and Migration section with the provider, equality, globals, function-rewrite, and surface-API deltas so agents do not need to read builds/{knockout,reference}/src/index.ts to answer this question.

Adversarial review: verified each row against builds/knockout/src/index.ts and builds/reference/src/index.ts on this branch; no behavior change, docs only.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 24, 2026

Warning

Rate limit exceeded

@phillipc has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 33 minutes and 52 seconds before requesting another review.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: aa9d51cd-0f55-491a-9401-48430282d32a

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 55195d0 and 36b7b6f.

πŸ“’ Files selected for processing (2)
  • tko.io/public/agents/soul.md
  • tko.io/src/content/docs/3to4.md
✨ Finishing Touches
πŸ§ͺ Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/build-knockout-vs-reference-deltas

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.

❀️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@brianmhunt
Copy link
Copy Markdown
Member

Adversarial review

Verified against builds/knockout/src/index.ts and builds/reference/src/index.ts on this branch:

  • βœ… Providers wired in each build
  • βœ… options.strictEquality = true in reference
  • βœ… bindingGlobals: defaultOptions.global in knockout
  • βœ… functionRewrite preparser registered in knockout only
  • βœ… expressionRewriting legacy shim in knockout only
  • βœ… ko.jsx = { createElement, Fragment, render } in reference only

Missed delta β€” foreach implementation differs

The two builds resolve data-bind="foreach: …" to different binding handlers:

  • knockout: bindings: [coreBindings, templateBindings, ifBindings, componentBindings, { each: foreachBindings.foreach }] β€” foreachBindings (the modern ForEachBinding) is not in the array. foreach resolves to TemplateForEachBindingHandler from packages/binding.template/src/index.ts ({ foreach: TemplateForEachBindingHandler, template: TemplateBindingHandler }). each is mapped to the modern ForEachBinding.
  • reference: foreachBindings is registered after templateBindings, so foreach is overridden to the modern ForEachBinding from @tko/binding.foreach. each also maps to ForEachBinding.

Net effect: knockout's foreach runs through the legacy template-engine path; reference's runs through the modern path. This is exactly the kind of behavioral delta this PR exists to surface, and it ties directly to #329 / #369 (afterRender/beforeMove/afterMove callback availability depends on which impl is active).

Suggested fix:

  • Add a row to Binding-expression behavior (or a new Bindings sub-table):

    Behavior @tko/build.knockout @tko/build.reference
    data-bind="foreach: …" resolves to TemplateForEachBindingHandler (legacy template-engine path) ForEachBinding (modern, from @tko/binding.foreach)
    data-bind="each: …" resolves to ForEachBinding ForEachBinding
  • Fix the prose in Build deltas in detail β€” "share the same core (… core/template/if/foreach/component bindings …)" should not assert foreach parity.

Minor

  1. Table style mixed. Providers / surface APIs use Yes/No; binding-expression table uses On/Off. Pick one for visual-scan parity.
  2. Phrasing. "drops the legacy functionRewrite preparser and the expressionRewriting shim" reads as if reference once shipped them. It never did. Suggest: "Has no functionRewrite preparser, no expressionRewriting shim."
  3. Redundant row. CSP-friendly parser is Yes / Yes β€” adds nothing to a deltas table. Drop, or annotate why it's worth keeping (e.g. "called out because users ask").
  4. Provider order. reference's MultiProvider order differs from knockout's, and MultiProvider resolution is order-sensitive. Not claimed in PR; worth a line if the goal is completeness.
  5. Adversarial-review log location. AGENTS.md says the audit line goes "at the end of the commit message that introduces the in-scope change β€” one audit line per in-scope commit, never the PR description". The note in this PR body should move to the commit message.

Verdict

Content is useful and the verified rows are accurate. Request changes for the foreach delta + prose claim, plus moving the adversarial-review log to the commit message. Style nits optional.

Adversarial pass: self-review. Result: clean.
@phillipc
Copy link
Copy Markdown
Member Author

phillipc commented May 5, 2026

Addressed the requested changes from review:

  • Added the missing foreach delta (TemplateForEachBindingHandler in @tko/build.knockout vs modern ForEachBinding in @tko/build.reference), plus explicit each row parity.
  • Updated the build-core prose so it no longer implies foreach parity across builds.
  • Standardized the binding-expression table to Yes/No values for scan consistency.
  • Reworded the reference-build phrasing to avoid implying it "dropped" legacy rewrite/shim behavior it never had.
  • Removed the redundant CSP row from the deltas table.
  • Added provider-order precedence notes for both builds.
  • Mirrored the same deltas in tko.io/public/agents/soul.md so agent docs stay aligned.

Verification run: bun run build in tko.io passed on this branch.

Commit: 36b7b6f3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants