Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gouniLee wants to merge 2 commits into
base: refactor-istring-plural-utils
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
677d396
Extract IStringFmt.js from IString.js
gouniLee Aug 7, 2026
8bec579
Register teststringfmt.js in the root test suite
gouniLee Aug 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions docs/IString-Refactoring.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ IString.js (final assembly)

---

### Phase 2: Extract IStringFmt.js
### Phase 2: Extract IStringFmt.js ← **done**

**Items to move:**

Expand All @@ -105,25 +105,33 @@ var IStringFmt = require('./IStringFmt');
Object.assign(IString.prototype, IStringFmt);
```

Also added `IString.prototype.constructor = IString;` right after the `IString.prototype = {...}` object-literal assignment. Replacing `.prototype` wholesale drops the auto-generated `constructor` property, so `formatChoice`'s internal `new this.constructor(...)` calls (used instead of a bare `IString` reference to avoid a require-time circular dependency with IStringFmt.js) resolved to `Object` instead of `IString` until this line was added.

`_testChoice`'s `IString._fncs.*` / `IString.plurals_default` references became `PluralUtils._fncs.*` / `PluralUtils.plurals_default` directly, and `setLocale`'s `IString.loadPlurals(...)` call became `PluralUtils.loadPlurals(...)`, since IStringFmt.js requires PluralUtils.js itself rather than reaching back through IString's static aliases. The `Locale` require moved from IString.js to IStringFmt.js (nothing else in IString.js used it).

**Verification:**
- The 3 external `formatChoice` callers (DurationFmt, UnitFmt, DateFmt) work unchanged
- The `format` → `formatChoice` internal call chain works correctly
- The 4 external `formatChoice` callers (DateFmt ×2, DurationFmt, UnitFmt) work unchanged ✓
- The 2 external `setLocale` callers (ResBundle, UnitFmt) work unchanged ✓
- The `format` → `formatChoice` internal call chain works correctly ✓
- New unit tests added for the formatting mixin ([test/root/teststringfmt.js](../js/test/root/teststringfmt.js)) ✓
- Existing suites (`teststrings.js`, `testpluralutils.js`, `teststringsasync.js`, `testdatefmt.js`, `testunitfmt.js`, `testnamefmt.js`, `testaddress.js`) pass unchanged ✓

**Expected result:** ~400 additional lines removed from IString.js
**Result:** IString.js reduced from 1158 to 712 lines (~446 removed); IStringFmt.js is 482 lines.

---

## Expected Final File Sizes

| File | Estimated lines | Actual (so far) |
| File | Estimated lines | Actual |
|------|----------------|-----------------|
| PluralUtils.js | ~460 | 378 (done) |
| IStringFmt.js | ~420 | — (Phase 2 pending) |
| IString.js | ~660 | 1158 (after Phase 1; Phase 2 pending) |
| IStringFmt.js | ~420 | 482 (done) |
| IString.js | ~660 | 712 (done) |

---

## Notes

- `formatChoice` internally creates `new IString(strings[i])`. After the split, `IStringFmt` does not require `IString` directly, so there is no circular dependency. By the time any `IStringFmt` method executes, `IString.prototype` already has the mixin applied.
- `_testChoice` calls `IString._fncs` (aliased to `PluralUtils._fncs`). `IStringFmt.js` can depend on `PluralUtils` directly, which is cleaner and avoids any indirect circular reference.
- `formatChoice` internally creates `new IString(strings[i])`. After the split, `IStringFmt` does not require `IString` directly, so there is no circular dependency. Instead, `formatChoice` uses `new this.constructor(...)`, which resolves to `IString` at call time via the instance's prototype chain.
- Because `IString.prototype` is replaced wholesale with an object literal (`IString.prototype = {...}`), the JS-engine-provided `constructor` property is lost in the process (it would otherwise point back to `IString` automatically). `IString.js` restores it explicitly with `IString.prototype.constructor = IString;` right after the object-literal assignment — this is required for `this.constructor` in `formatChoice` to resolve correctly.
- `_testChoice` calls `PluralUtils._fncs` and `PluralUtils.plurals_default` directly (not through the `IString._fncs`/`IString.plurals_default` aliases), and `setLocale` calls `PluralUtils.loadPlurals` directly (not `IString.loadPlurals`), since `IStringFmt.js` depends on `PluralUtils` directly, which is cleaner and avoids any indirect circular reference.
Loading