Opened 5 years ago

Last modified 2 years ago

#18129 assigned defect

[patch][cla] multiple dojo instance does not work as expected when using layers

Reported by: ido Owned by: Dasa Paddock
Priority: low Milestone: 1.14
Component: Loader Version: 1.10.0
Keywords: Cc:
Blocked By: Blocking:

Description

Let's consider the following config:

map: {
    'application1': {
        'dojo': 'dojo1.9'
    },
    'application2': {
        'dojo': 'dojo1.10'
    }
},
packages: [
    {
        name: 'application1',
        location '/apps/1'
    },
    {
        name: 'dojo1.9',
        location '/apps/1/dojo'
    }
    {
        name: 'application2',
        location '/apps/2'
    },
    {
        name: 'dojo1.10',
        location '/apps/2/dojo'
    }
]

Consider now that application1 is loaded first. In this application you have a button that load application2.

When I use non-builded code, requiring dojo/string from application2/myWidget load the file '/apps/2/dojo/string.js' as expected.

However, if application1 is a layer and application2 is also a layer, then requiring dojo/string from application2/myWidget resolve as '/apps/1/dojo/string.js'

This is due to the exclusion of mapProg in getModuleInfo: the change: https://github.com/dojo/dojo/commit/1d414f3b24fe22cc8f3ae8c3326d9693d3e70d47 the bug: #17475 the reason of that choice is described here : https://bugs.dojotoolkit.org/ticket/17475#comment:6

As a result: if the application1's layer does not contains the module required by application2, then it will be loaded from the separate file (it resolve to the right URL, but the layer become useless). Even worse if application1's layer contains the module it loads it from here, but the version might be totally different!

I think the maps should always be passed.

Note: it affect dojo 1.8+

Attachments (4)

layerBuild.zip (5.9 KB) - added by ido 5 years ago.
test case
layerBuildstarmap.zip (7.5 KB) - added by ido 5 years ago.
testcase including * mapping
require-test.zip (7.3 KB) - added by Dasa Paddock 3 years ago.
require with config example
require-test_mod.zip (12.7 KB) - added by Michael J Van Sickle 3 years ago.
modified version of require-test.zip that comments out non-Dojo packages

Download all attachments as: .zip

Change History (25)

comment:1 Changed 5 years ago by ido

in the test case, test-no-layer.html represent the "dev" env test-layer.html represent the "prod" env

Changed 5 years ago by ido

Attachment: layerBuild.zip added

test case

Changed 5 years ago by ido

Attachment: layerBuildstarmap.zip added

testcase including * mapping

comment:2 Changed 5 years ago by ido

I've attached another test case, which enrich the previous one with the * mapping (as described in #17475)

comment:3 Changed 5 years ago by ido

I've created a pull request: https://github.com/dojo/dojo/pull/101

Not sure if my tests are covering all possibles use-cases so if someone with strong loader knowledge can review it, that would be nice.

@dylan: Can you add [CLA] [PATCH] to the title ? Thanks

comment:4 Changed 5 years ago by dylan

Milestone: tbd1.11
Priority: undecidedhigh
Summary: multiple dojo instance does not work as expected when using layers[patch][cla] multiple dojo instance does not work as expected when using layers

comment:5 Changed 4 years ago by dylan

Owner: set to dylan
Status: newassigned

comment:6 Changed 4 years ago by dylan

Priority: highlow

comment:7 Changed 4 years ago by dylan

Resolution: fixed
Status: assignedclosed

comment:8 Changed 3 years ago by Dasa Paddock

I think this change caused a regression issue when passing runtime configuration to require.

comment:9 Changed 3 years ago by dylan

Milestone: 1.111.11.2
Resolution: fixed
Status: closedreopened

@dasa, I saw your note earlier and was thinking this might be the culprit, but can you explain what the regression is?

comment:10 Changed 3 years ago by Dasa Paddock

When our app passed the dojo config in its first call to require none of the config properties were then set in dojo.config.

comment:11 Changed 3 years ago by Dasa Paddock

I'm unable to create a reproducible test case so hopefully this problem is just unique to our specific app. To resolve this issue we've reverted this commit in our copy of dojo 1.11.1.

comment:12 Changed 3 years ago by iDo

@dasa can you post part of your setup and explain the initialisation of your app ? Maybe we can guess what's going on even if you can make a reproductible test case out of your app.

Changed 3 years ago by Dasa Paddock

Attachment: require-test.zip added

require with config example

comment:13 Changed 3 years ago by Dasa Paddock

I've attached a zip with our build profile and initialization code.

comment:14 Changed 3 years ago by Dasa Paddock

I should note that the problem only occurs when loading the built app. It's not a problem when loaded from source.

comment:15 Changed 3 years ago by iDo

@dasa thanks. Just to be sure on what I am searching :

When you do

<script data-dojo-config="parseOnLoad:false" src="js/jsapi/dojo/dojo.js"></script>


require({
    parseOnLoad: true
}, ['dojo/_base/config'], function(config) {
    console.log(config.parseOnLoad)
});


you expect "true" in the console and you got "false" ?

I do use contextual requires in my project but I inject only "packages". So far we had no issues with this property: it works on our setup (both build and dev), but we don't read the values over dojo/_base/config

I see in your example your are injecting

baseUrl: "js",
debug: false,
has: {'dojo-bidi': true},
locale: locale,
parseOnLoad: true,
packages: [/*...*/]

None of them are available ?

comment:16 Changed 3 years ago by Dasa Paddock

Correct, none of the properties passed to require are being set.

comment:17 Changed 3 years ago by dylan

We didn't think that require({config}) would modify dojoConfig at all. Instead it keeps it on the require object.

See https://github.com/dojo/dojo/blob/master/_base/config.js#L165 , it uses require.rawConfig if it is the dojo loader, which gets brought during dojo.js loading

During booting, dojo.js grabs dojoConfig and then sets its internal config from that point.

https://github.com/dojo/dojo/blob/master/dojo.js#L569 is where config is setup and it introspects whatever config is passed in, but it doesn't change what is passed in… and it depends on what the config actually contains as to whether it will affect the configuration

So in the case of async, subsequent calls passing a require(config) will overwrite whatever the async state is at boot (https://github.com/dojo/dojo/blob/master/dojo.js#L580).

I'm not sure if this fully helps clarify what the expected behavior is, but I think it clarifies for me that you should rely on the values in dojo/_base/config for that particular version of Dojo rather than reading the global dojoConfig.

comment:18 Changed 3 years ago by Michael J Van Sickle

I've done some research into the behavior described in the require-test.zip example, and I now believe that this is not a regression. Rather, it appears to be a confluence of two issues that are causing the build to behave differently than the unbuilt code.

I've modified the test case slightly to allow it to build in a pure Dojo environment (no external dependencies) and attached that file. To run the build, copies of dojo, dijit, dojox, and util have to be dropped into the src/js/jsapi folder and then the project can be built with node src/js/jsapi/dojo/dojo.js load=build --profile ./build/activity-dashboard-amd.profile.js, run from the root folder.

With that modified example, I was able to reproduce the behavior that the configuration was not being passed through. After researching the issue, I discovered that none of the configuration was available, not even Dojo's default values. This lead me to investigate how dojo/_base/config was loading. It turns out that this line: https://github.com/dojo/dojo/blob/master/_base/config.js#L152 was resolving to undefined in the built code. That led me to dojo/has, which is the place that the issue is stemming from.

The layer in the build profile has customBase: true, which excludes many of Dojo's standard configuration data from the build. Among the exclusions is the setting to add in "dojo-has-api" to 1 in dojo/has. That means that when the dojo/has module is loaded at runtime, this line (https://github.com/dojo/dojo/blob/master/has.js#L19) comes up undefined, and a new has module us built, without the previous values. Setting customBase to false resolves this issue and the built code code behaves as expected.

Another way to resolve this issue would be to include dojo/has in the layer. Since the profile has 'dojo-has-api': 1 in the staticHasFeatures hash, it will allow dojo/has to be loaded properly and, once again, the built code behaves as expected.

I tried both of these solutions with master's tip and the commit prior to https://github.com/dojo/dojo/commit/3c4337021dba9f60c6ffc2beb7a9696f87c2c4f8 and both versions worked and failed in the same manner.

My recommendation is to add "dojo/has" to the "dojo/dojo" layer and see if the issue is resolved.

Changed 3 years ago by Michael J Van Sickle

Attachment: require-test_mod.zip added

modified version of require-test.zip that comments out non-Dojo packages

comment:19 Changed 3 years ago by dylan

Milestone: 1.11.21.11.3

comment:20 Changed 3 years ago by dylan

Milestone: 1.11.31.13
Owner: changed from dylan to Dasa Paddock
Status: reopenedassigned

@dasa Did @mvansickle's suggestion resolve the issue for you?

comment:21 Changed 2 years ago by dylan

Milestone: 1.131.14
Note: See TracTickets for help on using tickets.