Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#12098 closed defect (fixed)

dojo/uacss is leaking globals with AMD transform

Reported by: Adam Peller Owned by: Rawld Gill
Priority: high Milestone: 1.6
Component: Core Version: 1.5
Keywords: amd Cc:
Blocked By: Blocking:

Description (last modified by Adam Peller)

...but only after a build. It seems like whatever transform is taking place ought to insert a closure to protect the global namespace?

See console for http://archive.dojotoolkit.org/dojo-2010-12-16/dojotoolkit/dijit/themes/themeTester.html

Change History (7)

comment:1 Changed 9 years ago by Adam Peller

Description: modified (diff)

comment:2 Changed 9 years ago by ben hockey

(In [23376]) putting back a closure which should not have been removed in r23032. !strict refs #12098

comment:3 Changed 9 years ago by ben hockey

i found a few more - will checkin after testing a build.

comment:4 Changed 9 years ago by ben hockey

Resolution: fixed
Status: newclosed

(In [23377]) putting back some more closures which should not have been removed in r23032. !strict fixes #12098

comment:5 Changed 9 years ago by Adam Peller

Ben, these closures you added are already in functions. Shouldn't the closure protection be implicit by the AMD syntax (and provided by the transform here?) There's no reason why a var d; inside the function definition should be a global or require that additional closure.

comment:6 in reply to:  5 ; Changed 9 years ago by ben hockey

Replying to peller:

Ben, these closures you added are already in functions. Shouldn't the closure protection be implicit by the AMD syntax (and provided by the transform here?) There's no reason why a var d; inside the function definition should be a global or require that additional closure.

i agree - we are in "limbo" until we have a fully implemented AMD loader and build system. the build transform is basically a regex to strip out the define call (and the closure created by those callback functions). so, that's the reason they still need to be in closures. there are also a number of other implications which are not ideal - #11919 covers a few more of these limitations. the current situation is definitely fragile and so i'm hoping we'll go all the way with a build tool and loader sooner rather than later.

fwiw, the changes i made just put back closures which existed before and with the current build transform, they still need to exist.

comment:7 in reply to:  6 Changed 9 years ago by Rawld Gill

Replying to neonstalwart:

Replying to peller:

Ben, these closures you added are already in functions.

[snip]

i agree - we are in "limbo" until we have a fully implemented AMD loader and build system. the build transform is basically a regex to strip out the define call (and the closure created by those callback functions).

Ben--Thanks for catching/fixing this so quickly!!

I agree with your analysis. The "optimization" I made will not work after the reverse transform is applied by the current build system. fwiw...I'm nearing completion of the new build system that will resolve all these problems.

Note: See TracTickets for help on using tickets.