Skip to content

Rewrite dateTime.js to use current localeData#378

Open
bearfriend wants to merge 36 commits into
mainfrom
dgleckler/rewrite-dateTime
Open

Rewrite dateTime.js to use current localeData#378
bearfriend wants to merge 36 commits into
mainfrom
dgleckler/rewrite-dateTime

Conversation

@bearfriend

@bearfriend bearfriend commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

GAUD-9717: Rewrite intl/common and intl/dateTime to use contained data sets per locale

Run #1

  • Use new locale-data/
  • Do not modify tests

644 failed tests

Run #2

  • Refactor tests so they properly set the language between tests, including waiting for locale data to load
  • Fix documentLocaleSettings reset method so it runs listeners when it "resets" (changes) the language back to English. This is only used in tests.
  • Rename locale-data/ to match the CLDR source names and update how the files are matched for loading

304 failed tests

Run #3

  • Update tests with appropriate whitespace (no-break-space, rtl-mark etc.) to match new locale data. These changes will only improve UX without being disruptive.

242 failed tests

Run #4

  • Be more forgiving when relying on separators to parse dates and date formats since they may contain invisible whitespace (e.g. rtl-mark)
  • Support unicode symbols without a .NET equivalent
    • LLLL - Stand-alone month in year (e.g. Catalan: MMMM -> "d'abril", LLLL -> "abril")
    • cccc - Stand-alone day of week
    • K - Hour in day, 12 hour, 0-11 (e.g. "0:29 pm")

225 failed tests

Run #5

  • Downgrade locale-data/ files to maintain existing time formats (minus whitespace improvements and legitimate bugs)
  • Where CLDR is wrong (ko), add overrides files and rework how they are applied

102 test failures

Run #6

  • Downgrade locale-data/ files to maintain existing date formats (minus whitespace improvements and legitimate bugs)
  • Align pt and pt-BR

0 failed tests

Comment thread lib/dateTime.js Outdated
'AZT': '+0400',
'B': '+0200',
'BDST': '+0100',
'BDST': '+0200',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were just mistakes you noticed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Mistakes" AI noticed, but I don't trust it understands the context. I'll make a story to review these, but will revert these from this PR.

Comment thread lib/dateTime.js Outdated
Comment thread lib/dateTime.js Outdated
'short': convertCldrDatePattern(localeData.dateFormatItems.yMd, true),
'monthYear': convertCldrDatePattern(localeData.dateFormatItems.yMMMM, false),
'monthDay': convertCldrDatePattern(localeData.dateFormatItems.MMMMd, false),
'shortMonthDay': convertCldrDatePattern(localeData.dateFormatItems.MMMd, false),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stuff ends up all being static eh? Wonder if it'd be useful doing these calculations at build-time instead of runtime and storing them in locale-data.js?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look at this, but it's cached after the first run. I do want to avoid storing converted data in locale-data/ so we just have a clean source of truth. A lot of this may go away very soon too with the skeleton work coming up, so we can look at it again then.

Comment thread lib/dateTime.js
@bearfriend bearfriend force-pushed the dgleckler/rewrite-dateTime branch from fc0bc46 to b46a9c9 Compare July 6, 2026 23:15
@bearfriend bearfriend force-pushed the dgleckler/rewrite-dateTime branch 2 times, most recently from 2490e5a to be95d77 Compare July 7, 2026 00:46
@bearfriend bearfriend force-pushed the dgleckler/rewrite-dateTime branch from be95d77 to 22568a3 Compare July 7, 2026 00:50
@bearfriend bearfriend force-pushed the dgleckler/rewrite-dateTime branch from 6b3fda0 to 7a5b68a Compare July 7, 2026 01:28
@bearfriend bearfriend force-pushed the dgleckler/rewrite-dateTime branch from 7a5b68a to 164c714 Compare July 7, 2026 01:32
@bearfriend bearfriend mentioned this pull request Jul 9, 2026
@bearfriend bearfriend marked this pull request as ready for review July 10, 2026 01:04
@bearfriend bearfriend requested a review from a team as a code owner July 10, 2026 01:04
@bearfriend bearfriend changed the base branch from dgleckler/build-lms-files to main July 10, 2026 01:11
@bearfriend bearfriend force-pushed the dgleckler/rewrite-dateTime branch from b38836d to b0ff22e Compare July 10, 2026 01:41
Comment thread test/dateTime.test.js
Comment thread lib/dateTime.js Outdated
Comment thread build-locale-data/locale-data-overrides/ko.js
Comment thread build-locale-data/build-files.js Outdated
Comment thread lib/locale-data/.DS_Store Outdated
Comment thread lib/locale-data/current.js Outdated
Comment thread lib/locale-data/locales/ca.js
@bearfriend bearfriend force-pushed the dgleckler/rewrite-dateTime branch from 50b052a to 946b23b Compare July 13, 2026 05:32
@bearfriend bearfriend force-pushed the dgleckler/rewrite-dateTime branch from 946b23b to d3d4765 Compare July 13, 2026 06:08
const documentLocaleSettings = getDocumentLocaleSettings();

let currentLocale;
let localeDataPromise = Promise.resolve();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A promise that can be overwritten each time the locale changes

Comment on lines +8 to +11
export const localeData = new Proxy(localeDataTarget, {
deleteProperty() { return false; },
defineProperty() { return false; }
});

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we can't Object.freeze the shared object (so we can update its properties), we proxy it and disallow (re)defining and deleting properties outside this module.


Object.defineProperty(localeDataTarget, 'then', { value: (...args) => localeDataPromise.then(...args) });
Object.defineProperty(localeDataTarget, 'catch', { value: (...args) => localeDataPromise.catch(...args) });
Object.defineProperty(localeDataTarget, 'finally', { value: (...args) => localeDataPromise.finally(...args) });

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make localeDataTarget thenable, always redirecting to the dynamic localeDataPromise. We use defineProperty here mostly to prevent enumerability. i.e. before first resolution, Object.keys(localeData) // -> []

This means localeData acts as its own promise, and allows existing and future intl functions like formatDate to remain synchronous, as long as there is a top-level await localeData anywhere before they can be called, which we already do right at the end of this module. Shifting that responsibility to consumers is an option that would prevent blocking while the initial locale data loads, but in practical terms would offer little to no benefit as something else is likely to be imported that already top-level awaits it. After that, any consumer is responsible for managing updates:

// library
import { localeData } from '@brightspace-ui/intl/lib/locale-data/current.js';
import { resolveSupportedLocale } from '@brightspace-ui/intl/lib/locale-data/supported.js';

// current.js already awaited the initial locale's data
export function getMonths(format) {
	const months = localeData.months.standAlone[format];
}

export async function changeLanguage(lang) {
	document.documentElement.lang = resolveSupportedLocale(lang);
	await localeData;
}
// component/mixin
...
import { localeData } from '@brightspace-ui/intl/lib/locale-data/current.js';

class MyComponent ... {
	...
	connectedCallback() {
		this.connectedCallback();
		this.addEventListener('d2l-locale-data-change', () => requestUpdate('localeData'));
	}

	render() {
		return html`
			<ul>${localeData.months.standAlone.wide.map(m => html`
				<li>${m}</li>`)}
			</ul>
		`;
	}
}

Comment on lines +34 to +35
localeDataPromise = setLocaleData();
return localeDataPromise;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the double proxied promise pointer

setLocaleData();
registerLocaleDataListener();
await updateLocaleData();
Object.preventExtensions(localeDataTarget);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proxy already prevents consumers adding new properties, but this ensures we aren't adding extraneous properties when we change locales. We can probably remove this in favor of more tests once generating/modifying the locale data files is more settled down, but for now it's useful to catch inconsistencies there.

Comment thread lib/dateTime.js
delete settings.overrides.date.calendar.months;
delete settings.overrides.date.calendar.days;
}
return merge(JSON.parse(JSON.stringify(descriptor)), settings.overrides.date);

@bearfriend bearfriend Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prevents accidental/attempted overwriting of localeData subproperties since without this they would be direct references. We can't use structuredClone yet. Thanks Safari!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants