Opened 13 years ago

Closed 13 years ago

Last modified 12 years ago

#627 closed defect (fixed)

Bootstrap refactoring to reduce bloat and separate package loading

Reported by: James Burke Owned by: James Burke
Priority: high Milestone:
Component: Core Version: 0.3
Keywords: bootstrap package load Cc:
Blocked By: Blocking:

Description

This is a ticket to track the work described in this URL:

http://dojo.jot.com/WikiHome/BootstrapBloat

Attachments (3)

bootstrapRefactor.patch (76.3 KB) - added by James Burke 13 years ago.
Patch to trunk to refactor bootstrap files.
bootstrapPatch2.txt (76.6 KB) - added by James Burke 13 years ago.
Pass #2 at patch to trunk to refactor bootstrap files.
bootstrapPatch2.tar (10.0 KB) - added by James Burke 13 years ago.
Pass #2 at patch to trunk to refactor bootstrap files (New files)

Download all attachments as: .zip

Change History (9)

comment:1 Changed 13 years ago by James Burke

I created a patch for this work. If it looks good, then I will apply it to the trunk. I created the patch by doing a svn diff against my version of trunk (r3562).

If possible, I would like to have Paul Sowden review the patch, but I welcome feedback from anyone on it. Some notes about the patch vs. the work that was described on the wiki page:

  • bootstrap2.js now loads before the hostenv_*.js file.
  • Put the addOnLoad stuff into bootstrap2.js, and the methods that back require and provide. This means for a dojo-lite that it would have to provide stub functions for this functionality.
  • Kept dojo.hostenv.conditionalLoadModule and removed dojo.kwCompoundRequire. There were lots of references to conditionalLoadModule in package.js files, and these files also use dojo.hostenv.moduleLoaded(). It seemed odd to have dojo.kwCompoundRequire paired with dojo.hostenv.moduleLoaded().
  • docscripts/parser.php: has a reference to dj_deprecated. Also, it is looking for dojo.hostenv.kwCompoundRequire, but it should just be dojo.kwCompoundRequire (but I removed that method now anyway).
  • Removed requireAfterIf and used requireIf instead. However, I kept the requireAfterIf function defintion since it was used in a bunch of widgets. Didn't want to remove it yet, since I figured someone would holler (if they had done a copy/paste from the old code).
  • Kept dojo.loaded() for now.

Stuff I didn't do, but might be worth considering.

  • hostenv_adobesvg.js: affected by changes, but I don't think adversely.
  • hostenv_browser.js: 1st part parses the query string to look for djConfig parameters. Is this used by anyone?
  • hostenv_browser.js: 3rd part does browser detection and sets variables off of dojo.render. There is one block that is concerned with detecting Adobe SVG. Can it be removed? Look for the comment: check for ASVG
  • Didn't try to convert dojo.exists to dojo.evalObjPath. Anyone else want to give it a shot to see if they are interchangeable (just got lazy).
  • bootstrap2.js: addedToLoadingCount: is modified by never read. Remove?
  • bootstrap2.js: removedFromLoadingCount is modified by never read. Remove?
  • dojo.hostenv.setModulePrefix and dojo.setModulePrefix. It would be nice to just go to dojo.setModulePrefix, but the hostenv version is mentioned extensively in docs.
  • Lots of test files push into modulesLoadedListeners directly instead of using dojo.addOnLoad. It would be nice to change everyone to use addOnLoad.

I'll upload the patch to this ticket.

Changed 13 years ago by James Burke

Attachment: bootstrapRefactor.patch added

Patch to trunk to refactor bootstrap files.

comment:2 Changed 13 years ago by psowden

  • _addHostEnv should be wrapped up in a closure so as not to remain in global scope.
  • dojo.debug should be added back in somewhere. I liked the idea of a debug.js, but I would be tempted to keep this seperate from browser_debug.js, so creating a new file to contain the lost debug methods seems like an idea.
  • Stub functions should be created for all removed functions that indicate they will be removed by 0.4. No functionality should be removed before the next release.

comment:3 Changed 13 years ago by James Burke

Addressing your three points, Paul:

  1. I will do that.
  1. I think I had a problem again with SVN and newly added files. I do have a debug.js that contains dojo.debug() and dojo.debugShallow(). You can see it with the snapshot of my code up here: http://tagneto.org/dojo/bootstrap/src/debug.js. I added the following block to dojo.js (also visible at http://tagneto.org/dojo/bootstrap/dojo.js):
    	if((this["djConfig"])&&(djConfig["isDebug"])){
    		tmps.push("debug.js");
    	}
    
    

I think I should also tmps.push("debug.js") if debugAtAllCosts is true. I will add that to the if statement above. How does that sound?

  1. I thought one of the points of the cleanup was to remove the functions, and the posts I did to the listservs and dojo blog were to warn people that they were going away. I will follow up in the IRC meeting today to see what I should add back.

Thanks for the review!

comment:4 Changed 13 years ago by James Burke

Attaching new patch file that addresses the following points. Paul Sowden, can you review again?

  1. _addHostEnv is wrapped up in a closure so as not to remain in global scope.
  2. debug.js will be included if djConfig.isDebug is true or if debugAtAllCosts is true.
  3. There is support for a compatibility package. If djConfig.compat = "0.2.2" is set, then dojo.js will load src/compat/0.2.2.js. This file has the function definitions that were removed as part of the bootstrap refactoring.

I will upload a diff file and a tar of the newly added files. You can also find the source and tests online at:

http://tagneto.org/dojo/bootstrap/src and http://tagneto.org/dojo/bootstrap/tests

Changed 13 years ago by James Burke

Attachment: bootstrapPatch2.txt added

Pass #2 at patch to trunk to refactor bootstrap files.

Changed 13 years ago by James Burke

Attachment: bootstrapPatch2.tar added

Pass #2 at patch to trunk to refactor bootstrap files (New files)

comment:5 Changed 13 years ago by James Burke

Resolution: fixed
Status: newclosed

I'm done with this for now. Any new bootstrap bloat reduction ideas should be filed as new bugs.

comment:6 Changed 12 years ago by (none)

Milestone: 0.3release

Milestone 0.3release deleted

Note: See TracTickets for help on using tickets.