Opened 7 years ago

Closed 7 years ago

#15535 closed defect (fixed)

somewhere between 1.7.1 and 1.7.2 packageMap broke with i18n dependencies after a build

Reported by: ben hockey Owned by: Rawld Gill
Priority: blocker Milestone: 1.8
Component: BuildSystem Version: 1.7.2
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by ben hockey)

packageMap no longer works with i18n resources after doing a build. see attached test case for a demonstration of code that works with dojo 1.7.1 but is broken in 1.7.2+

i'm attaching files that should be arranged like this:

rootDir/

pkg/

layer.js - some code with an i18n dependency

pkg.profile.js - build profile plain.html - loads pkg/layer without using a packageMap config packageMap.html - loads pkg/layer using a packageMap config

Attachments (4)

layer.js (141 bytes) - added by ben hockey 7 years ago.
pkg/layer module
pkg.profile.js (454 bytes) - added by ben hockey 7 years ago.
build profile
packageMap.html (543 bytes) - added by ben hockey 7 years ago.
load layer with packageMap config
plain.html (298 bytes) - added by ben hockey 7 years ago.
load layer without packageMap

Download all attachments as: .zip

Change History (17)

Changed 7 years ago by ben hockey

Attachment: layer.js added

pkg/layer module

Changed 7 years ago by ben hockey

Attachment: pkg.profile.js added

build profile

Changed 7 years ago by ben hockey

Attachment: packageMap.html added

load layer with packageMap config

Changed 7 years ago by ben hockey

Attachment: plain.html added

load layer without packageMap

comment:1 Changed 7 years ago by ben hockey

Milestone: tbd1.8
Priority: undecidedblocker

comment:2 Changed 7 years ago by ben hockey

Description: modified (diff)

comment:3 Changed 7 years ago by Rawld Gill

Priority: blockerlow
Status: newassigned

The problem lies in the preload localizations code at the bottom of the built layer, to wit:

'*now':function(r){r(['dojo/i18n!*preload*pkg/nls/layer*["ar","ca","cs","da","de-de","el","en-gb","en-us","es-es","fi-fi","fr-fr","he-il","hu","it-it","ja-jp","ko-kr","nl-nl","nb","pl","pt-br","pt-pt","ru","sk","sl","sv","th","tr","zh-tw","zh-cn","ROOT"]']);}
}

Notice that pkg/nls/layer is an absolute module id, and therefore does not get mapped since the v_pkg config has no mapping from pkg. Normally, such a mapping is not required because all modules in the pkg package will have been expressed with the best practice of only using relative module ids. In this case, the builder breaks that best practice, which causes the error.

In the given example, the simple fix is to add a line to the package map configuration for the v_pkg package in packageMap.html like this:

{
	name: 'v_pkg',
	location: 'pkg',
	packageMap: {
		dojo: 'v_dojo',
		pkg: 'v_pkg'
	}
}

Notice that single additional line that maps pkg is mapped to v_pkg. With this, the example works.

The alternative fix is to change the builder to output relative module ids. I'm keeping the ticket open to look into this.

@neonstalwart: could you verify that the quick fix works for you...particularly if you've got a real-world app?

comment:4 Changed 7 years ago by ben hockey

Priority: lowblocker

did you try the example or make any other changes to the example? i can't even get the example to work with this single additional line. it keeps trying to grab the nls bundle from one directory too high. rather than "release/pkg/nls/..." it's trying "pkg/nls/..."

Last edited 7 years ago by ben hockey (previous) (diff)

comment:5 Changed 7 years ago by ben hockey

btw, i updated the packageMap for dojo (which is wrong in my example). my latest packages config is

packages: [
        {
                name: 'v_dojo',
                location: 'dojo',
                packageMap: {
                        dojo: 'v_dojo',
                        dijit: 'v_dijit',
                        dojox: 'v_dojox'
                }
        },
        {
                name: 'v_pkg',
                location: 'pkg',
                packageMap: {
                        dojo: 'v_dojo',
                        pkg: 'v_pkg'
                }
        }
],
Last edited 7 years ago by ben hockey (previous) (diff)

comment:6 Changed 7 years ago by Rawld Gill

As of [2117]: The i18n module is using the wrong context-require to resolve the module id into a url. It is using the context-require with respect to dojo/i18n...which was mapped to v_dojo in the example. Since v_dojo did not map pkg to v_pkg, there was no package config found and the loader resolved the absolute module id pkg/nls/layer to the wrong location.

It is possible to get this to work without changes to the code with the following config:

packages: [
		{
				name: 'v_dojo',
				location: 'dojo',
				packageMap: {
						pkg: 'v_pkg',
						dojo: 'v_dojo',
						dijit: 'v_dijit',
						dojox: 'v_dojox'
				}
		},
		{
				name: 'v_pkg',
				location: 'pkg',
				packageMap: {
						dojo: 'v_dojo',
						pkg: 'v_pkg'
				}
		}
],

This causes pkg to be mapped to v_pkg in the v_dojo package...which is the package that loads dojo/i18n.

Still, dojo/i18n should always resolve modules ids with respect to the context-require that is loading a particular module.

comment:7 Changed 7 years ago by Rawld Gill

Resolution: fixed
Status: assignedclosed

In [29072]:

resolve modules ids with respect to the context-require that is loading a particular module instead of the context-require of dojo/i18n; fixes #15535; !strict

comment:8 Changed 7 years ago by ben hockey

Resolution: fixed
Status: closedreopened

this is looking better but still it's not correct. there are 3 nls files being loaded - "release/pkg/nls/layer_en-us.js", "release/dojo/cldr/nls/number.js" and "release/dojo/cldr/nls/en/number.js". there should only be the first one loading. you can see how this works without packageMap by using the plain.html file.

comment:9 Changed 7 years ago by ben hockey

fyi - my current config with packageMap:

packages: [
        {
                name: 'v_dojo',
                location: 'dojo',
                packageMap: {
                        dojo: 'v_dojo',
                        dijit: 'v_dijit',
                        dojox: 'v_dojox'
                }
        },
        {
                name: 'v_pkg',
                location: 'pkg',
                packageMap: {
                        dojo: 'v_dojo',
                        pkg: 'v_pkg'
                }
        }
],

comment:10 Changed 7 years ago by Rawld Gill

Resolution: fixed
Status: reopenedclosed

In [29080]:

map preload bundles; fixes #15535; !strict

comment:11 Changed 7 years ago by ben hockey

YES!!!

comment:12 Changed 7 years ago by ben hockey

Resolution: fixed
Status: closedreopened

this has regressed after r29249

comment:13 Changed 7 years ago by Rawld Gill

Resolution: fixed
Status: reopenedclosed

Just retested after [29257] and I got it to work with the following package config:

packages: [
	{
		name: 'dojo',
		location: 'dojo'
	},{
		name: 'v_dojo',
		location: 'dojo',
		packageMap: {
			dojo: 'v_dojo',
			dijit: 'v_dijit',
			dojox: 'dojox'
		}
	},
	{
		name: 'v_pkg',
		location: 'pkg',
		packageMap: {
			pkg: 'v_pkg',
			dojo: 'v_dojo'
		}
	}
],

I think this is reasonable. Notice that a dojo package config is required because the profile caches the dojo modules needed by pkg/layer into dojo/dojo.js. The loader needs a way to understand that those modules are the same. Since the pkg/layer dependencies are not cached explicitly, the loader looks to see if the modules that reside at the same URL happen to be cached (which they are). But the only way to know that (e.g.) dojo/i18n resides at release/dojo/i18n.js is to inform the loader where the dojo package resides.

Note: See TracTickets for help on using tickets.