fix #290 - overloadOperator

[phillipc]

fix: add precedence parameter to overloadOperator and update usage in index.ts #290

Summary by CodeRabbit

• Bug Fixes

  • Error parsing Binding expressions with Operators == and != in reference Build #290

• New Features

  • Added a new configuration option to toggle strict equality behavior for loose-equality operators (defaults to off).

• Refactor

  • Removed the runtime API for registering operator overloads; loose-equality behavior is now controlled by the new option rather than dynamic registration.

[coderabbitai]

Warning

Rate limit exceeded

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

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 15 minutes and 49 seconds.

⌛ 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: fbbbe524-acd8-4945-ae5d-d8cc5e3f5c62

📥 Commits

Reviewing files that changed from the base of the PR and between c23f386 and 2951ea8.

📒 Files selected for processing (1)

• packages/utils/src/options.ts

📝 Walkthrough

Walkthrough

Removed the exported overloadOperator API and its runtime registrations; added an options.overloadEvilTwins flag that toggles ==/!= behavior to strict forms; removed an unused propertyChangeFired field and updated the IE autocomplete blur handler to reference propertyChangedFired.

Changes

Cohort / File(s)	Summary
Parser operator API packages/utils.parser/src/index.ts	Removed the exported overloadOperator function (no runtime API to register operators).
Operator implementations packages/utils.parser/src/operators.ts	== and != now conditionally use strict equality/inequality (=== / !==) when options.overloadEvilTwins is truthy; otherwise they preserve prior loose-equality semantics.
Options packages/utils/src/options.ts	Added new boolean option overloadEvilTwins: boolean = false to control the ==/!= behavior.
Build reference / runtime registrations builds/reference/src/index.ts	Removed runtime operator-overload registrations for == and != that relied on overloadOperator.
Binding handler fix packages/binding.core/src/value.ts	Removed the propertyChangeFired class field and updated the IE autocomplete blur listener to reference the existing propertyChangedFired variable.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

• brianmhunt

Poem

🐰 I hopped through code with nimble paws,
I swapped the twins for stricter laws,
A flag decides which path to choose,
A field removed — no more name blues,
I nibble bugs and leave small claws.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title 'fix #290 - overloadOperator' clearly identifies the main issue being addressed and the primary component involved, accurately reflecting the core changes across multiple files.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix_#290

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🤖 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.parser/src/index.ts`:
- Around line 11-17: The overloadOperator function mutates operators[op] before
validating precedence, so if the precedence check throws the operator is
partially applied; update the logic in overloadOperator to validate that
precedence is an integer before mutating the global operators map (or create a
local tmp for fn, set tmp.precedence after validation, then assign operators[op]
= tmp); reference the overloadOperator function and the operators symbol when
making the change.

🪄 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: 0e6bf427-b6f9-410b-b970-5428243348b7

📥 Commits

Reviewing files that changed from the base of the PR and between ad24da6 and 3239945.

📒 Files selected for processing (3)

• builds/reference/src/index.ts
• packages/binding.core/src/value.ts
• packages/utils.parser/src/index.ts

[Copilot]

Pull request overview

Fixes operator-precedence loss when overriding ==/!= in the reference build (issue #290) and corrects an IE autocomplete hack flag check in the value binding handler.

Changes:

• Updates overloadOperator to require and validate an integer precedence, and always assigns it to the operator function.
• Updates the reference build to overload ==/!= with strict equivalents using precedence 10.
• Fixes a typo in value binding by using propertyChangedFired consistently.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
packages/utils.parser/src/index.ts	Changes the overloadOperator API to require validated precedence and always set it on the operator.
builds/reference/src/index.ts	Updates ==/!= overload calls to pass an explicit precedence value to avoid parsing errors.
packages/binding.core/src/value.ts	Fixes incorrect propertyChangeFired reference so the IE autocomplete workaround can trigger correctly.

[brianmhunt]

Could you split this into two PRs? The typo fix in value.ts (propertyChangeFired → propertyChangedFired) and the operator overload refactor (overloadOperator → options.overloadEvilTwins) are unrelated changes — easier to review and merge independently.

[phillipc]

@brianmhunt Sure, done, sometimes git revert actually works ;)

[brianmhunt]

@phillipc This changes the default behavior for @tko/build.reference users. Previously, == was silently overloaded to === at startup via overloadOperator. With this PR, options.overloadEvilTwins defaults to false, so == reverts to loose equality.

That's a breaking change for every existing @tko/build.reference binding that uses == — they've been getting strict equality for years.

The default should be true (preserving existing behavior), with the option to explicitly set false for loose equality. Could you flip the default?

[brianmhunt]

Update on this — I pushed a fix to the default but then realized the bigger issue: the if (options.overloadEvilTwins) check in the operator functions runs on every ==/!= evaluation in every binding expression. That's the parser's hot path.

The original overloadOperator pattern was actually the right approach — mutate the operator table once at startup, zero per-call overhead. I've reverted my changes on the branch.

I think we should close this PR and keep the existing overloadOperator API. The current code works correctly and is performant. If we want to make the evil-twin behavior configurable, we can add an options.overloadEvilTwins flag that calls overloadOperator at init time (one-time cost) rather than checking a flag per evaluation.

@phillipc what do you think?

[phillipc]

@brianmhunt I know but we have no init-hook. We would then also have to differentiate between dynamic runtime options and static init options. That would certainly be the best way forward, but more extensive and should be knockout compatible designed.

[brianmhunt]

Alternative approach: PR #314

We've opened #314 with a different approach that avoids adding a branch to the hot-path operator functions.

ko.options.strictEquality

Instead of checking options.overloadEvilTwins on every ==/!= evaluation, #314 swaps the function references in the operators table at configuration time — same mechanism as the existing overloadOperator, but as a clean setter on ko.options:

ko.options.strictEquality = true   // == becomes ===, != becomes !==

Zero per-evaluation cost. Strongly typed. Fully tested (6 new tests).

Also introduces defineOption() — a plugin-friendly API for registering custom options with side-effect setters, so other packages can extend ko.options the same way.

See #314 for details.