#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 )
...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 10 years ago by
Description: | modified (diff) |
---|
comment:2 Changed 10 years ago by
comment:4 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:5 follow-up: 6 Changed 10 years ago by
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 follow-up: 7 Changed 10 years ago by
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 Changed 10 years ago by
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.
(In [23376]) putting back a closure which should not have been removed in r23032. !strict refs #12098