test(formatter): move formatter specs out of the adapter packages (#DS-3496) - #19
Merged
Merged
Conversation
The specs of native- and internationalized-date-adapter asserted through DateFormatter, so both packages depended on date-formatter in the Nx graph while date-formatter is what they are dependencies of. That cycle blocks putting a formatter suite that spans several adapters into date-formatter, which is where such a suite belongs. Move every assertion that goes through DateFormatter into packages/date-formatter/src/formatter-formats.spec.ts and leave the adapter specs to the adapter API alone. No assertion changed: the workspace still runs 452 tests, redistributed as date-formatter 13 -> 170, internationalized-date-adapter 180 -> 25, native-date-adapter 111 -> 109. Also drop the claim that jest does not pin TZ from the native spec — it has been pinned in jest.preset.js since 3843bb4 — and give moment-date-adapter the displayName every other package's jest config sets.
The relative, absolute, range and duration expectations for DateFormatter lived
in angular-components, which only wraps this library in Angular DI and cannot
see a formatting change made here until it is released and the version bumped.
Bring them across: 506 tests over ru-RU and en-US, every name from the original
accounted for.
The port is not a copy. The original spelled its date tokens out per locale and
duplicated the whole suite for ru and en; here they are read back from
`adapter.config.variables`, which collapses the duplication and makes the
expectations independent of an adapter's token dialect. Because an expectation
derived from the config would move together with a wrong token and still pass,
a snapshot per locale pins the literal output as the other half of the check.
Date arithmetic goes through the adapter rather than luxon, and `today()` is
pinned to a fixed date, so nothing drifts with the clock or the calendar.
Two tests were fixing themselves as they moved, and now assert what their names
say. The ru "Relative long" milliseconds and seconds cases called
relativeShortDateTime, duplicating the short block; the en "after tomorrow
(other year)" case measured from 2015, which is neither after tomorrow nor
reachable from today. Both pass in their corrected form.
Two oddities in the en-US templates are pinned rather than normalised, since
changing them would change the library's output: opened ranges join the year
with a bare space where every other template uses a comma ("Until Mar 16 2027"),
and the long datetime range closes with "to" after a plain space rather than
with "until" after a non-breaking one.
Also adds two cases the original lacked, for seconds and milliseconds on an
opened rangeLongDateTime.
The suite already went through the DateAdapter interface, so pointing it at moment, native and internationalized is a matter of registering them. That turns it into the cross-adapter check the token contract has been missing: 2194 tests, and native and internationalized now come out byte-identical to luxon in both locales, snapshots included. Moment is the outlier, and only ever about durations — every date and range format matches. Four defects fall out, all documented at its registration: durations of whole years and months floor to one less because they are measured against fixed average lengths; durationFormat renders through moment.utc(ms), so anything past 24 hours wraps and durationShortest reports 5:02:25 where the others report 9605:02:25; with no explicit units every non-zero unit is kept rather than the leading two; and fractions are not rounded to half steps, so "1 y" comes out as "1.0957377989167014 y". The year and month cases are skipped rather than given weaker expectations, so they will pass as written once the adapter is fixed. The rest are visible in moment's snapshot. Pulling the adapter sources into date-formatter's compilation also surfaced two members in the moment adapter missing the `override` modifier that the other three adapters already carry; date-formatter is the only package with noImplicitOverride, which is why nothing had caught it.
The ported suite runs `DateFormatter` over all four adapters, which the build configuration never caught up with. Nx still resolved `date-formatter` to a single dependency on `date-adapter`, so its `test` target hashed against inputs that exclude the code it now exercises: breaking `LuxonDateAdapter.format()` failed `luxon-date-adapter:test` while `date-formatter:test` replayed 2074 passing tests straight from the cache. The target now declares the four adapter source trees in its `inputs`. `noImplicitOverride` was set in `date-adapter` and `date-formatter` only, so the strict consumer type-checked four foreign packages through the path mappings and a missing `override` surfaced as a TS4114 inside an unrelated project's test run. Hoisted to `tsconfig.base.json`; every package's lib and spec config still type-checks unchanged, and the error now lands on the adapter's own build. `Intl.PluralRules` is ES2018 against the base `lib` of es2017, so `formatter-locales.spec.ts` did not compile under `date-formatter/tsconfig.spec.json` at all. It only ran because the package's jest config overrides the preset transform and ts-jest falls back to `tsconfig.json`. Pinned `lib` on the spec config, leaving the shipped library's type surface alone.
`acrossDay` handed the caller's own instance to `adapter.startOf`, which `MomentDateAdapter` forwards to moment's in-place `startOf`: it returned two aliases of one mutated object and rewrote the pinned `today` that `adapter.today` returns. Harmless only because the fixture sits at midnight. Everything now goes through a `startOfDay` helper that clones first. `rangeMiddleDateTime` was the one range method whose seconds and milliseconds cases were switched off, by a boolean threaded through `closedDateTime` for no other purpose. They pass; the parameter is gone and the cases run for every adapter and locale. `durationShort` kept only a two-digit millisecond value after the port, so the padding branch was covered and the three-digit one was not — it now runs both. `formatter-formats.spec.ts` read the wall clock for the fixture year and compared it against a formatter whose `today()` was also the wall clock, so no "(current year)" case could fail, in a file that already defines a `FixedTodayAdapter`. Both factories collapse into one built on it, and the year reads from the fixture. Its two cases named "from CalendarDate" built a `CalendarDateTime`; since `DateFormatter<CalendarDateTime>` cannot take a `CalendarDate` they are renamed for what they do carry, a midnight value, and the unused import is dropped. Its two top-level describes no longer collide with the ones the harness generates. Dropped the `MONTH` token, which nothing read — mutating it left all 506 native tests green — and the `setLocale` call that repeated what every adapter constructor already does, at 31 formatted dates per test. The comment above the moment suite claimed every date and range format matches luxon exactly; the snapshot next to it records `7 мар. 2015` against `7 мар 2015`, from moment's own locale data. Corrected, and CLAUDE.md no longer describes the adapter specs as the place formatter assertions live.
There was a problem hiding this comment.
🟡 Changes recommended
Enabling noImplicitOverride at the base config will introduce TypeScript compile errors until all DateAdapter subclasses consistently add the override modifier to overriding members.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR consolidates DateFormatter integration/locale formatting tests into the @koobiq/date-formatter package (instead of running them inside individual adapter packages), and tightens TypeScript override checking across the workspace.
Changes:
- Move formatter integration and format-matrix specs out of
native-/internationalized-adapter test suites intopackages/date-formatter/src(with new snapshots). - Enable
noImplicitOverrideat the workspacetsconfig.base.jsonlevel and remove redundant per-package settings. - Update Nx/Jest configuration to keep
date-formattertests correctly invalidated when adapter sources change.
File summaries
| File | Description |
|---|---|
| tsconfig.base.json | Enables noImplicitOverride globally for the workspace. |
| packages/native-date-adapter/src/adapter.spec.ts | Removes formatter integration assertions so adapter specs focus on adapter-only behavior. |
| packages/moment-date-adapter/src/adapter.ts | Adds override to selected methods (partial migration). |
| packages/moment-date-adapter/jest.config.js | Adds Jest displayName for clearer output. |
| packages/internationalized-date-adapter/src/adapter.spec.ts | Removes formatter-driven tests, leaving adapter behavior assertions. |
| packages/date-formatter/tsconfig.spec.json | Adjusts test TS lib targets for specs. |
| packages/date-formatter/tsconfig.json | Drops redundant noImplicitOverride (now inherited from base). |
| packages/date-formatter/src/formatter-locales.spec.ts | New cross-adapter, cross-locale formatter suite + snapshot coverage. |
| packages/date-formatter/src/formatter-formats.spec.ts | New format-matrix + native integration tests moved from adapter packages. |
| packages/date-formatter/src/snapshots/formatter-locales.spec.ts.snap | New snapshots backing the consolidated locale suite. |
| packages/date-formatter/project.json | Adds adapter source trees to Nx test inputs to avoid stale cached test runs. |
| packages/date-adapter/tsconfig.json | Drops redundant noImplicitOverride (now inherited from base). |
| CLAUDE.md | Updates repo guidance to reflect the new test locations and Nx inputs rationale. |
Review details
- Files reviewed: 13/13 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
"throw Error" -> "throws Error" and "more then" -> "more than", both flagged in review. Neither name is a snapshot key, so no snapshots move.
NikGurev
approved these changes
Sep 4, 2026
artembelik
approved these changes
Sep 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.