Opened 10 years ago
Closed 10 years ago
#13208 closed defect (fixed)
AMD: change return values from base modules
Reported by: | ben hockey | Owned by: | ben hockey |
---|---|---|---|
Priority: | high | Milestone: | 1.7 |
Component: | Core | Version: | 1.7.0b1 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description
for background, see discussion at http://thread.gmane.org/gmane.comp.web.dojo.devel/14589
http://bugs.dojotoolkit.org/attachment/ticket/12672/baseReturns.patch is a proposed patch
use this ticket to track changing the return value of the base modules and any follow on changes needed due to this change.
Attachments (3)
Change History (18)
Changed 10 years ago by
Attachment: | baseReturns2.patch added |
---|
comment:1 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:2 Changed 10 years ago by
Milestone: | tbd → 1.7 |
---|
comment:3 Changed 10 years ago by
Changed 10 years ago by
Attachment: | dojox.charting.themes.Tufte.patch added |
---|
dojox/charting/themes/Tufte.js adapted to new dojo/_base/Color.js return value (see http://permalink.gmane.org/gmane.comp.web.dojo.devel/15171)
Changed 10 years ago by
patch to update dojox.mvc files to use dependency on dojo/_base/kernel as dojo.
comment:4 Changed 10 years ago by
I have created a patch to update dojox.mvc files to use dependency on dojo/_base/kernel as dojo. I will attach it here even though this ticket is currently closed.
comment:5 Changed 10 years ago by
comment:6 Changed 10 years ago by
@edchat - I'm not sure why you made that dojox.mvc patch, looking at the dojox.mvc files more closely, they look OK already to me. Most modules need to include dojo/_base/kernel because they are referencing the dojo variable, but dojox/mvc apparently isn't. Is something broken?
comment:7 Changed 10 years ago by
Hi Bill, nothing was broken, I created the patch based upon this discussion, http://thread.gmane.org/gmane.comp.web.dojo.devel/14589/focus=15171 Specifically your comment "My greps show these files that need to be fixed:" which included the dojox.mvc files. I don't think this patch changes anything about how dojox.mvc works, I thought it was a question of being consistent in using dojo/_base/kernel to get "dojo". If the update is not needed, please ignore or delete it. Thanks
comment:8 Changed 10 years ago by
OK, yah sorry for the confusion, I just didn't realize that the MVC code was already accessing each module's return value directly (rather than trying to use the dojo variable). Eventually all modules should work this way, but unfortunately most of them don't yet.
comment:9 Changed 10 years ago by
i want to echo bill's comments. i tried to make mvc the template for how we could write modules from the ground up. using dojo/_base/kernel is more of just a convenience for people to avoid refactoring dojo
to it's various components lang
, declare
, etc. but if anyone is writing new code i think using the return values of the base modules is a good idea.
also bill, i think your grep probably completely missed dojox/mobile. for example i randomly picked dojox/mobile/ComboBox - dojo.experimental, dojo.declare, dojox.mobile.TextBox?, dijit.form._AutoCompleterMixin, dojo.style, dojo.window, dojo.position, dojo.marginBox, dojo.hitch, dojo.doc, dojox.mobile.hasTouch - all used without declaring any dependencies on dojo, dijit or dojox. these are relying on the globals and will not work if either globals are turned off or those globals are rescoped to other names. admittedly, rescoping the globals is probably rarely going to happen but going forward we will drop the globals and i don't see why we shouldn't be writing code to prepare for that whenever we can.
a similar thing is in the themes patch on this ticket - dojox.charting.themes.Tufte
is just expected to exist as a global.
comment:10 Changed 10 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Reopening this ticket so we don't forget the other files that need to be fixed.
My grep show these files likely need to be updated:
./demos/mobileGallery/src/forms.js ./demos/mobileGallery/src/headings.js ./demos/mobileGallery/src/icons.js ./demos/mobileGallery/src/jsonp.js ./dojox/app/tests/multiSceneApp/multiSceneApp.js ./dojox/charting/themes/Tufte.js ./dojox/data/PersevereStore.js ./dojox/date/timezone.js ./dojox/encoding/crypto/Blowfish.js ./dojox/fx/_core.js ./dojox/fx/ext-dojo/complex.js ./dojox/fx/ext-dojo/NodeList-style.js ./dojox/fx/ext-dojo/NodeList.js ./dojox/fx/ext-dojo/reverse.js ./dojox/fx/flip.js ./dojox/io/OAuth.js ./dojox/io/proxy/xip.js ./dojox/io/scriptFrame.js ./dojox/io/windowName.js ./dojox/io/xhrMultiPart.js ./dojox/io/xhrScriptPlugin.js ./dojox/io/xhrWindowNamePlugin.js ./dojox/rpc/Service.js ./dojox/sketch/_Plugin.js ./dojox/sketch/Anchor.js ./dojox/sketch/Annotation.js ./dojox/sketch/DoubleArrowAnnotation.js ./dojox/sketch/Figure.js ./dojox/sketch/LeadAnnotation.js ./dojox/sketch/PreexistingAnnotation.js ./dojox/sketch/SingleArrowAnnotation.js ./dojox/sketch/Slider.js ./dojox/sketch/Toolbar.js ./dojox/sketch/UndoStack.js ./dojox/socket.js ./dojox/store/LightstreamerStore.js ./dojox/timing/_base.js ./dojox/timing/Sequence.js ./dojox/uuid/_base.js ./dojox/uuid/tests/uuid.js ./dojox/uuid/Uuid.js ./dojox/validate/_base.js ./dojox/validate/br.js ./dojox/validate/check.js ./dojox/validate/creditCard.js ./dojox/validate/isbn.js ./dojox/widget/ColorPicker.js ./dojox/xml/DomParser.js ./dojox/xml/Script.js ./dojox/xml/tests/parser.js ./dojox/xml/widgetParser.js
comment:11 Changed 10 years ago by
comment:12 Changed 10 years ago by
comment:13 Changed 10 years ago by
comment:14 Changed 10 years ago by
comment:15 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
the work related to this ticket seems to be completed. i'm closing it but feel free to reopen if you think it needs to be opened.
newer patch than one on http://bugs.dojotoolkit.org/attachment/ticket/12672/baseReturns.patch