Opened 9 years ago

Closed 8 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)

baseReturns2.patch (7.0 KB) - added by bill 9 years ago.
newer patch than one on http://bugs.dojotoolkit.org/attachment/ticket/12672/baseReturns.patch
dojox.charting.themes.Tufte.patch (2.7 KB) - added by cjolif 8 years ago.
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)
mvc.patch (6.6 KB) - added by Ed Chatelain 8 years ago.
patch to update dojox.mvc files to use dependency on dojo/_base/kernel as dojo.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 9 years ago by bill

Resolution: fixed
Status: newclosed

(In [25490]) fix dojo/_base modules to only return the symbols they define, fixes #13208 !strict

comment:2 Changed 9 years ago by bill

Milestone: tbd1.7

comment:3 Changed 9 years ago by bill

(In [25520]) fix missing dependency on dojo/_base/kernel, fixes #12850, refs #12863, #13208 !strict.

Changed 8 years ago by cjolif

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 8 years ago by Ed Chatelain

Attachment: mvc.patch added

patch to update dojox.mvc files to use dependency on dojo/_base/kernel as dojo.

comment:4 Changed 8 years ago by Ed Chatelain

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 8 years ago by bill

(In [25533]) Fix from cjolif (thanks!) to adapt to new dojo/_base/Color.js return value, refs #13208.

comment:6 Changed 8 years ago by bill

@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 8 years ago by Ed Chatelain

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 8 years ago by bill

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 8 years ago by ben hockey

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 8 years ago by bill

Resolution: fixed
Status: closedreopened

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 8 years ago by ben hockey

(In [25616]) ensure reference to dojo is as expected refs #13208 !strict

comment:12 Changed 8 years ago by ben hockey

(In [25617]) added missing dependencies in dojox/fx. probably still not complete but better than it was. refs #13208 !strict

comment:13 Changed 8 years ago by ben hockey

(In [25618]) added missing dependencies in dojox/io/OAuth refs #13208 !strict

comment:14 Changed 8 years ago by bill

(In [25868]) For event.js, return hash with keys "fix" and "stop" rather than "fixEvent" and "stopEvent". The latter names are redundant with the module name itself. Refs #13208 !strict.

comment:15 Changed 8 years ago by ben hockey

Resolution: fixed
Status: reopenedclosed

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.

Note: See TracTickets for help on using tickets.