Skip to content

Conversation

@phillipc
Copy link
Member

@phillipc phillipc commented Dec 22, 2025

eslint) Change the definition of several "let" variables to "const."

Summary by CodeRabbit

  • Refactor
    • Tightened internal variable immutability across the codebase and tests (many let→const adjustments).
    • Purely stylistic/code-quality changes with no alterations to public APIs, behavior, or user-facing functionality.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 22, 2025

Walkthrough

Converted numerous local variable declarations from let/var to const across source and test files throughout the repository where bindings are not reassigned. No public API signatures or control flow logic were intentionally changed; edits are immutability/style focused.

Changes

Cohort / File(s) Summary
bind package
packages/bind/spec/*, packages/bind/src/*
Switched many local test and implementation variables (provider, observables, nodes, isElement, shouldApplyBindings, mapping accumulators) from let to const.
binding.core
packages/binding.core/spec/*, packages/binding.core/src/*
Replaced internal and test locals (value, modelValue, checkedValue, innerContext, etc.) with const where not reassigned.
binding.component / foreach / if / template
packages/binding.component/spec/*, packages/binding.foreach/*, packages/binding.if/*, packages/binding.template/*
Converted many test scaffolding and internal locals (provider, templateSource, templateText, container, changeMap, nodes) to const; small test helper adjusted to append/return node in one spec.
computed
packages/computed/spec/*, packages/computed/src/computed.ts
Converted numerous test locals and internal computed implementation temporaries (writeFunction, state, dependency, etc.) to const.
observable
packages/observable/spec/*, packages/observable/src/*
Replaced many locals (primitiveTypes, outputProperties, maxNestedObservableDepth, startIndex, pendingValue handling) with const; minor guarded cast added when invoking internals.
filter.punches
packages/filter.punches/src/*, packages/filter.punches/spec/*
Made internal prototype and exported filters object const; test locals switched to const.
provider. & component providers*
packages/provider.*/*
Converted provider/test variables and small parser destructuring to const.
utils packages
packages/utils*/spec/*, packages/utils*/src/*
Large set of let→const changes across utilities: dom helpers, memoization, registry, parser, jsx observer, tasks, array helpers, virtual elements; some exports changed to const (e.g., allowedBindings, hasBindingValue).
parser / utils.parser
packages/utils.parser/src/*, packages/utils.parser/spec/*
Replaced many temporary and loop variables with const (refs, deReffedArgs, result, match, etc.).
tests (general)
many packages/*/spec/*
Widespread update of test-local variables from let to const where reassignment did not occur.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~25–40 minutes

  • Rationale: Very large number of files but highly homogeneous, single-pattern edits (let/var → const). Review is mainly spot-checking rather than deep logic changes.
  • Files/areas to spot-check:
    • Any exported/module-scoped bindings converted to const (e.g., filters, allowedBindings, hasBindingValue) to ensure consumers expect immutable bindings.
    • The noted test helper change in binding.template/spec/nativeTemplateEngineBehaviors.ts that now appends and returns the created node.
    • Places where internal reassignment was intended (loops, mutation of binding references) — ensure no missed necessary let left as const.

Possibly related PRs

Poem

🐰 Hoppity-hop, const I bring,

Let goes quiet, bindings sing.
Across the files I softly prance,
Immutable steps in tidy dance.
A carrot code-hop — neat, not loud!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.54% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Prefer const' directly and concisely summarizes the main change: converting let declarations to const throughout the codebase for better immutability practices.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f09b87d and 1ef718e.

📒 Files selected for processing (3)
  • packages/binding.foreach/spec/eachBehavior.ts
  • packages/binding.template/spec/foreachBehaviors.ts
  • packages/utils/spec/arrayEditDetectionBehaviors.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/utils/spec/arrayEditDetectionBehaviors.ts
🧰 Additional context used
🪛 ast-grep (0.40.0)
packages/binding.template/spec/foreachBehaviors.ts

[warning] 171-171: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: testNode.innerHTML = 'x-'
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html

(unsafe-html-content-assignment)


[warning] 171-171: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: testNode.innerHTML = 'x-'
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html

(dom-content-modification)


[warning] 567-567: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: testNode.innerHTML = ''
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html

(dom-content-modification)


[warning] 754-755: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: testNode.innerHTML =
"

xx,
"
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html

(unsafe-html-content-assignment)


[warning] 754-755: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: testNode.innerHTML =
"

xx,
"
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html

(dom-content-modification)

packages/binding.foreach/spec/eachBehavior.ts

[warning] 857-857: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: target.innerHTML = "

    " + "
  • " + '
'
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html

(unsafe-html-content-assignment)


[warning] 865-867: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: target.innerHTML = <div data-bind='foreach: $data'> <!-- ko text: $index --><!-- /ko --><!-- ko text: $data --><!-- /ko --> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html

(unsafe-html-content-assignment)


[warning] 875-877: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: target.innerHTML = <div data-bind='foreach: $data'> <!-- ko text: $index() * 10 --><!-- /ko --><!-- ko text: $data --><!-- /ko --> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html

(unsafe-html-content-assignment)


[warning] 885-888: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: target.innerHTML = <div data-bind='foreach: $data'> <!-- ko text: $data === 'b' ? $index() * 10 : '-' --><!-- /ko --> <!-- ko text: $data --><!-- /ko --> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html

(unsafe-html-content-assignment)


[warning] 896-898: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: target.innerHTML = <div data-bind='foreach: $data'> <!-- ko text: $index() * 10 --><!-- /ko --><!-- ko text: $data --><!-- /ko --> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html

(unsafe-html-content-assignment)


[warning] 857-857: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: target.innerHTML = "

    " + "
  • " + '
'
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html

(dom-content-modification)


[warning] 865-867: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: target.innerHTML = <div data-bind='foreach: $data'> <!-- ko text: $index --><!-- /ko --><!-- ko text: $data --><!-- /ko --> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html

(dom-content-modification)


[warning] 875-877: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: target.innerHTML = <div data-bind='foreach: $data'> <!-- ko text: $index() * 10 --><!-- /ko --><!-- ko text: $data --><!-- /ko --> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html

(dom-content-modification)


[warning] 885-888: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: target.innerHTML = <div data-bind='foreach: $data'> <!-- ko text: $data === 'b' ? $index() * 10 : '-' --><!-- /ko --> <!-- ko text: $data --><!-- /ko --> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html

(dom-content-modification)


[warning] 896-898: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: target.innerHTML = <div data-bind='foreach: $data'> <!-- ko text: $index() * 10 --><!-- /ko --><!-- ko text: $data --><!-- /ko --> </div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html

(dom-content-modification)

🔇 Additional comments (2)
packages/binding.foreach/spec/eachBehavior.ts (1)

30-30: LGTM! Excellent use of const for immutable bindings.

The conversion from let to const throughout this test file is appropriate and follows JavaScript/TypeScript best practices. All changed variables are initialized once and never reassigned within their scope, making const the correct choice. This improves code quality by signaling immutability intent and preventing accidental reassignment.

Also applies to: 48-49, 55-56, 62-63, 69-70, 84-85, 101-102, 113-114, 120-121, 132-133, 145-146, 152-153, 159-161, 168-170, 178-180, 189-191, 197-199, 205-207, 213-215, 221-223, 229-231, 238-240, 430-433, 456-456, 461-461, 477-477, 480-480, 489-489, 500-500, 505-505, 514-514, 548-548, 552-552, 580-583, 586-586, 595-597, 630-630, 636-637, 643-643, 680-681, 687-688, 692-692, 735-737, 739-739, 742-742, 755-755, 760-760, 775-775, 780-780, 790-790, 797-799, 813-813, 822-824, 839-840, 848-849, 857-857, 859-859, 865-865, 869-869, 875-875, 879-879, 885-885, 890-890, 896-896, 900-900, 909-910, 917-918, 928-929, 939-940, 950-951, 957-958, 966-967, 976-977, 983-984, 992-993, 1001-1002, 1021-1021, 1031-1031, 1045-1046, 1060-1061, 1078-1079, 1097-1098, 1104-1105, 1111-1112, 1118-1119, 1125-1126

packages/binding.template/spec/foreachBehaviors.ts (1)

29-969: Excellent refactor—all letconst conversions are valid.

All the variables changed from let to const throughout this test file are never reassigned. The mutations you see (like someItems.push(), x('newValue'), planets.splice()) modify the contents of observables and arrays but don't reassign the variable bindings themselves, making const the correct choice. This improves code clarity and enforces immutability at the binding level.


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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/utils/spec/arrayEditDetectionBehaviors.ts (1)

1-1: Fix Prettier formatting issues before merging.

The pipeline detected code style issues. Please run the following command to auto-fix:

npx prettier --write packages/utils/spec/arrayEditDetectionBehaviors.ts

Copy link
Member

@brianmhunt brianmhunt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 fantastic, thank you! Looks useful and safe, so approving and merging.

@brianmhunt brianmhunt merged commit 0411267 into knockout:main Dec 22, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants