Opened 9 years ago

Closed 9 years ago

Last modified 8 years ago

#12200 closed enhancement (fixed)

construct an AMD package bootstrap

Reported by: Rawld Gill Owned by: Rawld Gill
Priority: high Milestone: 1.6
Component: Loader Version: 1.6.0b1
Keywords: Cc: Kris Zyp, James Burke, ben hockey
Blocked By: Blocking:

Description

Construct an AMD package bootstrap that is compat with 1.6 for use with compliant AMD loaders. The bootstrap need not be optimal and is considered experimental, but it should be free of gross kludges and serve as a reasonable starting point towards clean AMD operation.

Attachments (1)

12200_plugins.diff (7.9 KB) - added by ben hockey 9 years ago.
changes to plugins

Download all attachments as: .zip

Change History (31)

comment:1 Changed 9 years ago by Rawld Gill

Owner: changed from anonymous to Rawld Gill
Status: newassigned

comment:2 Changed 9 years ago by Rawld Gill

Cc: ben hockey added

comment:3 Changed 9 years ago by Rawld Gill

[23633] fixes this ticket; it includes the following:

  • bootstrap modules to load dojo as a package with an AMD loader
  • removal of the requirement to load the sync loader when loading with an AMD loader (loading the sync loader during bootstrapping an AMD-loaded dojo made no sense since many of the loader functions looked like the worked, but in fact, they did not)
  • redesign of the AMD bootstrap sequence to remove the circular dojo dependency
  • separation of the AMD-only modules from the standard 1.x modules to the dir dojo/lib
  • i18n and text AMD plugins that leverage dojo in order to begin work on making dojo very small
  • rewrapping of dojo/_base/_loader/bootstrap and dojo/_base/_loader/hostenv_browser so that they operate exactly as 1.5 if not using an AMD loader
  • additional build pragmas to ensure that no AMD artifacts remain after a build with the v1.6 build utility

See commentary in resources in dojo/lib/* for details.

Finally, note that dojo/lib/main-browser.js replaces dojo/_base/_loader/package-main.

This change set works with requirejs, bdBuild, cpm, neonstalwart's template, and other examples that I'll point to once they are published.

comment:4 Changed 9 years ago by ben hockey

rawld,

this change is definitely a cleaner approach. i'm testing the changes at the moment and i'm finding that dojo.moduleUrl isn't working as expected with requirejs (v0.22.0). dojo.moduleUrl('dijit') gives a uri of "./dijit/[object Object]"

comment:5 in reply to:  4 Changed 9 years ago by Rawld Gill

Replying to neonstalwart:

rawld,

this change is definitely a cleaner approach. i'm testing the changes at the moment and i'm finding that dojo.moduleUrl isn't working as expected with requirejs (v0.22.0). dojo.moduleUrl('dijit') gives a uri of "./dijit/[object Object]"

Thanks!

Yes, I need to write some tests for the compat module. I ported it from dojo-sie, but some things have changed since it was constructed over there. I tested [23633] with requirejs and bdLoad with Kris's cpm demo, your template demo, and a couple of other quick and dirty smoke tests. I also ran all the dojo and many of the dijit tests with both non-build and build on several platforms including ie6. I was hugely concerned with not breaking non-AMD, and, in fact, removing AMD stuff from 1.6 built code in hopes of making 1.6 more reliable at this point in the rel cycle.

comment:6 Changed 9 years ago by Rawld Gill

Tests need to be added for the backCompat module.

comment:7 Changed 9 years ago by Rawld Gill

(In [23642]) repairs to AMD backCompat, refs #12200 !strict

comment:8 Changed 9 years ago by Rawld Gill

Resolution: fixed
Status: assignedclosed

(In [23653]) fixes #12200, !strict

comment:9 Changed 9 years ago by Rawld Gill

(In [23655]) fixed bad test config, refs #12200

comment:10 Changed 9 years ago by Rawld Gill

Tests have been added at dojo/tests/amd.

In order to run the tests, you'll need an AMD loader. Install one or both of

as follows:

+ some-directory
  + bdLoad
    + lib
    + test
  + dojotoolkit
    + dijit
    + dojo
    + dojox
    + util
  + requirejs
    require.js
    + require
    <etc>

Next point your browser to

some-directory/dojo/tests/amd/smoke-bdLoad.html
  ...to see bdLoad load the dojo package
some-directory/dojo/tests/amd/smoke-requirejs.html
  ...to see requirejs load the dojo package
some-directory/dojotoolkit-rcg/util/doh/runner.html?dojoUrl=../../../bdLoad/lib/require.js&testUrl=../../../dojo/tests/amd/main
  ...to see the backcompat unit tests with bdLoad
some-directory/dojotoolkit-rcg/util/doh/runner.html?dojoUrl=../../../requirejs/require.js&testUrl=../../dojo/tests/amd/main
  ...to see the backcompat unit tests with requirejs

comment:11 in reply to:  10 Changed 9 years ago by Rawld Gill

Let me try those directories again:

some-directory/dojotoolkit/dojo/tests/amd/smoke-bdLoad.html
some-directory/dojotoolkit/dojo/tests/amd/smoke-requirejs.html
some-directory/dojotoolkit/util/doh/runner.html?dojoUrl=../../../bdLoad/lib/require.js&testUrl=../../../dojo/tests/amd/main
some-directory/dojotoolkit/util/doh/runner.html?dojoUrl=../../../requirejs/require.js&testUrl=../../dojo/tests/amd/main

comment:12 Changed 9 years ago by Douglas Hays

(In [23659]) Refs #12200. All robot tests were failing on IE after [23633]. dijit was not being defined globally by the robot.

comment:13 in reply to:  12 Changed 9 years ago by Rawld Gill

Replying to doughays:

(In [23659]) Refs #12200. All robot tests were failing on IE after [23633]. dijit was not being defined globally by the robot.

Thanks for finding/fixing. After looking at the code, I don't understand how [23633] caused this change in behavior (dijit is being put into the global scope in bootstrap). I also ran some dijit+robot tests on IE before the check-in and they seemed to work. If you know why [23633] caused the change, could you give me a quick explanation so I can consider if there are other potential problems? Also, which version of IE...I'd like to make sure my test environment is showing the same failures so I can test this environment in the future.

Thanks again!

comment:14 Changed 9 years ago by Douglas Hays

dijit/tests/layout/robot/GUI.html should be a good test if you can find out what was broken since it was failing on IE6/7/8 starting with [23633]. The test executed dijit.byId(blah) and that was undefined, even thought dijit was defined. dijit should have been the dijit of the inner iframe and not the dijit in which the robot was running. I don't understand either what [23633] did to break that but it was failing on all 3 IE boxes that I was testing with. I backed out [23633] and the tests passed after that.

comment:15 Changed 9 years ago by Rawld Gill

(In [23662]) fixed dojo._Url and warning msgs in backCompat, refs #12200 ,!strict

comment:16 Changed 9 years ago by Rawld Gill

Resolution: fixed
Status: closedreopened

dojo/i18n is not compatible with an AMD-loaded dojo.

comment:17 Changed 9 years ago by Rawld Gill

Resolution: fixed
Status: reopenedclosed

(In [23679]) fixes #12200; repairs dojo/i18n error when using AMD loader; improves build output for dojo/query, !strict

comment:18 Changed 9 years ago by ben hockey

Resolution: fixed
Status: closedreopened

due to some of the backCompat i18n changes, dojo.requireLocalization fails with requirejs. as an example:

require(['dojo/date/locale'], function (locale) { 
    locale.getNames('months', 'wide', 'standAlone'); 
});

the error reported by requirejs is "require: module name 'require/i18n!dojo/cldr/nls/en-us/gregorian' has not been loaded yet for context: _"

which is correct - the only module that requirejs knows has been requested is 'require/i18n!dojo/cldr/nls/gregorian'.

i tried to get requirejs to load i18n modules this same way you've tried (ie via a "sync" require including the locale in the full path to the i18n module, see r23279) but it didn't work.

in r23409 i settled on leaving it to the i18n plugin to only load the default locale (even if a different one was requested of dojo.getLocalization). this is where things get kind of messy - i'll try to make it as clear as i can...

if the user wants any locale other than the default locale, then they have to explicitly declare the i18n package of that locale as a dependency of their AMD module so that the bundle can be loaded asynchronously. for example if i needed the australian locale then i would need to declare it as a dependency of my module. this is because in my module, dojo.getLocalization is going to try to synchronously get the i18n package for the locale i'm asking for

define(['dojo/date/locale', 'i18n!dojo/cldr/nls/en-au/gregorian'], function (locale, au) {
    locale.getNames('months', 'wide', 'standAlone', 'en-au'); // tries to syncrhonously resolve en-au bundle
});

iirc, i was able to get the i18n working for different locales because define provided by requirejs had been wrapped so that we could keep dojo._loadedModules in the same state it would have been if dojo.require had been used. this meant that even though dojo.getLocalization would only request the default locale, since the alternative locale had been specified as a dependency (as it needs to be), the rest of dojo.getLocalization that resolved the bundle would work because it would find the en-au bundle in dojo._loadedModules.

let me know if you don't follow what i'm saying here and i'll try and simplify it. i had it working previously and now it doesn't. getting i18n right took me the longest time out of any of the changes i made relating to AMD. any ideas how to get it working again?

comment:19 Changed 9 years ago by ben hockey

there is also a problem with dojo.moduleUrl("dojo", "resources/blank.gif")).toString() in dijit._Widget being returned as 'path/to/dojo/resources/blank.gif.js'

comment:20 Changed 9 years ago by Rawld Gill

(In [23702]) refs #12200; added plugin tests to smoke tests; improved text plugin to interop with dojo/cache; fixed ws and URL resolution in i18n plugin; !strict

comment:21 in reply to:  18 ; Changed 9 years ago by Rawld Gill

Replying to neonstalwart:

due to some of the backCompat i18n changes, dojo.requireLocalization fails with requirejs. as an example:

require(['dojo/date/locale'], function (locale) { 
    locale.getNames('months', 'wide', 'standAlone'); 
});

the error reported by requirejs is "require: module name 'require/i18n!dojo/cldr/nls/en-us/gregorian' has not been loaded yet for context: _"

which is correct - the only module that requirejs knows has been requested is 'require/i18n!dojo/cldr/nls/gregorian'.

This should not be true with the dojo/i18n! plugin. dojo/date/locale depends on i18n!dojo/cldr/nls/gregorian and the dojo/i18n! plugin will ensure that locale=dojo.locale is loaded when this root bundle is requested.

I augmented the smoke tests in dojo/tests/amd to load this specific module; they appear to work on my machine.

I have not studied the requirejs i18n! plugin recently; it may not have this capability.

if the user wants any locale other than the default locale, then they have to explicitly declare the i18n package of that locale as a dependency of their AMD module so that the bundle can be loaded asynchronously.

Correct...or have a nested require. I intend to add the additional feature to the dojo i18n plugin that it can load several locales.

Note also: in the context of dojo, "default locale" means dojo.locale

iirc, i was able to get the i18n working for different locales because define provided by requirejs had been wrapped so that we could keep dojo._loadedModules in the same state it would have been if dojo.require had been used. this meant that even though dojo.getLocalization would only request the default locale, since the alternative locale had been specified as a dependency (as it needs to be), the rest of dojo.getLocalization that resolved the bundle would work because it would find the en-au bundle in dojo._loadedModules.

You lost me here: I'm unaware of anything requirejs is doing with dojo._loadedModules. dojo._loadedModules doesn't exist in the AMD version of dojo.

As to dojo.getLocalization, it should work so long as the bundles are already on board.

let me know if you don't follow what i'm saying here and i'll try and simplify it. i had it working previously and now it doesn't. getting i18n right took me the longest time out of any of the changes i made relating to AMD. any ideas how to get it working again?

I put the specific example in the smoke tests; there are working for both bdLoad and requirejs. Give them a try and tell me what you see.

Thanks, Rawld

comment:22 Changed 9 years ago by Rawld Gill

(In [23703]) refs #12200; fixes moduleUrl error when used with requirejs; added test; !strict

comment:23 in reply to:  19 Changed 9 years ago by Rawld Gill

Replying to neonstalwart:

there is also a problem with dojo.moduleUrl("dojo", "resources/blank.gif")).toString() in dijit._Widget being returned as 'path/to/dojo/resources/blank.gif.js'

Confirmed this error with requirejs (bdLoad did not show this). Fixed, added test in [23703].

comment:24 in reply to:  21 Changed 9 years ago by ben hockey

i can see now that i didn't realize that the intention is that i should be using dojo's plugins rather than the requirejs plugins. this is a perfectly reasonable expectation - when using dojo, use dojo's plugins. however, i don't think our plugins match the api currently used by requirejs http://requirejs.org/docs/plugins.html#apiload. at first i thought it was just a matter of changing the order of the parameters passed to the plugins but it seems more involved than that. i'm not sure how much the api has shifted from what the plugins were coded against. without the plugins being compatible i can't comment much about whether or not they work as expected.

Replying to rcgill:

Replying to neonstalwart:

[snip]

iirc, i was able to get the i18n working for different locales because define provided by requirejs had been wrapped so that we could keep dojo._loadedModules in the same state it would have been if dojo.require had been used. this meant that even though dojo.getLocalization would only request the default locale, since the alternative locale had been specified as a dependency (as it needs to be), the rest of dojo.getLocalization that resolved the bundle would work because it would find the en-au bundle in dojo._loadedModules.

You lost me here: I'm unaware of anything requirejs is doing with dojo._loadedModules. dojo._loadedModules doesn't exist in the AMD version of dojo.

i'll try to clarify by rephrasing and referencing dojo's code(although the point is moot if dojo's plugins work with requirejs). previously, in dojo/_base/_loader/loader.js, if a global define existed then dojo wrapped that define so that for dojo, dijit and dojox modules, a call to dojo.provide could be made whenever they were defined (see http://trac.dojotoolkit.org/browser/dojo/trunk/_base/_loader/loader.js?rev=23472#L814). this call to dojo.provide would ensure that anything loaded via define was also registered with dojo._loadedModules. requirejs didn't do anything with dojo._loadedModules but the dojo code that wrapped the global define (provided by requirejs) interacted with dojo._loadedModules. given this clarification, maybe now the comment makes more sense but if it doesn't then just ignore it since i can now see that i should be approaching this differently.

Changed 9 years ago by ben hockey

Attachment: 12200_plugins.diff added

changes to plugins

comment:25 Changed 9 years ago by ben hockey

rawld, i've attached a patch that would make our plugins work with requirejs if requirejs also accepts this patch https://github.com/jrburke/requirejs/pull/65 to allow plugins to be called during a sync call to require.

comment:26 Changed 9 years ago by Rawld Gill

(In [23715]) refs #12200; fix plugin load signature to comply with requirejs; improve caching support; !strict

comment:27 Changed 9 years ago by Rawld Gill

(In [23716]) refs #12200; improves requirejs compat; !strict

comment:28 Changed 9 years ago by ben hockey

(In [23742]) allow access to locales other than dojo.locale when using requirejs. refs #12200 !strict.

comment:29 Changed 9 years ago by Rawld Gill

Resolution: fixed
Status: reopenedclosed

comment:30 Changed 8 years ago by bill

Component: GeneralLoader
Note: See TracTickets for help on using tickets.