Opened 7 years ago

Closed 7 years ago

#14798 closed defect (fixed)

[patch][cla] loader tries to call an array as a function

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

Description (last modified by ben hockey)

see http://bugs.dojotoolkit.org/browser/dojo/tags/release-1.7.2rc1/dojo/dojo.js#L1780

r27839 revealed what looks like a copy/paste bug in dojo.js where if defaultConfig.deps gets set (as is done by dojo/_base/configNode.js) then bootCallback is assigned to defaultconfig.deps rather than defaultConfig.callback. when the code reaches, http://bugs.dojotoolkit.org/browser/dojo/tags/release-1.7.2rc1/dojo/dojo.js#L690, a1.length is 0 but a2 is not a function (it's the empty defaultConfig.deps array).

i think the fix is simply this (change defaultConfig.deps to defaultConfig.callback):

    bootCallback = defaultConfig.callback || userConfig.callback || dojoSniffConfig.callback;

i think this fix should be in the final release of 1.7.2 since the bug was introduced/uncovered by a change for 1.7.2 and it completely breaks dojo in node.js when used without the --load command line arg.

Change History (6)

comment:1 Changed 7 years ago by ben hockey

Description: modified (diff)
Milestone: tbd1.7.2
Priority: undecidedblocker

comment:2 Changed 7 years ago by Rawld Gill

In [27872]:

fixed typo; thanks neonstalwart; refs #14798; !strict

comment:3 Changed 7 years ago by Rawld Gill

In [27873]:

backport [27872]; refs #14798; !strict

comment:4 in reply to:  description Changed 7 years ago by Rawld Gill

Replying to neonstalwart:

Thanks for the catch Ben.

Looking at this code, I can't remember why we are preferring the default config to the user config or the sniffed config...it seems exactly backwards. Do you remember?

comment:5 Changed 7 years ago by ben hockey

i don't know that i have any idea why it happens the way it does. i agree that it seems exactly backwards (sniff > user > default). given that we've found this typo perhaps it's reasonable to doubt this section of code. also since it seems exactly backwards maybe that's another indicator that the order is a result of human error rather than being intended.

we should probably close this ticket and (if needed) open a new one that's not marked as a blocker for 1.7.2

comment:6 Changed 7 years ago by Rawld Gill

Resolution: fixed
Status: newclosed

tracking the questionable code about priorities in #14810.

Note: See TracTickets for help on using tickets.