The content of a flattened _ROOT nls bundle is not taken into account

[Patrick Ruzand]

We found something very strange regarding how dojo treat nls modules included in a _Root flattened bundle compare to the case of _<locale> flattened bundles. For example, my dojo build does not have "en" or "en_us" included in the generated locales. So, when the page loads up with "en" or "en_us" locale, dojo will fallback to load the _Root flattened bundle for the nls. Compare to other locale, e.g., fr, which is one of the generated locales, when page loads up with "fr" locale, dojo will load the _fr flattened bundle, and not the individual files. However, whenever dojo falls back to use the _Root nls flattened bundle, it does not seem to recognize the nls modules inside the layer _ROOT, in fact, it tries to load individual nls file again.

From what I see in the dojo/i18n code, the problem seems to be due to the fact that, in the case the current locale is not in the list of the preloaded locales, the i18n plugin will load the _ROOT bundle, and a root bundle contained in a flattened bundle is cached in the loader cache with a mid like "myapp/nls/mybundle/ROOT" (done in the i18n.preload() function). But then, when a require for this bundle is done, (e.g requiredojo/i18n!myapp/nls/mybundle?), the i18n plugin will falls back to its "new 1.7 default" behavior implemented in doLoad(). Which means, it first requires the root bundle before iterating over the possible locales:

doLoad = function(require, bundlePathAndName, bundlePath, bundleName, locale, load){
  require([bundlePathAndName], function(root){
    var current = lang.clone(root.root),
	availableLocales = getAvailableLocales(!root._v1x && root, locale, bundlePath, bundleName);
    require(availableLocales, function(){
[...]
}

The problem here is that the root bundle is loaded using its "original" mid (aka "myapp/nls/mybundle") instead of the one used during the preload phase (aka "myapp/nls/bundle/ROOT").

To summarize, it seems to me there's an inconsistency in the ways a root bundle is identified when it is loaded via a preloaded flattened bundle (cached using the "myapp/nls/bundle/ROOT" id) and when it is loaded during the fallback path in doLoad().

[Patrick Ruzand]

Changed Component to "Loader" as it seems it's more a i18n/loader/build issue, so I guess should be in the rawld's hands. (Feel free to change if I am wrong).

[Patrick Ruzand]

I raised the priority as I think it's really a critical issue: it forces the deployment of the individual root bundles while the goal of the flattened bundle was to avoid this.

[cjolif]

FYI this is hurting one of our product badly.

[Colin Snover]

1.9.1 is released; moving all outstanding tickets to next milestone.

[cjolif]

This was reported one more time to me today. Any chance to get it fixed?

[Patrick Ruzand]

testcase. the dojo src is assumed to be in scripts/dtk.

[Patrick Ruzand]

The above testcase reproduces the issue. The build profile is located in <install-dir>/scripts. The dojo src is assumed to be in <install-dit>/scripts/dtk. The app html is <install-dir>/demo-release17-build.html, and it displays a message in the console. The build is run using the following command line:

node ..\..\dojo\dojo.js load=build --profile ..\..\..\demo.profile.js

The <install-dir>/demo-release17-build.html forces the locale to "aa", that is, a locale for which there's no flattened bundle, and therefore, the fallback mechanism occurs and the flattened ROOT bundle is loaded. If you look at the Network traffic in the debugger, one can notice the following files are loaded: /dojo/testbuild/demo-release17-build.html /dojo/testbuild/release/dojo/dojo.js.uncompressed.js /dojo/testbuild/release/dojo/nls/dojo_ROOT.js /dojo/testbuild/release/myapp/src.js /dojo/testbuild/release/myapp/nls/src_ROOT.js /dojo/testbuild/release/myapp/nls/greetings.js /dojo/testbuild/release/dojo/cldr/nls/number.js /dojo/testbuild/release/dojo/cldr/nls/currency.js

which means the individuals nls modules (myapp/nls/greetings, dojo/cldr/nls/number and dojo/cldr/nls/currency) are loaded individually while they are already included in the src_ROOT.js flattened bundled.

If you force the locale to "en" (for which there's a flattened bundle), then it works as expected.

[Clement Mathieu]

After some investigations, I think this bug is coming from this part of the i18n plugin (around line 275) :

	targetLocale =	localeSpecified || dojo.locale || "",
	loadList = localeSpecified ? [targetLocale] : getLocalesToLoad(targetLocale),
	
	[...]
	
	array.forEach(loadList, function(locale){
		var target = bundlePathAndName + "/" + locale;
		if(!cache[target]){
			doLoad(require, bundlePathAndName, bundlePath, bundleName, locale, finish);
		}
	}

During this step, the code is testing if the bundle corresponding to the targeted locale is in the i18n cache and if not, it loads the bundle via a require() (the doLoad method), The problem is, when flattened bundles are in use, that the target locale may not be a generated locale. So, in such case (flattened bundle + locale that is not in the localeList ), when the plugin is looking for the target locale bundle in its cache, it does not find it (logical, since the locale does not match a preloaded flattened bundle), and ends up requiring the module using the new 1.7 amd way (loading the individuals nls modules). To fix that bug, it should test the cache with the real used locale (that is, if flattened bundles are in use, the generated locale that best matches the target locale, or "ROOT" if none) instead of the targeted locale.

I have attached a "patch" to demonstrate where the problem appears and a possible approach to fix it following the methodology described above.

[Rawld Gill]

I generally agree with cmathieu's analysis. Let me say what's happening is slightly different words:

1. When preloads are present, loading a layer will cause a preload of the best bundle given the current locale.

2. If the current locale is is not mentioned in the preloads, then the best is "ROOT" and that flattened bundle is loaded; a "flattened root" bundle is not equivalent to a typical root bundle since a flattened root bundle does not include the hash of available locales.

3. When a bundle is then required by a normal module, the i18n machinery will fail to find bundle and proceed with the normal i18n loading sequence...which causes the normal (i.e., not flattened) root bundle to be loaded, and then the locale-specific bundle of available.

I think this is not fixable solely by patching the i18n module. Consider the case where, in the example provided by pruzand, the locale aa does exist, but was not flattened. Then, ideally, the root should be loaded by the preload algorithm and followed by the aa bundle using the normal algorithm.

I believe the fix should be two-fold:

1. Include the available locales in the flattened root bundle (change to builder).
2. Cache the preloaded root (change to i18n).

I think these are both micro changes. I'll proposed a patch today.

[Clement Mathieu]

I have made two pull request on GitHub? with a patch to fix that bug. Following rcgill proposition, the patch is two-fold so there is two pull request.

They can be reviewed here:

• change to builder ​https://github.com/dojo/util/pull/9
• change to i18n plugin ​https://github.com/dojo/dojo/pull/34

[Rawld Gill]

I think the proposed patch above is not the best solution for the following reasons:

1. The line

​https://github.com/clmath/util/blob/bbba18cf8d6e009e9be74232aa515cc1acc2362b/build/transforms/writeAmd.js#L212

causes the preload command to instruct the i18n module that *every* bundle has *every* locale mentioned in *any* bundle. This may not be the case.

2. There is some expressive inefficiency. For example, the function forEachLocale (​https://github.com/clmath/dojo/blob/98ffecc49ce110c8f802ae0a13f27a9c13cd75eb/i18n.js#L40) is already mentioned in the i18hn module...why write it twice?

I still believe my proposed solution is the way forward (see attached files). However, cmathieu noted that if a locale is requested (e.g., en-us) for which there is a less-specific flattened version, then the flattened resources are ignored.

I'm not sure that is really worth worrying about. Considering the example above...if en-us is so important that the normal load sequence is offensive, then simply add that locale to the locale list during the build.

The alternative implies yet more complexity. Continuing the example with en-us, we must notate the flatted en-bundle to say either that a specialized en-us is available (or not), or when trying to preload en-us and noticing that only a less specific bundle is available, causing the root to also be preloaded. This complexity for very little if any performance gain seems foolish.

Interested folks should provide a counter argument.

I'm attaching my proposed change.

[Rawld Gill]

include all available localizations in a flattened root bundle

[Rawld Gill]

cause flattened root bundle to be understood loaded by the loader

[Clement Mathieu]

Thank you rcgill for your feedback on my proposition, and I agree this is not the best solution.

---

However, I am still convinced that the case where there is a less-specific flattened bundle for the locale requested (e.g., flatten exist for en when en-us is requested) worth being solved.

Here are my counter arguments:

• Adding the locale to the localeList cannot be a good solution as an unflattened locale will be requested at some points.
• The performance gain is huge if the user is on a high-latency connection. Currently, when accessing an application with only 10 nls bundles (which is a very reasonable number), for locale en-us and with en and ROOT flatten bundle available, dojo will download :
  • the en flatten bundle (1 http request)
  • all the individual root bundles (10 http requests)
  • all available individual en bundles (around 10 http requests)
  • all available individual en-us bundles (I don't count those ones as they have to be downloaded anyway)

So there is 19 unnecessary http requests and that is if we only have 10 nls modules. With an average latency of 400ms on a 3G network, doing those requests from a smartphone has a real impact on performance.

---

Considering this, I have started with your patch, and added support for that functionality. As you suggested in your comment, I changed the preloadL10n function to preload the root flatten bundle if there was no flatten bundle for the requested locale. I have also changed the getAvailableLocales function so it stops to enumerate bundles when one is in cache.

For example, assuming:

• the root bundle indicates specific bundles for "fr" and "fr-ca",
• bundlePath is "myPackage/nls"
• bundleName is "myBundle"
• myPackage/nls/myBundle/fr is already in cache

Then a locale argument of "fr-ca" would return ["myPackage/nls/fr/myBundle", "myPackage/nls/fr-ca/myBundle"]

This way the i18n plugin is not doing unnecessary requests and the example from my counter argument above will do only 2 http requests (one for root flatten bundle and one for en flatten bundle). It also saves a few computations as the bundle from cache has already been mixed-in with the root bundle.

---

I would greatly appreciate if you had time to look at my proposition and give me your feedback.

You can review my modifications on github or in the attached file. ​https://github.com/clmath/dojo/compare/17155-rawld...17155-me

[Clement Mathieu]

i18n with rawld patch and less specifique fallback.

[Clement Mathieu]

Since there was a lot of discussion on this bug, I will try to summarize it before presenting my proposed fix.

Setup

Let's assume the following setup, with the user locale set to fr-fr and the list of flattened locales being [fr, root]:

myapp/
  |_nls/
    |_fr-fr/
      |_greetings.js
    |_fr/
      |_greetings.js
    |_greetings.js
    |_src_fr.js
    |_src_ROOT.js
  |_src.js

myapp/src.js is the main app layer and myapp/nls/src_fr.js is the corresponding flattened nls layer for the locale fr.

The bug

The bug was first reported appearing when a user requested a non existing locale like foo. Since there is no flattened nls layer for foo, the i18n preload system loads the flattened nls layer with the closest match to the required locale (myapp/nls/src_ROOT.js in that case). Then, the app will require the greetings bundle for the locale foo. Since the bundle is not in cache, the plugin switches to "unbuilt" mode, hence requiring the individual root bundle, even though the ROOT nls layer has already been preloaded. Further investigation has shown that the issue also appears when a more specific locale is required (e.g. fr-fr) and only a less specific locale has been flattened (e.g. fr). In this case the individual fr bundles are loaded, even though the fr nls layer has been preloaded.

rcgill proposal

rcgill submitted a fix solving the problem for the ROOT fallback issue only.

When a user is requiring a non existing locale (e.g. aa), the i18n preload system will load the root flattened nls layer. When a bundle will be required by the application, the i18n plugin will look for the bundle with locale aa in its cache and will not found it (since only the root layer was preloaded). So the system also switches to "unbuilt" mode and requires the individual root bundle.

To fix this, rcgill proposal is to add the list of available locales for each bundle in the root flattened layer. This way the loading of the individual root bundle can be avoided.

nfortunately this fix does not fix the more general case of the fallback to less-specific locale.

My proposal

Since rcgill fix is not completely solving the issue, I enhanced it to account for the general case.

I have made two changes on top of rcgill patch:

1. If the flattened nls layer is not a *perfect* match (e.g. there is a flattened nls layer for fr but not for fr-fr), the preload system will also preload the ROOT layer as it will be useful later to determine for which locale there's a specific bundle.

2. When requiring a bundle, the i18n plugin will check if the module is in cache not only for the required locale (e.g. fr-fr) but also for all the fallback locales (e.g. fr and ROOT). This way, the system avoids to download the individual fr bundle that was preloaded in the fr nls layer.

[Rawld Gill]

annotate flattened bundles to say what other more-specific locales are available

[Rawld Gill]

improve preloading to ensure the best locale is loaded for each bundle even if it wasn't flattened

[Rawld Gill]

I think there is a better way that requires fewer changes (in the attached proposal, notice all changes are localized to the preload loop).

During build time, annotate each flattened bundle with the list of more-specific locales that are available that were not flattened. For example, if the localizations for "fr", "fr-fr", and "fr-ca" all exist, but only "fr" and "fr-fr" were flattened, then notate in the "fr" bundle that "fr-ca" is available.

Now at runtime, assume there is a demand to load this bundle (that is, this bundle is part of a flattened bundle) with the run-time locale==="fr-ca". Then the preload algorithm will fall back and load "fr". Upon doing so, it will see "fr-ca" is available and since that is the desired locale, load "fr-ca" and mix it into the already-loaded "fr" locale.

This is automatic for every bundle mentioned in a flattened bundle (remember, "flattening" does two things: (1) aggregates specific bundles at build-time (for example, the "fr-fr" bundle will be root + "fr" + "fr-fr", and (2) aggregates all of the bundles required for a particular layer in a single module). So, when we make the decision to preload, we're also making the decision to automatically load the best available bundle given the run-time locale for every bundle mentioned in the rollup.

See attached proposed code above. It has not been tested (or even run) yet. Interested folks are encouraged to give it a try

[Rawld Gill]

Proposal debugged and tested. Test attached below. See pull requests: ​https://github.com/dojo/dojo/pull/41 and ​https://github.com/dojo/util/pull/13

[Rawld Gill]

extensive test

[Patrick Ruzand <[email protected]…>]

In 341e41f1797739449d883e8951cda54bde53417e/dojo:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

[Patrick Ruzand <[email protected]…>]

In 33a471c3cbd9dbe878d257a9bdb8b507721ce52c/dojo:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

[Patrick Ruzand <[email protected]…>]

In d76bd85648251308483b9148b6389cb363e988f9/dojo:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

[Clement Mathieu]

This fix introduces a regression. It prevents the loading of 1.6 i18n layers.

At some point in the new code there is a null pointer exception when preloading 1.6 i18n layers. I proposed an easy fix in ​PR#56 on github.

To reproduce the bug, download the test case and navigate to ticket17155/test17155.html.

[Clement Mathieu]

Test case for the regression introduced by fix for #17155

[cjolif]

regression fixed in all targeted versions. See: ​https://github.com/dojo/dojo/commit/b2fa879954860e6669258eee2a25c5e4432b86d3 for master.

[jandockx]

Something still seems amiss with 1.9.3.

We just spend a day trying to understand a weird issue, introduced by 1.9.3. We do not understand as of yet exactly what happens, nor the issue discussed above.

What we see now is:

Without a localeList in the build profile, for a layer xxx/main.js, we get xxx/nls/main_xx.js files for a number of languages, a/o en-us, and fr-fr, but not en nor fr. We presume this is because localeList gets a default value that contains these languages.

When we add en and fr to dojoConfig.extraLocale, things go wrong when using i18n.getLocalization. The bundle we get for "en", or "fr", e.g., is the root bundle, and it contains no entries for "en" or "fr", resp..

We assume this has to do with preloading: no main_en or main_fr exists, and when we ask for en or fr, we get what was preloaded, which is wrong.

When we remove dojoConfig.extraLocale, things work. When we add localeList with en and fr to the build profile, things work.

In conclusion, we think these 2 need to be in sync since 1.9.3. Actually, this might be kind of a bug. If en or fr is not built, but en-us and fr-fr are, the loader should not assume that there is no "en" or "fr".

All in all, the default for localeList would better be [], to not get unexpected behaviour.

Note that our understanding of what is happening might not be correct.

[Clement Mathieu]

Hi jandockx,

The default localeList is the same since 1.8 so it's not new in 1.9.3. From which version are you migrating to 1.9.3 ?

Also do you have a test-case that would demonstrate the problem ?

Thanks, Clément

[jandockx]

We migrated from 1.9.2. No test-case available at the moment. Sorry.

Replying to cmathieu:

Hi jandockx,

The default localeList is the same since 1.8 so it's not new in 1.9.3. From which version are you migrating to 1.9.3 ?

Also do you have a test-case that would demonstrate the problem ?

Thanks, Clément

[Clement Mathieu]

Hi Jan,

I have been able to track down the bug. I will post a test case tomorrow. Anyway, the bug is pretty subtle so it will take some time to fix it.

Regards, Clément

[Adrian Vasiliu]

Reopened on cmathieu's behalf.

[Clement Mathieu]

Test case for the second regression.

[Clement Mathieu]

I have made a test case to demonstrate the second regression.

To run, copy dojo and util directory as siblings of the app directory. Then build the app using app/dojo.profile.js and navigate to app/demo.html and open the console to see that the root bundle for number is used instead of the french one.

[Wulf]

The issue disappeared, when I replaced the compiled Version of the ./nls/-path with the original one. The dojo-compiler changes messages.js and adds a parameter at the beginning of the define()-statement:

>>built define("module/nls/messages",{root:{...},de:!0}); # sourceMappingURL=messages.js.map

When I simply delete this parameter like

>>built define({root:{...},de:!0}); # sourceMappingURL=messages.js.map

everything works fine again.

Tested under Win 7 and dojo 1.9.3

I hope this helps to find a solution.

[bill]

As per meeting.

[dylan]

Unfortunately due to a lack of recent activity, we're going to have to punt this one until after 1.11.

[dylan]

Ticket planning... move current 1.13 tickets out to 1.15 to make it easier to move tickets into the 1.13 milestone.