Search 3.0: honor the site's date_format option in result cards#48763
Search 3.0: honor the site's date_format option in result cards#48763kangzj wants to merge 3 commits into
Conversation
|
@claude please review this PR. |
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Claude finished @kangzj's task in 2m 46s —— View job PR Review
Overall this is a well-designed feature. The architecture (seed on PHP, self-contained JS parser, fallback for legacy callers) is exactly right for the IAPI constraint. A few things worth noting:
|
| Token | Meaning | Risk |
|---|---|---|
W |
ISO week number (01–53) |
Low — rarely used in date_format |
U |
Unix timestamp | Low — rarely used in date_format |
z |
Day of year (0–365) |
Low |
t |
Number of days in the month | Low |
c / r |
ISO 8601 / RFC 2822 compound formats | Low |
The default: return token pass-through is the right choice — it makes the gap visible rather than silently dropping characters. The file-level JSDoc already enumerates supported tokens, so the contract is documented. Just worth confirming this is an accepted limitation rather than an oversight.
w token (0=Sunday, 6=Saturday) vs N token (1=Monday, 7=Sunday) — subtle but correct
N is (( getUTCDay() + 6 ) % 7 ) + 1 — Sunday (getUTCDay()=0) → (0+6)%7+1 = 7 ✓. This is easy to get wrong; it's correct here.
Untested edge case in 12-hour clock tokens
h and g use % 12 || 12 which correctly maps midnight (0) and noon (12) to 12. The tests cover 10:30 and 22:30 but not the midnight/noon boundary. Not a bug — the logic is clearly right — but a 00:00 test would close the loop.
// Example missing test
const midnight = new Date( Date.UTC( 2026, 3, 20, 0, 0, 0 ) );
expect( formatWpDate( midnight, 'h:i', 'en-US' ) ).toBe( '12:00' ); // not '00:00'
const noon = new Date( Date.UTC( 2026, 3, 20, 12, 0, 0 ) );
expect( formatWpDate( noon, 'g:i', 'en-US' ) ).toBe( '12:00' );getIntlFormatter key composition
const key = `${ locale }|${ Object.entries( options ).flat().join( ',' ) }`;Object.entries order is insertion-order for string keys in all modern engines, and all options objects here are single-key literals — so in practice this is stable. It would silently produce a wrong cache hit if someone passed { month: 'short', timeZone: 'UTC' } vs { timeZone: 'UTC', month: 'short' }, but since timeZone is always injected via the spread inside getIntlFormatter itself and not passed in options, this is fine as-is.
D / M tokens and locale-specific abbreviation punctuation
Intl.DateTimeFormat with { month: 'short' } returns locale-specific short names, which in some locales include trailing periods (e.g. German "Apr." vs PHP's "Apr"). This is actually the correct behavior — WP's own dateI18n similarly defers to locale translation tables for short month names. Worth documenting in the JSDoc if not already: the output for D/M follows the browser's ICU data, not PHP's own translation strings.
PHP: get_option default vs WordPress built-in default
'dateFormat' => function_exists( 'get_option' )
? (string) get_option( 'date_format', '' )
: '',WordPress's built-in default for date_format is 'F j, Y' (from wp-includes/option.php). Using '' as the fallback here is intentional — it tells the JS side to use legacy toLocaleDateString rather than a format the user hasn't explicitly configured. However, if date_format is somehow absent from the options table on a fresh install (before WordPress sets its defaults), the JS falls back to the legacy shape rather than the WP default F j, Y. This is a reasonable tradeoff since the option should always be present, and the legacy shape is not broken — just not matching the setting.
PHP test: cleanup pattern is correct
try {
update_option( 'date_format', 'Y-m-d' );
$state = Search_Blocks::build_initial_state();
$this->assertSame( 'Y-m-d', $state['dateFormat'] );
} finally {
if ( false === $original ) {
delete_option( 'date_format' );
} else {
update_option( 'date_format', $original );
}
}This is the right cleanup pattern — finally guarantees restoration even on assertion failure. ✓
index.js — threading looks correct
Both normalizeResult call sites pass state.dateFormat as the fourth argument:
- Line 751:
normalizeResult( r, state.locale, state.searchQuery, state.dateFormat ) - Line 822: same
Both are consistent with the updated signature. ✓
Overall
The implementation is solid — UTC-consistent date reading, correct ordinal suffix (including the 11th/12th/13th exception), correct ISO week year boundary, clean fallback for legacy callers, and good test coverage for the key cases. The suggestions above are minor; none are blockers.
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Search plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Result cards rendered post dates through `toLocaleDateString` with a fixed
`{ year, month, day }` shape, so "April 20, 2026" (the WP default
`F j, Y`) came back as "Apr 20, 2026" and other configured formats
(`Y-m-d`, `d/m/Y`, `l, F jS Y`, …) were ignored entirely.
Seed `get_option( 'date_format' )` into the Interactivity state and
parse it client-side in a new `wp-date-format.js` module covering every
PHP `date()` token a WP `date_format` setting is likely to use plus the
`\` literal-escape. Month and day names continue to use the seeded
blog locale via `Intl.DateTimeFormat` since the Interactivity API view
bundle still can't import `@wordpress/i18n` / `@wordpress/date`.
Fixes RSM-269.
…Doc (comment IC_kwDOAOho7M8AAAABCHRcnw) Adds explicit coverage that `h` / `g` 12-hour tokens render midnight and noon as `12`, not `00`, matching PHP `date()` semantics. Clarifies in the module JSDoc that `F` / `M` / `l` / `D` tokens follow the browser's ICU data — so locale-specific abbreviation conventions (German `"Apr."` vs `"Apr"`) apply, mirroring `dateI18n()`.
697f1d1 to
8bda38d
Compare
|
Addressing claude[bot]'s review (comment #4436810911) in
Missing-tokens (W, U, z, t, c, r), the |
|
@claude please re-review. |
|
Claude finished @kangzj's task in 1m 17s —— View job Re-Review
The commit Midnight/noon tests — ✓ correctThe new test at it( 'maps midnight and noon onto the 12-hour clock as 12, not 00', () => {
const midnight = new Date( Date.UTC( 2026, 3, 20, 0, 0, 0 ) );
expect( formatWpDate( midnight, 'h:i A', 'en-US' ) ).toBe( '12:00 AM' ); // h ✓
expect( formatWpDate( midnight, 'g:i a', 'en-US' ) ).toBe( '12:00 am' ); // g ✓
const noon = new Date( Date.UTC( 2026, 3, 20, 12, 0, 0 ) );
expect( formatWpDate( noon, 'h:i A', 'en-US' ) ).toBe( '12:00 PM' ); // h noon ✓
expect( formatWpDate( noon, 'g:i a', 'en-US' ) ).toBe( '12:00 pm' ); // g noon ✓
} );This also implicitly verifies JSDoc update for ICU-based tokens — ✓ correct
This is accurate and covers the concern about locale-specific punctuation (e.g. German Items kept as-is — confirmed appropriateThe three items flagged as accepted limitations/tradeoffs in the first review remain unchanged, per the author's note. All are documented or low-risk:
No new issuesThe full data flow ( This PR is ready to merge. |
Code Coverage SummaryCoverage changed in 3 files.
1 file is newly checked for coverage.
|
`state.dateFormat` is a one-shot value seeded by PHP at hydration and never changes for the page lifetime, so threading it through every `normalizeResult` call was unnecessary noise. Capture it once via `setSeededDateFormat()` in `result-utils.js` during `initialize()`, and have `formatDate()` read it from module scope by default. `normalizeResult()`'s signature reverts to its trunk shape (raw, locale, searchQuery); call sites in `store/index.js` no longer need to know about the format string. Tests gain a `beforeEach` reset so the module-scoped state can't leak between cases, plus coverage that the seed flows through `formatDate()` by default and that an explicit `dateFormat` argument still overrides it.
|
Claude finished @kangzj's task in 1m 25s —— View job Re-Review (post-refactor —
|
|
Don't know if this is worth the hassle... |
Fixes RSM-269
Why
Site authors set their preferred date format under Settings → General (
F j, Y,Y-m-d,d/m/Y, …) expecting every date on the site to match. Today, the Search 3.0 result card always renders dates with a fixedIntl.DateTimeFormatshape (Apr 20, 2026), so the chosen layout is silently ignored — the search results look out of place next to the rest of the theme. This PR teaches the result card to honour the site'sdate_formatsetting so search results pick up the same layout authors already see everywhere else.Proposed changes
get_option( 'date_format' )into the Interactivity API state asdateFormatalongside the existinglocaleseed.projects/packages/search/src/search-blocks/store/wp-date-format.js— a minimal PHPdate()-style token formatter (coversd D j l N S w F m M n Y y L o H h G g i s a Aplus the\literal-escape). The Interactivity API view bundle can't import@wordpress/i18n/@wordpress/date, so the parser is self-contained.dateFormatthroughnormalizeResult()→formatDate(); when supplied, the new parser renders dates token-by-token using the seeded blog locale for month / day names viaIntl.DateTimeFormat. When empty (legacy callers / older tests),formatDate()falls back to the originaltoLocaleDateStringshort-form output unchanged.formatDate/normalizeResulttests to cover the new fourth argument. PHP seed test pins the new state key.Related product discussion/links
Does this pull request change what data or activity we track or use?
No.
date_formatis an existing site setting; nothing new is collected or transmitted. The value is exposed only to the same Interactivity API state seed that already carrieslocale.Testing instructions
jp docker up -dand visit any page that hosts a Search 3.0 results region (e.g. a page containing<!-- wp:jetpack-search/search-results -->with aresults-list).F j, Y→November 5, 2024Y-m-d→2024-11-05d/m/Y→05/11/2024l, F jS Y→Tuesday, November 5th 2024\Y\e\a\r: Y→Year: 2024(\-escape passes literals through)F j, Y, and confirm month names localize (mai 4, 2026) while the layout order staysF j, Y.Screenshots
Settings → General → Date Format set to
Y-m-d,?q=helloagainst a docker env. Trunk renders the legacytoLocaleDateStringshape regardless of the WP setting; this branch renders the configured ISO format.