String.prototype.localeCompare: reuse collator for (string locale, no options) - #36103
Open
robobun wants to merge 9 commits into
Open
String.prototype.localeCompare: reuse collator for (string locale, no options)#36103robobun wants to merge 9 commits into
robobun wants to merge 9 commits into
Claude / Claude Code Review
completed
Jul 27, 2026 in 13m 38s
Code review found 1 potential issue
Found 1 candidates, confirmed 1. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | test/js/web/intl/intl.test.ts:186-192 |
Missing coverage: options=null must throw TypeError, not hit the fast-path cache |
Annotations
Check warning on line 192 in test/js/web/intl/intl.test.ts
claude / Claude Code Review
Missing coverage: options=null must throw TypeError, not hit the fast-path cache
Consider also asserting `expect(() => "a".localeCompare("b", "en", null)).toThrow(TypeError)` after seeding the `"en"` cache. Per ECMA-402, `options=null` must throw at `ToObject(null)`, but the idiomatic JSC gate `.isUndefinedOrNull()` (vs. the correct `.isUndefined()`) would let a warm cache silently return a number here — none of the new tests exercise `null` options, so that mistake would ship green. Same class of one-line strengthening as the five already applied on this PR.
Loading