Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#14169 closed defect (fixed)

[regression] build does not create layer-specific nls bundles

Reported by: Kenneth G. Franqueiro Owned by: Rawld Gill
Priority: blocker Milestone: 1.8
Component: BuildSystem Version: 1.7.0
Keywords: Cc: cjolif, Patrick Ruzand
Blocked By: Blocking:

Description

In Dojo 1.6 and earlier, any layer whose dependencies pull in i18n bundles would lead to the generation of consolidated bundles under the nls folder, containing all bundles needed by modules combined into the layer.

For example, building with the following profile:

dependencies = { 
  layers: [
    {   
      name: "../dijit/test.js",
      dependencies: [
        "dijit.layout.ContentPane", // requires loading bundle
        "dijit.form.ValidationTextBox" // requires form/validate bundle
      ]   
    }   
  ],  
  prefixes: [
    [ "dijit", "../dijit" ],
    [ "dojox", "../dojox" ]
  ]
}

In 1.6, this would produce dijit/nls/test_<lang>.js files. In 1.7, no such files are generated. However, testing with the following simple page:

<!DOCTYPE html>
<html>
  <body>
    <script src="../dojotrunk/release/test/dojo/dojo.js"></script>
    <script src="../dojotrunk/release/test/dijit/test.js"></script>
    <script>
require(["dojo/i18n!dijit/nls/loading", "dojo/i18n!dijit/form/nls/validate"],
function(loading, validate){
  console.log(loading, validate);
});
    </script>
  </body>
</html>

...it would appear that the nls bundles are baked in for the en-us locale, but not for any others - e.g. if I add data-dojo-config="locale:'es-es'", the two individual bundles get pulled in, whereas in 1.6 the layer would pull in dijit/nls/test_es-es.js.

(The same thing seems to happen regardless of whether I script src the layer into the page, or require it.)

I'm certainly not suggesting that we bake all locales into the layer itself - that would be unfair to everyone, since they'd be clogging their tubes with any number of locales they'll never be interested in. But having the original behavior of consolidating nls into bundles was certainly a plus; however, I'm not sure whether it's still intended to work that way. Is this a builder regression, or a doc issue for a scenario necessitating migration for Dojo <=1.6 users?

Change History (32)

comment:1 Changed 8 years ago by vvoovv

I suggest that both options should be supported:

1) consolidated nls bundles are generated

2) the bundles for given languages are include to the build

comment:2 in reply to:  1 ; Changed 8 years ago by Kenneth G. Franqueiro

Replying to vvoovv:

I suggest that both options should be supported:

1) consolidated nls bundles are generated

2) the bundles for given languages are include to the build

Just so we don't get confused: number 2 here would be a separate enhancement request. From the viewpoint of the regression, number 1 is the concern.

(I'd also debate that number 2 would be an anti-feature - assuming you build more than one locale, why do you want to punish all of your users by making them download information they will never need?)

comment:3 in reply to:  2 Changed 8 years ago by vvoovv

Replying to kgf:

There is a trade off between the build size and related memory consumption vs. additional network requests. Let's imagine we need to support only 2 languages and the number of strings in all nls bundles is small (3 strings in the case of form/validate bundle). Then the number 2 from my first comment makes sense. However I'm not sure if this feature deserves attention of builder/loader maintainers, so I won't submit a separate ticket for it. I can simply list all required bundles (like any other module) in the dependencies array of a build file.

comment:4 Changed 8 years ago by Kenneth G. Franqueiro

Priority: normalhigh

comment:5 Changed 8 years ago by Rawld Gill

Milestone: 1.7.11.8
Priority: highnormal

comment:6 Changed 8 years ago by Rawld Gill

Status: newassigned

comment:7 Changed 7 years ago by cjolif

Cc: cjolif added

comment:8 Changed 7 years ago by cjolif

Shouldn't it be a 1.7.x thing? It looks like a regression?

comment:9 Changed 7 years ago by Colin Snover

Milestone: 1.81.7.2
Priority: highblocker

Setting 1.7+blocker as per ML discussion.

comment:10 Changed 7 years ago by Rawld Gill

In [27801]:

fixed builder to build flattened layer i18n bundles as available in 1.6-; refs #14092; refs #14169; !strict

comment:11 Changed 7 years ago by Rawld Gill

In [27801] I added the capability to automatically build flattened i18n bundles for layer modules as per v1.6-.

The v1.6 function dojo._preloadLocalizations was added back in [27797]. These two fixes allow flattened layers to be *optionally* used as per 1.6-.

The build program requires one additional enhancement: automatically inserting the call to dojo._preloadLocalizations as was the case in v1.6-.

Notice that this enhancement will not improve performance in async mode under normal circumstances and would require yet another loader plugin to work correctly. Currently, when a built 1.7 layer is loaded, it will include the root bundle, which instructs the runtime of the available localized bundles. Any required localized bundles can then be requested *concurrently* so long as the loader is in async mode.

In sync mode, requiring the flattened bundle would improve performance; because it is only useful for sync mode, automatically including it must be triggered by a build switch.

comment:12 Changed 7 years ago by Rawld Gill

In [27802]:

backport [27801]; refs #14169, refs #14092; !strict

comment:12 Changed 7 years ago by Rawld Gill

In [27806]:

fixed layers to write call to preloadLocalizations as per 1.6-; refs #14092, refs #14169; !strict

comment:13 Changed 7 years ago by Rawld Gill

Resolution: fixed
Status: assignedclosed

In [27807]:

fixed layers to write call to preloadLocalizations as per 1.6-; fixes #14092, fixes #14169; !strict

comment:14 Changed 7 years ago by Rawld Gill

See also #14092

comment:15 Changed 7 years ago by Adam Peller

I see the nls layer files getting built now... but running themeTester.html, I don't see dijit-all_en.js getting loaded, just individual nls files like gregorian.js

comment:16 Changed 7 years ago by Adam Peller

Milestone: 1.7.2
Resolution: fixed
Status: closedreopened

comment:17 Changed 7 years ago by Adam Peller

Milestone: 1.7.2

comment:18 Changed 7 years ago by Rawld Gill

In [27860]:

fixed nls bundle writing out zero-length file; refs #14169; !strict

comment:19 Changed 7 years ago by Rawld Gill

In [27861]:

fixed nls bundle writing out zero-length file; refs #14169; !strict

comment:20 Changed 7 years ago by Rawld Gill

In [27868]:

fixed i18n trying to preload nonexisting i18n bundle from 1.6 build output; refs #14169; !strict

comment:21 Changed 7 years ago by Rawld Gill

In [27869]:

checked in wrong file in [27868], try again, fixed i18n trying to preload nonexisting i18n bundle from 1.6 build output; refs #14169; !strict

comment:22 Changed 7 years ago by Rawld Gill

In [27871]:

fixed i18n trying to preload nonexisting i18n bundle from 1.6 build output; refs #14169; !strict

comment:23 Changed 7 years ago by Rawld Gill

Status: reopenedassigned

comment:24 Changed 7 years ago by Rawld Gill

Resolution: fixed
Status: assignedclosed

In [27889]:

removed unnecessary dependency; fixes #14169

comment:25 in reply to:  16 ; Changed 7 years ago by Rawld Gill

Resolution: fixed
Status: closedreopened

Replying to peller: themetester.html contains the following:

require([
	"dojo",
	"dijit/dijit",
	"dijit/dijit-all",
	"dojo/parser",
	"dojo/date/locale",
	"dojo/data/ItemFileReadStore",
	"dojo/dnd/Source"], function(dojo, dijit){

Notice that dojo/date/locale (which depends on dojo/cldr/nls/gregorian) is included in dojo/dijit-all. Therefore, it is not necessary in the require vector above and including it there is causing the extra call. This is consequent to a very subtle interaction with the concept of dynamic plugins that I need to debug further to fully understand.

However, removing then unnecessary dojo/date/locale from the dependency vector fixes the immediate problem.

I'm reopening this ticket until I have time to explain why the dynamic plugin is causing the extra request.

comment:26 Changed 7 years ago by Rawld Gill

Milestone: 1.7.21.8

comment:27 in reply to:  25 Changed 7 years ago by bill

Replying to rcgill:

Notice that dojo/date/locale (which depends on dojo/cldr/nls/gregorian) is included in dojo/dijit-all.

FYI, that's probably because we can't (or shouldn't) assume that a layer called dijit-all contains a particular file from dojo/core. (Note that dijit-all does not explicitly list dojo/date/locale in it's require list.)

comment:28 Changed 7 years ago by Rawld Gill

Status: reopenedassigned

comment:29 Changed 7 years ago by Patrick Ruzand

Cc: Patrick Ruzand added

comment:30 Changed 7 years ago by Rawld Gill

Resolution: fixed
Status: assignedclosed

In [28438]:

fixed error in preloadLocalizations which caused waiting queue to stall; gave fast path for preloadLocalizations for known AMD bundles; fixes #14169; refs #14092; !strict

comment:31 Changed 7 years ago by Rawld Gill

After [28438], build dojo with

./build.sh -p standard -r --layerOptimize 0 --optimize 0 --mini 0

and themetester loads w/out any double hits and w/out any XHRs (for NLS bundles).

Note: if any i18n work is backported to 1.7, it will all be backported. That will be decided on done on #14092.

Note: in [28438] the only substantive change in writeAmd.js is line 200; the other changes are formatting cleanup only.

Note: See TracTickets for help on using tickets.