Ordinal suffix token does not work in wp.date.format

[iandunn]

console.log( format( 'l, F jS', Date.now() ) );

Expected: Friday, April 26th
Actual: Friday, April 26

[iandunn]

This is odd, the tests in 054c5d3 pass, but the bug is reproducible in the console.

[talldan]

Hi @iandunn, @tellthemachines and I were looking into this issue. It's a tough one to solve!

It seems as though the locale dictates what's output by the format function. In the test environment the locale defaults to en.

In the browser it's whatever the user selects in their admin settings. The odd part is that even when we tested setting the locale in the browser to something like 'English (UK) (which should include the ordinal according to the settings), the ordinal still wasn't present after running the format function.

The locale is initially set by the script loader here:
https://github.com/WordPress/wordpress-develop/blob/2264493b0d6d4c4496eb3b6272c7becec9f67476/src/wp-includes/script-loader.php#L587-L625

Which is handled by the date package here:

gutenberg/packages/date/src/index.js

Lines 36 to 83 in 7e3b2ab

	export function setSettings( dateSettings ) {
	settings = dateSettings;
	// Backup and restore current locale.
	const currentLocale = momentLib.locale();
	momentLib.updateLocale( dateSettings.l10n.locale, {
	// Inherit anything missing from the default locale.
	parentLocale: currentLocale,
	months: dateSettings.l10n.months,
	monthsShort: dateSettings.l10n.monthsShort,
	weekdays: dateSettings.l10n.weekdays,
	weekdaysShort: dateSettings.l10n.weekdaysShort,
	meridiem( hour, minute, isLowercase ) {
	if ( hour < 12 ) {
	return isLowercase ? dateSettings.l10n.meridiem.am : dateSettings.l10n.meridiem.AM;
	}
	return isLowercase ? dateSettings.l10n.meridiem.pm : dateSettings.l10n.meridiem.PM;
	},
	longDateFormat: {
	LT: dateSettings.formats.time,
	LTS: null,
	L: null,
	LL: dateSettings.formats.date,
	LLL: dateSettings.formats.datetime,
	LLLL: null,
	},
	// From human_time_diff?
	// Set to `(number, withoutSuffix, key, isFuture) => {}` instead.
	relativeTime: {
	future: dateSettings.l10n.relative.future,
	past: dateSettings.l10n.relative.past,
	s: 'seconds',
	m: 'a minute',
	mm: '%d minutes',
	h: 'an hour',
	hh: '%d hours',
	d: 'a day',
	dd: '%d days',
	M: 'a month',
	MM: '%d months',
	y: 'a year',
	yy: '%d years',
	},
	} );
	momentLib.locale( currentLocale );
	setupWPTimezone();
	}

The moment library has some hard to follow docs that don't shed much light.

@youknowriad - I had a look at the git blame, it seems as though this code was originally copied from the core codebase by you, is that correct? It'd be great to find someone who has an understanding of how this is supposed to work.

[youknowriad]

I had a look at the git blame, it seems as though this code was originally copied from the core codebase by you, is that correct? It'd be great to find someone who has an understanding of how this is supposed to work.

That is correct, the idea is to try to map the WordPress PHP l10n config with the format expected by moment.

[tellthemachines]

Not sure how to fix this. What we found so far:

Running date.format() in the test and in the browser yields different results because of the locale strings set in each environment. When in test it is set to en but in browser it is en_US by default; however setting it to en_GB in WP-admin doesn't change the outcome.

Hypothesis: moment doesn't recognise the locale string or is unable to translate it to default en. The absence of the ordinal suffix in the browser indicates that moment is falling back to its default setting for dayOfMonthOrdinalParse

Questions:

• Where is the locale string being set for the test?
• Why do the browser-set locale strings not work with moment?

I'm running around in circles, so decided to stop work on this ticket for now :(

[iandunn]

Another wp.date bug that's possibly related: #16218

[tellthemachines]

OK peeps I finally understood what's going on here.

The problem is that the dayOfMonthOrdinalParse moment function is defined differently for each locale that moment knows about. The only locale moment knows about by default is 'en', and for that one the ordinal suffix works as expected.
To test, try setting moment.locale('en') in the browser console. After that, wp.date.format( 'l, F jS', Date.now() ) should show the suffix. That's why the unit test is passing: in the test environment, locale is set to 'en'.
However, we're not using any of the locale definitions optionally provided by moment. Instead, what we're doing is taking the locale string set in WP admin and loading it into moment with a custom config. That happens in the core script-loader.
The only thing moment knows about whatever custom locale we load (the locale strings we use 'en_GB', 'en_AU', etc.) is that it's not the same as 'en', so it falls back to the default definition of dayOfMonthOrdinalParse which doesn't output any suffixes. Suffixes are also not output for any other language in which they exist.

What we can do to fix this:

• If we're mainly concerned with fixing it on the user side, for English only, then passing settings.l10n.locale.split( '_' )[0] to dateMoment.locale() here should be enough to make it render correctly e.g. in the editor document publish section. But that won't make it work on the browser console, because there's a different moment instance running in the browser.
• To make it work in the browser console, for English only, in addition to the above, we'd need to change this line in the core script-loader to strtolower( explode( '_', get_user_locale() )[0] ) or something of the sort.
• To make it work for all languages will require either loading in the moment-supported locales instead of our own custom locale config, or else adding language-specific definitions of dayOfMonthOrdinalParse to our own custom locale definitions.

I'm happy to help fix this issue but would be good to have some opinions on what's the priority here, because loading in a bunch of moment locales could affect performance quite a bit.

[youknowriad]

adding language-specific definitions of dayOfMonthOrdinalParse to our own custom locale definitions.

How complex would that be? Can this definition be "generated" somehow from the php configs like we do for the strings...?

[tellthemachines]

How complex would that be? Can this definition be "generated" somehow from the php configs like we do for the strings...?

We would need to have a different regex for each language that supports ordinal suffixes, like moment has. When we define our custom locales in script-loader.php the strings are coming from a $wp-locale global; not sure where it lives but I assume it would be possible to add our regexes to each supported language in that global?

[youknowriad]

A random though:

I think the @wordpress/date format function is designed to match php behavior, if we don't have the orginal suffixes support in php, why are we trying to support them in JS as well? I mean it would be good but I wouldn't consider that a bug unless it's supported in php too?

[tellthemachines]

I think the @wordpress/date format function is designed to match php behavior, if we don't have the orginal suffixes support in php, why are we trying to support them in JS as well?

That's the thing, we do have suffix support in PHP. If you go to WP admin General Settings > Date Format, choose Custom and add the string jS F Y, then go to any post or index page and you'll see the post date appear with the correct formatting, e.g. 20th August 2019. So the get_the_date() function that themes use to output the post dates knows about suffixes, but I haven't found whereabouts in core that is defined.
(It only seems to work correctly for English though, so I'm guessing it doesn't have locale-specific suffixes.)

[iandunn]

get_the_date() calls mysql2date() which calls PHP's date().

date_i18n() also calls date().

[tellthemachines]

Soooo if the idea is having parity of support between PHP and JS, perhaps we can go with the quick-fix-it-just-for-English option? 😁