i18n - support global locale data #33523
Closed
Conversation
This file contains 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
petebacondarwin
force-pushed
the
locale-global-umd-2
branch
from
219e4cb
to
f72ecad
Compare
alfaproject
reviewed
Nov 1, 2019
| root.ng = root.ng || {}; | ||
| root.ng.common = root.ng.common || {}; | ||
| root.ng.common.locale = root.ng.common.locale || {}; | ||
| const u = undefined; |
There was a problem hiding this comment.
I'm fairly certain that the compression is better if you don't capture undefined
There was a problem hiding this comment.
So initially it was because we removed the undefined values in the arrays (because they are not necessary): [1, , ,3] is valid, and the files were smaller
But then some people complained that it was causing errors in some webpack plugins, so we were forced to re-add the undefined values
We did some comparisons and since all the files are independent (they are not always bundled, some people lazy load them), it was smaller to just define a constant equal to undefined.
Since then the compilers might have gotten smarter, maybe it isn't necessary anymore, you can try and benchmark that :)
There was a problem hiding this comment.
Thanks @ocombe for the explanation. Data is king! We should indeed measure it.
As it turns out this PR is generating JS files that will not be further processed, so I am thinking that the generation of those files should go through terser to get them as compact as possible.
There was a problem hiding this comment.
👍 for just inlining undefined and not trying to be compressor.
👍 for going through terser
There was a problem hiding this comment.
I tried to update BUILD.bazel to pass these generated files through terser but it turned out to be a lot more complicated that expected. I suggest we come back to that optimisation task after this PR has landed.
petebacondarwin
force-pushed
the
locale-global-umd-2
branch
from
f72ecad
to
bdddf78
Compare
petebacondarwin
added
area: build & ci
|
Yes, but it's also nice to be able to open the files and see the content,
which is not possible if it's all minified...
|
|
@ocombe these files are pretty obscure even when they are not minified :-) |
petebacondarwin
force-pushed
the
locale-global-umd-2
branch
2 times, most recently
from
c13b677
to
321b6a8
Compare
petebacondarwin
added
the
action: review
|
This appears to address all the concerns I had. I’ll give it a more thorough review today but nice job putting this together. |
petebacondarwin
commented
Nov 1, 2019
mhevery
suggested changes
Nov 1, 2019
| `${I18N_CORE_FOLDER}/locale_en.ts` | ||
| ], | ||
| {base: '.'}) | ||
| .pipe(format.format('file', clangFormat)) |
There was a problem hiding this comment.
I see that you are doing clang-format here, but I don't see the output file actually being formatted. Something is going on here.
There was a problem hiding this comment.
Originally I had not planned to format the generated files.
Since we have agreed to pass these generated files through terser before adding to the npm package, I changed the PR to actually format the new files with clang after generation.
IgorMinar
reviewed
Nov 1, 2019
clydin
reviewed
Nov 1, 2019
petebacondarwin
added
action: merge
atscott
pushed a commit
that referenced
this pull request
Nov 5, 2019
In order to support adding locales during compile-time
inlining of translations (i.e. after the TS build has completed),
we need to be able to attach the locale to the global scope.
This commit modifies CLDR extraction to emit additional "global"
locale files that appear in the `@angular/common/locales/global` folder.
These files are of the form:
```
(function() {
const root = typeof globalThis !== 'undefined' && globalThis ||
typeof global !== 'undefined' && global || typeof window !== 'undefined' && window;
root.ng = root.ng || {};
root.ng.common = root.ng.common || {};
root.ng.common.locale = root.ng.common.locale || {};
const u = undefined;
function plural(n) {
if (n === 1) return 1;
return 5;
}
root.ng.common.locale['xx-yy'] = [...];
})();
```
The IIFE will ensure that `ng.common.locale` exists and attach the
given locale (and its "extras") to it using it "normalized" locale
name.
* "extras": in the UMD module locale files the "extra" locale data,
currently the day period rules, and extended day period data, are
stored in separate files under the "common/locales/extra" folder.
* "normalized": Angular references locales using a normalized form,
which is lower case with `_` replaced by `-`. For example:
`en_UK` => `en-uk`.
PR Close #33523
atscott
pushed a commit
that referenced
this pull request
Nov 5, 2019
This commit contains the global locale files generated by the change to the CLDR `extract.js` tool, from the previous commit. PR Close #33523
atscott
pushed a commit
that referenced
this pull request
Nov 5, 2019
atscott
pushed a commit
that referenced
this pull request
Nov 5, 2019
To limit the exposure of the private `LOCALE_DATA` from outside `@angular/core` this commit exposes private functions in the core to hide the internal structures better. * The `registerLocaleData()` implementation has moved from `@angular/common` to `@angular/core`. A stub that delegates to core has been left in common for backward compatibility. * A new `ɵunregisterLocaleData()` function has been provided, which is particularly useful in tests to clear out registered locales to prevent subsequent tests from being affected. * A private export of `ɵregisterLocaleData()` has been removed from `@angular/common`. This was not being used and is accessible via `@angular/core` anyway. PR Close #33523
atscott
pushed a commit
that referenced
this pull request
Nov 5, 2019
To support compile time localization, we need to be able to provide the locales via a well known global property. This commit changes `getLocaleData()` so that it will attempt to read the local from the global `ng.common.locales` if the locale has not already been registered via `registerLocaleData()`. PR Close #33523
atscott
pushed a commit
that referenced
this pull request
Nov 5, 2019
atscott
pushed a commit
that referenced
this pull request
Nov 5, 2019
In order to support adding locales during compile-time
inlining of translations (i.e. after the TS build has completed),
we need to be able to attach the locale to the global scope.
This commit modifies CLDR extraction to emit additional "global"
locale files that appear in the `@angular/common/locales/global` folder.
These files are of the form:
```
(function() {
const root = typeof globalThis !== 'undefined' && globalThis ||
typeof global !== 'undefined' && global || typeof window !== 'undefined' && window;
root.ng = root.ng || {};
root.ng.common = root.ng.common || {};
root.ng.common.locale = root.ng.common.locale || {};
const u = undefined;
function plural(n) {
if (n === 1) return 1;
return 5;
}
root.ng.common.locale['xx-yy'] = [...];
})();
```
The IIFE will ensure that `ng.common.locale` exists and attach the
given locale (and its "extras") to it using it "normalized" locale
name.
* "extras": in the UMD module locale files the "extra" locale data,
currently the day period rules, and extended day period data, are
stored in separate files under the "common/locales/extra" folder.
* "normalized": Angular references locales using a normalized form,
which is lower case with `_` replaced by `-`. For example:
`en_UK` => `en-uk`.
PR Close #33523
atscott
pushed a commit
that referenced
this pull request
Nov 5, 2019
This commit contains the global locale files generated by the change to the CLDR `extract.js` tool, from the previous commit. PR Close #33523
atscott
pushed a commit
that referenced
this pull request
Nov 5, 2019
atscott
pushed a commit
that referenced
this pull request
Nov 5, 2019
To limit the exposure of the private `LOCALE_DATA` from outside `@angular/core` this commit exposes private functions in the core to hide the internal structures better. * The `registerLocaleData()` implementation has moved from `@angular/common` to `@angular/core`. A stub that delegates to core has been left in common for backward compatibility. * A new `ɵunregisterLocaleData()` function has been provided, which is particularly useful in tests to clear out registered locales to prevent subsequent tests from being affected. * A private export of `ɵregisterLocaleData()` has been removed from `@angular/common`. This was not being used and is accessible via `@angular/core` anyway. PR Close #33523
atscott
pushed a commit
that referenced
this pull request
Nov 5, 2019
To support compile time localization, we need to be able to provide the locales via a well known global property. This commit changes `getLocaleData()` so that it will attempt to read the local from the global `ng.common.locales` if the locale has not already been registered via `registerLocaleData()`. PR Close #33523
mohaxspb
pushed a commit
to mohaxspb/angular
that referenced
this pull request
Nov 7, 2019
mohaxspb
pushed a commit
to mohaxspb/angular
that referenced
this pull request
Nov 7, 2019
…#33523) In order to support adding locales during compile-time inlining of translations (i.e. after the TS build has completed), we need to be able to attach the locale to the global scope. This commit modifies CLDR extraction to emit additional "global" locale files that appear in the `@angular/common/locales/global` folder. These files are of the form: ``` (function() { const root = typeof globalThis !== 'undefined' && globalThis || typeof global !== 'undefined' && global || typeof window !== 'undefined' && window; root.ng = root.ng || {}; root.ng.common = root.ng.common || {}; root.ng.common.locale = root.ng.common.locale || {}; const u = undefined; function plural(n) { if (n === 1) return 1; return 5; } root.ng.common.locale['xx-yy'] = [...]; })(); ``` The IIFE will ensure that `ng.common.locale` exists and attach the given locale (and its "extras") to it using it "normalized" locale name. * "extras": in the UMD module locale files the "extra" locale data, currently the day period rules, and extended day period data, are stored in separate files under the "common/locales/extra" folder. * "normalized": Angular references locales using a normalized form, which is lower case with `_` replaced by `-`. For example: `en_UK` => `en-uk`. PR Close angular#33523
mohaxspb
pushed a commit
to mohaxspb/angular
that referenced
this pull request
Nov 7, 2019
This commit contains the global locale files generated by the change to the CLDR `extract.js` tool, from the previous commit. PR Close angular#33523
mohaxspb
pushed a commit
to mohaxspb/angular
that referenced
this pull request
Nov 7, 2019
mohaxspb
pushed a commit
to mohaxspb/angular
that referenced
this pull request
Nov 7, 2019
…lar#33523) To limit the exposure of the private `LOCALE_DATA` from outside `@angular/core` this commit exposes private functions in the core to hide the internal structures better. * The `registerLocaleData()` implementation has moved from `@angular/common` to `@angular/core`. A stub that delegates to core has been left in common for backward compatibility. * A new `ɵunregisterLocaleData()` function has been provided, which is particularly useful in tests to clear out registered locales to prevent subsequent tests from being affected. * A private export of `ɵregisterLocaleData()` has been removed from `@angular/common`. This was not being used and is accessible via `@angular/core` anyway. PR Close angular#33523
mohaxspb
pushed a commit
to mohaxspb/angular
that referenced
this pull request
Nov 7, 2019
To support compile time localization, we need to be able to provide the locales via a well known global property. This commit changes `getLocaleData()` so that it will attempt to read the local from the global `ng.common.locales` if the locale has not already been registered via `registerLocaleData()`. PR Close angular#33523
mohaxspb
pushed a commit
to mohaxspb/angular
that referenced
this pull request
Nov 7, 2019
mohaxspb
pushed a commit
to mohaxspb/angular
that referenced
this pull request
Nov 7, 2019
…#33523) In order to support adding locales during compile-time inlining of translations (i.e. after the TS build has completed), we need to be able to attach the locale to the global scope. This commit modifies CLDR extraction to emit additional "global" locale files that appear in the `@angular/common/locales/global` folder. These files are of the form: ``` (function() { const root = typeof globalThis !== 'undefined' && globalThis || typeof global !== 'undefined' && global || typeof window !== 'undefined' && window; root.ng = root.ng || {}; root.ng.common = root.ng.common || {}; root.ng.common.locale = root.ng.common.locale || {}; const u = undefined; function plural(n) { if (n === 1) return 1; return 5; } root.ng.common.locale['xx-yy'] = [...]; })(); ``` The IIFE will ensure that `ng.common.locale` exists and attach the given locale (and its "extras") to it using it "normalized" locale name. * "extras": in the UMD module locale files the "extra" locale data, currently the day period rules, and extended day period data, are stored in separate files under the "common/locales/extra" folder. * "normalized": Angular references locales using a normalized form, which is lower case with `_` replaced by `-`. For example: `en_UK` => `en-uk`. PR Close angular#33523
mohaxspb
pushed a commit
to mohaxspb/angular
that referenced
this pull request
Nov 7, 2019
This commit contains the global locale files generated by the change to the CLDR `extract.js` tool, from the previous commit. PR Close angular#33523
mohaxspb
pushed a commit
to mohaxspb/angular
that referenced
this pull request
Nov 7, 2019
mohaxspb
pushed a commit
to mohaxspb/angular
that referenced
this pull request
Nov 7, 2019
…lar#33523) To limit the exposure of the private `LOCALE_DATA` from outside `@angular/core` this commit exposes private functions in the core to hide the internal structures better. * The `registerLocaleData()` implementation has moved from `@angular/common` to `@angular/core`. A stub that delegates to core has been left in common for backward compatibility. * A new `ɵunregisterLocaleData()` function has been provided, which is particularly useful in tests to clear out registered locales to prevent subsequent tests from being affected. * A private export of `ɵregisterLocaleData()` has been removed from `@angular/common`. This was not being used and is accessible via `@angular/core` anyway. PR Close angular#33523
mohaxspb
pushed a commit
to mohaxspb/angular
that referenced
this pull request
Nov 7, 2019
To support compile time localization, we need to be able to provide the locales via a well known global property. This commit changes `getLocaleData()` so that it will attempt to read the local from the global `ng.common.locales` if the locale has not already been registered via `registerLocaleData()`. PR Close angular#33523
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This is my preferred solution to generating (and using) locales via the global object.
See https://hackmd.io/BFc4P-JYT7-VO0id7-afhg
It is best to review commit by commit, since one simply adds over 500 newly generated locale files.
The idea is that at translation time (after the build) the required locale data is included in what will be executed by the browser.
main.jsbundle at translation time (after the main build).<script src="locale.js">tag in theindex.htmlbefore themain.jsscript tagExecuting this script will attach the locale data to the global scope as
ng.common.locale['xx-xx']making it available to the application.At runtime, when Angular requires locale data, the
findLocaleData()function is called. This function returns locales that have been explicitly registered, the same as before v9, by a call toregisterLocaleData()but now it will also fallback to looking for the locale data on the global scope,ng.common.locale[..].The previous PR #33504 simply updated the current locale files, which are UMD, to support being attached to global scope. This is not optimal for a number of reasons:
main.js) are increased significantly.This PR takes a different approach. The locale data files are generated from CLDR data on a periodic basis. Therefore it is straightforward to generate additional locale files that are optimised for the global attachment use case that is required here. These new files have the following properties:
findLocaleData()requires.findLocaleData()function.