Update I18n modules to support loading automatically bundled CLDR data #657

agubler · 2020-01-27T14:37:36Z

Type: feature

The following has been addressed in the PR:

There is a related issue
All code has been formatted with prettier as per the readme code style guidelines
Unit or Functional tests are included in the PR

Description:

Comprehensive change of i18n support in Dojo; The motivation is to enable automatic detection of CLDR data requirements with lazy loading and resolution of i18n CLDR data bundles. Ensuring that CLDR data automatically included in i18n enables Dojo to remove existing custom locale resolution logic and entirely depend on Globalize for locale resolution.

This should be backwards compatible, unless the downstream application is importing interfaces that are no longer needed (and can be removed) or using the core i18n module directly (which shouldn't be required).

Details:

Expose all i18n message bundle and locale behaviour using reactive patterns from the existing i18n mixin and middleware.
Expose an API for registering CLDR loaders, default locale and other supported locales.
Migrate to using Globalize for all message bundle resolution.
Lazily load required CLDR data using loaders registered with the i18n system.
Fallback to using a complete CLDR supplemental bundle when an unknown locale is used.
Basic i18n support for codesandbox using a util cldr module that loads the required supplemental data (This import will automatically be elided during the Dojo build)
Lazily loading CLDR data when updating a widget's locale through either the global application injector or explicitly via the locale property (support in both the middleware and mixin)

Related to #655

codesandbox-ci · 2020-01-27T14:39:11Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 449f5dc:

Sandbox	Source
dank-mountain-ygze5	Configuration

mwistrand · 2020-01-31T16:34:50Z

src/i18n/i18n.ts

+	const localCldrLoader = cldrLoaders[userLocale];
+	if (localCldrLoader && localCldrLoader !== true) {
+		loaderPromises.push(localCldrLoader());
+	}


Can this be changed from localCldrLoader to localeCldrLoader since it's specific to the locale and has nothing to do with the local option?

Yup absolutely

mwistrand · 2020-01-31T16:36:31Z

src/i18n/i18n.ts

+		messages: Object.keys(bundle.messages).reduce(
+			(messages, key) => {
+				const message = globalize.cldr.get(`${MESSAGE_BUNDLE_PATH}/${bundleId}/${key}`);
+				messages[key] = message;
+				return messages;
+			},
+			{} as any
+		),


When called from within middleware/i18n.localize(), will this object be regenerated each time the widget is invalidated? If so, caching the result would be nice.

mwistrand · 2020-01-31T16:58:34Z

tests/core/unit/middleware/i18n.tsx

+			root.innerHTML,
+			'<div><div>{"foo":""}</div><div></div><div>true</div><div>{"locale":"es"}</div><button>es</button></div>'
+		);
+		es.resolver({ default: { foo: 'holla, {name}' } });


Nit: "hello" in Spanish is "hola"

Lol Thanks 😄

mwistrand · 2020-01-31T17:09:09Z

tests/i18n/support/cldr.ts

+	loadData(frCurrencies);
+	loadData(frNumbers);
+	loadData(frUnits);
+});


Just verifying nothing throws?

This is used to load data in for the tests, runs in both browser and node.

agubler · 2020-02-05T12:51:20Z

@mwistrand I think I've addressed your comments

codecov · 2020-02-11T18:03:06Z

Codecov Report

Merging #657 into master will decrease coverage by 0.25%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #657      +/-   ##
==========================================
- Coverage   98.06%   97.80%   -0.26%     
==========================================
  Files         120      117       -3     
  Lines        6654     6570      -84     
  Branches     1510     1494      -16     
==========================================
- Hits         6525     6426      -99     
- Misses        129      144      +15

Impacted Files	Coverage Δ
src/i18n/util/cldr.ts	`100.00% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b61c6ff...449f5dc. Read the comment docs.

…CLDR data (dojo#657)" This reverts commit 6224e88.

agubler requested review from matt-gadd and mwistrand January 27, 2020 14:37

agubler force-pushed the refector-i18n branch 2 times, most recently from 55135af to e3fc460 Compare January 28, 2020 22:35

mwistrand reviewed Jan 31, 2020

View reviewed changes

agubler force-pushed the refector-i18n branch from ed6f373 to f28b9b0 Compare January 31, 2020 18:14

mwistrand approved these changes Feb 11, 2020

View reviewed changes

agubler added 21 commits February 11, 2020 17:47

Adapt i18n to use Globalize/CLDR for message resolution

772123e

Update test name

f24874b

remove debugger

f79f682

unit tests

9b8dca4

add i18n api for loading and setting locale info

8d6df99

fix unit tests

63ab3a7

testing

aa03923

supported locales include default locale

7285fe5

Use diffProperty to influence the locale property

7beda44

small tweaks

359747c

Support proper fallback for locale in mixin

d161012

Support i18nBundle property

fa8dedd

remove unused var

0ef9949

unit tests

f74a546

fix cldr loader

255188b

fallback to the full cldr supplemental data

9ee9611

Use fallback supplemental cldr in error scenarios

fa6f20f

use warn not log

90b70ce

fix tests

1e2eb36

clean up the i18n cldr fallback and resolution

e1f84c9

split out the core i18n functionality for clarity

ec85e66

agubler added 5 commits February 11, 2020 17:47

tidy

d092ffb

address review feedback

a32c5f1

Use property locale

fbc0584

cache bundles

c485111

package-lock

449f5dc

agubler force-pushed the refector-i18n branch from 5ab6911 to 449f5dc Compare February 11, 2020 17:50

agubler merged commit 6224e88 into dojo:master Feb 11, 2020

agubler added a commit to agubler/framework that referenced this pull request Feb 13, 2020

Revert "Update I18n modules to support loading automatically bundled …

c44860d

…CLDR data (dojo#657)" This reverts commit 6224e88.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update I18n modules to support loading automatically bundled CLDR data #657

Update I18n modules to support loading automatically bundled CLDR data #657

Uh oh!

agubler commented Jan 27, 2020

Uh oh!

codesandbox-ci bot commented Jan 27, 2020 •

edited

Loading

Uh oh!

mwistrand Jan 31, 2020

Uh oh!

agubler Jan 31, 2020

Uh oh!

mwistrand Jan 31, 2020

Uh oh!

agubler Feb 5, 2020

Uh oh!

mwistrand Jan 31, 2020

Uh oh!

agubler Jan 31, 2020

Uh oh!

mwistrand Jan 31, 2020

Uh oh!

agubler Jan 31, 2020

Uh oh!

agubler commented Feb 5, 2020

Uh oh!

codecov bot commented Feb 11, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Update I18n modules to support loading automatically bundled CLDR data #657

Update I18n modules to support loading automatically bundled CLDR data #657

Uh oh!

Conversation

agubler commented Jan 27, 2020

Type: feature

Description:

Details:

Uh oh!

codesandbox-ci bot commented Jan 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agubler commented Feb 5, 2020

Uh oh!

codecov bot commented Feb 11, 2020

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codesandbox-ci bot commented Jan 27, 2020 •

edited

Loading