Extract PluralUtils from IString.js and fix plural rule evaluation bugs (Phase 1) by gouniLee · Pull Request #509 · iLib-js/iLib

gouniLee · 2026-07-22T07:58:15Z

Checklist

At least one test case is included for this feature or bug fix.
ReleaseNotes has been updated or is not needed.
This PR has passed all test cases on Node.js and Chrome Browser.
This is an API breaking change.
Requires a major version change.

Summary

Extracts the plural rule engine (plurals_default, loadPlurals, _fncs) out of
IString.js into a standalone PluralUtils.js module (Phase 1 of the IString
refactoring plan), then refactors and fixes several correctness bugs surfaced while
adding unit tests.

IString.js shrinks by ~383 lines; the extracted module is 378 lines. The public
API is unchanged — IString.loadPlurals, IString._fncs, and
IString.plurals_default are retained as backward-compatible aliases, so none of the
38 require('./IString') callers need to change.

What changed

Refactor

Move the plural rule engine into js/lib/PluralUtils.js; IString keeps aliases for
all three static members.
Internal cleanup of the engine: local fn alias (removes 27 repetitions of the
qualified name), inline matchRange into its callers, remove dead code
(firstProp), Array.isArray instead of Object.prototype.toString.call, drop
unused ilib.bind. Net −131 lines in the engine itself.

Bug fixes (all predate the extraction; all only affect the fallback path used when
Intl.PluralRules is unavailable, e.g. Node 10 with small-icu)

calculateNumberDigits: use the absolute value so CLDR operands are correct for
negative numbers; read the full exponent rather than just its first digit (e.g.
1e-21); keep large integers intact instead of mangling them through parseInt;
strip trailing zeros so the w/t operands are no longer identical to v/f.
matchRangeContinuous / inrange: treat a 3+ value list (e.g. [11,71,91]) as a
set, not a range; handle nested/mixed range lists (e.g. [[3,10],[13,19]],
[[3,4],9]).
neq: guard typeof(n) === 'object' for symmetry with eq.

Known limitation (documented, not fixed here)

The CLDR data encoding can't distinguish a two-value range 2..4 from a two-value
set 2,4 — both collapse to [2,4]. The runtime assumes "range" (correct for e.g.
Russian, wrong for Scottish Gaelic). Documented in PluralUtils.js with the root
cause in tools/cldr/genplurals.js. A proper fix requires an unambiguous data format
and regenerating every locale's plurals.json.

Tests

New js/test/root/testpluralutils.js — ~63 tests / 138 assertions covering
calculateNumberDigits (negative, exponent, large-int), set-vs-range matching,
nested/mixed lists, and several real locales (Breton, Russian, English). Passes in
all three build modes (dynamic, dynamicdata, assembled).
Verified against Intl.PluralRules for 214/215 locales over 0..200 (only
Scottish Gaelic differs — the documented data-format limitation).
Updated testunitfmt_be_BY / testunitfmt_mt_MT temperature-2 expectations for the
non-Intl branch, since the absolute-value fix changes the plural category of negative
temperatures on that path only (be-BY → "many" градусаў; mt-MT → "many" -il grad).
Intl-path expectations unchanged.

Notes for reviewers

This is Phase 1 only; Phase 2 (extracting IStringFmt.js) is planned separately and
tracked in the refactoring doc.
The bug fixes are CLDR-correct and confined to the iLib fallback path — behavior under
modern Node/browsers (Intl path) is unaffected.

Move plural rule engine (plurals_default, loadPlurals, _fncs) out of IString.js into a standalone PluralUtils.js module. IString retains backward-compatible aliases for all three static members so no callers need to change. Also add docs/IString-Refactoring.md with the full split plan.

Correctness: - Fix w/t CLDR operands in calculateNumberDigits: trailing zeros were not stripped, making w identical to v and t identical to f Cleanup: - Introduce local `fn` alias for PluralUtils._fncs, eliminating 27 repetitions of the full qualified name throughout _fncs methods - Simplify loadPlurals locale resolution to a single nested ternary - Remove firstProp: defined but never called (dead code) - Inline matchRange into its two callers (inrange, notin) and remove the one-line wrapper - matchRangeContinuous: combine duplicate return true branches; use continue to skip undefined entries instead of wrapping in if - calculateNumberDigits: remove intermediate numberDigits object; return object literals directly from each branch - getValue: collapse string branch to single ternary return - eq/neq: remove (x ? true : false) / (x === y ? true : false) idiom - or/and: remove unnecessary intermediate result variable - Replace Object.prototype.toString.call checks with Array.isArray - Remove ilib.bind from loadPlurals callback (this was unused) Net result: 422 → 291 lines (-131)

Add unit tests for PluralUtils.js and fix several bugs surfaced while writing them. All bugs predate the extraction from IString.js and only affect the fallback path used when Intl.PluralRules is unavailable. calculateNumberDigits: - use the absolute value so operands match CLDR for negative numbers - read the full exponent, not just its first digit (e.g. 1e-21) - keep large integers intact instead of mangling them via parseInt matchRangeContinuous / inrange: - treat a 3+ value list (e.g. [11,71,91]) as a set, not a range, so values between the endpoints no longer match - handle nested/mixed range lists (e.g. [[3,10],[13,19]], [[3,4],9]) neq: - guard typeof(n) === 'object' for symmetry with eq Document the remaining two-value-list ambiguity ([2,4] as range vs set) in PluralUtils.js and its root cause in tools/cldr/genplurals.js. Verified against Intl.PluralRules for 214/215 locales (Scottish Gaelic is the known data-format limitation).

…perands The CLDR operand fix in PluralUtils (using the absolute value of the number) changes the plural category of negative temperatures when iLib's own plural rules are used instead of Intl.PluralRules. For -16.6°C: - be-BY: n mod 10 = 6 (in 5..9) -> "many" (градусаў) - mt-MT: n mod 100 = 16 (in 11..19) -> "many" ("-il grad") This path is only taken where Intl.PluralRules is unavailable (e.g. Node 10 with small-icu), so the tests only failed there. Update the expected values for the non-Intl branches and add the standard nodejs/browser CLDR-version split to the mt-MT test, which previously asserted a single value. Intl-path expectations are unchanged.

ehoogerbeets · 2026-07-23T07:58:25Z

                test.equal(str, "-16,666666666666668 градуса Цэльсія");
            } else {
-                test.equal(str, "-16,666666666666668 градусы Цэльсія");
+                test.equal(str, "-16,666666666666668 градусаў Цэльсія");


Why did this change? All you did is move the code to a new file, so the results should not be any different.

The changes in this branch include not only splitting the PluralUtil.js file, but also improving the existing logic. In the process, a bug was also identified and fixed. The same conditional branch also exists in the browser case of the same test.

To explain this part in more detail:
PluralUtils.calculateNumberDigits was fixed to use Math.abs(number) before computing CLDR operands, as the spec defines operand n on the absolute value. With -16.666, the old code computed n mod 10 = -6 (no rule matched → fell through to few); the fixed code computes n mod 10 = 6, which falls in the 5..9 range → many → градусаў.

This only affects the non-Intl.PluralRules path (Node 10 / small-icu). Modern Node uses Intl.PluralRules directly and is unaffected.

ehoogerbeets · 2026-07-23T07:58:58Z

        });
        var str = uf.format(m1);
-        test.equal(str, "-16.666666666666668 grad Celsius");
+        var platform = ilib._getPlatform();


This is for the same reason I mentioned in the comment above.

gouniLee · 2026-07-30T03:07:38Z

@ehoogerbeets Could you check this PR? I have left a response to your question.

gouniLee added 5 commits July 21, 2026 16:29

Add testpluralutils.js

0c87e65

gouniLee self-assigned this Jul 22, 2026

gouniLee added 3 commits July 22, 2026 17:02

Update the ReleaseNotes.md

c933aa4

Update copyright year in PluralUtils.js

9940d47

Update the copyright year for all modified files

edc0293

gouniLee requested a review from ehoogerbeets July 23, 2026 07:44

ehoogerbeets reviewed Jul 23, 2026

View reviewed changes

gouniLee requested a review from ehoogerbeets July 30, 2026 03:05

gouniLee mentioned this pull request Aug 7, 2026

Extract IStringFmt.js from IString.js (Phase 2) #511

Open

8 tasks

gouniLee changed the title ~~Extract PluralUtils from IString.js and fix plural rule evaluation bugs~~ Extract PluralUtils from IString.js and fix plural rule evaluation bugs (Phase 1) Aug 7, 2026

Navigation Menu

Uh oh!

Extract PluralUtils from IString.js and fix plural rule evaluation bugs (Phase 1) - #509

Extract PluralUtils from IString.js and fix plural rule evaluation bugs (Phase 1)#509
gouniLee wants to merge 8 commits into
developmentfrom
refactor-istring-plural-utils

gouniLee commented Jul 22, 2026

Uh oh!

ehoogerbeets Jul 23, 2026

Uh oh!

gouniLee Jul 27, 2026

Uh oh!

ehoogerbeets Jul 23, 2026

Uh oh!

gouniLee Jul 27, 2026

Uh oh!

gouniLee commented Jul 30, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

gouniLee commented Jul 22, 2026

Checklist

Summary

What changed

Tests

Notes for reviewers

Uh oh!

ehoogerbeets Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

gouniLee Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

ehoogerbeets Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

gouniLee Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

gouniLee commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gouniLee commented Jul 30, 2026 •

edited

Loading