Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#17475 closed defect (fixed)

Star mapping giving incorrect results with layers

Reported by: Bryan Forbes Owned by: Rawld Gill
Priority: blocker Milestone: 1.8.6
Component: Loader Version: 1.9.1
Keywords: Cc:
Blocked By: Blocking:

Description

When modules go from the pending cache of a built layer to the cache of the loader, if they have been mapped using "*" they are incorrectly placed. I will attach a test case, but the gist of it is that I'm using the following map configuration:

map: {
    'my/router': {
        'dojo/router': 'dojo/router'
    },
    'my/request': {
        'dojo/request/xhr': 'dojo/request/xhr'
    },
    '*': {
        'dojo/router': 'my/router',
        'dojo/request/xhr': 'my/request'
    }
}

When the layers of my application are built, 'dojo/router' comes after 'my/router' in the pending cache object. When it is iterated through and modules go from the pending cache to the loader cache, 'my/router' gets placed on the correct property ('my/router'), but then 'dojo/router' gets placed on that same property ('my/router') and overwrites the correct module. This doesn't happen when using an unbuilt version and only happens in a build.

This also seems to affect lookups of relative modules that are star mapped. In this same tarball, I have 'my/request' which gets overwritten with 'dojo/request/xhr' and the relative modules which are needed for 'dojo/request/xhr' are looked up relative to 'my'.

I believe what needs to happen is that the star configuration needs to be ignored when going from the pending cache to the loader cache. I'm unsure how best to do that, so I'll leave that to Rawld.

To build the tarball, untar it as a sibling to 'dojo' and run the following command:

node dojo/dojo.js load=build --dojoConfig ./my/dojoConfig.js -p ./my/profile.js

To test unbuilt vs. built, open my-test.html and my-built-test.html.

Attachments (2)

my.tar.gz (2.5 KB) - added by Bryan Forbes 6 years ago.
my2.tar.gz (2.8 KB) - added by ben hockey 5 years ago.

Download all attachments as: .zip

Change History (14)

Changed 6 years ago by Bryan Forbes

Attachment: my.tar.gz added

comment:1 Changed 6 years ago by Bryan Forbes

Owner: set to rawld
Status: newassigned

comment:2 Changed 6 years ago by bill

Owner: changed from rawld to Rawld Gill

comment:3 Changed 6 years ago by Rawld Gill

Excellent reduced case (as usual).

Try changing https://github.com/dojo/dojo/blob/master/dojo.js#L462

 m = getModuleInfo(p, referenceModule);

in consumePendingCacheInsert()

to...

m = getModuleInfo_(p, referenceModule, packs, modules, req.baseUrl, [], pathsMapProg, aliases);

This seems to fix the problem illustrated in the reduced case. I need to consider whether or not pathsMapProg and aliases should also be empty arrays before committing a fix.

Let me know what you see.

Changed 5 years ago by ben hockey

Attachment: my2.tar.gz added

comment:4 Changed 5 years ago by ben hockey

i've got another similar problem... the key to this problem showing up is that there is a layer that contains the module being replaced by the map. if the replaced module is not included in the layer or if the module is in the initial layer (dojo/dojo) then it works as expected (try uncommenting either of the 2 lines in the profile with the mid 'my/thing' to see how it will work in those cases).

the archive is similar to bryan's - extract it and add dojo as a sibling to the "my" package. there is my-test.html and myRelease/my-built-test.html for testing.

comment:5 Changed 5 years ago by ben hockey

I haven't tried with the reduced test case but in the application where I found this, I saw the same problem using aliases. I wonder if a similar one line fix is needed in another location

comment:6 Changed 5 years ago by Bryan Forbes

I and a coworker tested this fix pretty thoroughly and Rawld's changes are what need to happen. I have submitted a pull request (https://github.com/dojo/dojo/pull/39) with some small changes. I also took the time to think about paths and aliases and came to the conclusion that they should be ignored when modules are being inserted into the cache from the pending cache as those configuration options could also cause relative URL lookups to fail. Because of the extra changes, I added a parameter to getModuleInfo and made the change you suggested in the body of that function. Please review the changes and let me know what you think.

comment:7 Changed 5 years ago by Bryan Forbes

This pull request will also need to be backported to 1.9 and 1.8.

comment:8 Changed 5 years ago by Bryan Forbes <bryan@…>

Resolution: fixed
Status: assignedclosed

In 1d414f3b24fe22cc8f3ae8c3326d9693d3e70d47/dojo:

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

comment:9 Changed 5 years ago by Colin Snover

Resolution: fixed
Status: closedreopened

Please, backport fixes.

comment:10 Changed 5 years ago by Bryan Forbes <bryan@…>

Resolution: fixed
Status: reopenedclosed

In 58c16e0489b2a52a16299dc700c642db9533d988/dojo:

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

comment:11 Changed 5 years ago by Bryan Forbes <bryan@…>

In 9d5d0a6d24e8dc50709f4309f988f18f41154236/dojo:

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

comment:12 Changed 5 years ago by Colin Snover

Milestone: tbd1.8.6
Priority: undecidedblocker
Note: See TracTickets for help on using tickets.