#15616 closed defect (fixed)
make dojo work with other AMD loaders
Reported by: | James Burke | Owned by: | Colin Snover |
---|---|---|---|
Priority: | blocker | Milestone: | 1.9 |
Component: | General | Version: | 1.8.0b1 |
Keywords: | Cc: | skaegi, dylan, James Thomas, Richard Backhouse, Rawld Gill | |
Blocked By: | #15866 | Blocking: |
Description
Trying dojo 1.8b1, and dojo/ready calls require.on(), where ideally there is a check for require.on done before calling it.
This allows other loaders, like requirejs, which do not place an "on" on require to load the file. I did not do a search, but if there are other uses of require.on, they should also get wrapped in a check.
Attachments (1)
Change History (55)
comment:1 Changed 9 years ago by
Owner: | set to Rawld Gill |
---|---|
Status: | new → assigned |
comment:2 Changed 9 years ago by
Resolution: | → duplicate |
---|---|
Status: | assigned → closed |
This is a dup of #14587.
comment:3 Changed 9 years ago by
Cc: | skaegi added |
---|---|
Resolution: | duplicate |
Status: | closed → reopened |
Suggesting we reopen since #14587 is closed and there is an interop issue here. We could rework the dijit/* calls to avoid loading dojo/ready or tackle the require API issue in dojo/ready directly. @jburke, perhaps we need common API support to make this happen?
I'd prefer we have a solution before 2.0 so that everyone on the Dojo 1.x line can use alternate loaders, including requirejs and zazl
comment:4 Changed 9 years ago by
For Eclipse Orion we're currently using a tweaked 1.6.1 with RequireJS and would _really_ like to move up to 1.8 but are blocked at the moment. We'd be happy to help out here but just want to make sure this is something the committers will work with us on.
comment:5 Changed 9 years ago by
My first thought: that code is set up to want to know when all scripts have loaded and dom is ready. However, that should only be needed for indirect dependencies. I wonder if it is OK for non-dojo loaders to just have that code fire ready when the dom is ready, and just have the caveat for people using other loaders, make sure you have direct dependencies for things that need to be loaded.
Maybe that means the auto require stuff for the parser could not be supported in other loaders?
comment:6 Changed 9 years ago by
as someone who managed to get requirejs to work with dojo 1.6 at some point early on - i'd guess that you've got a few issues to work through to get dojo to work with any loader besides dojo's loader.
the main places that i've had issues with previously are:
- dojo/i18n
- dojo/text
- the priority queue to implement dojo/ready
has anyone looked past this issue to see what else will be a problem?
also, what is missing/broken with dojo's loader that you would need to use another? maybe we should also be looking to address that?
another point... in dojo, you could use alias
to replace the dojo/ready module with another implementation. if the other loaders offer a similar option (i think it might work to use paths
) then you could replace that module with whatever implementation you want. i'm not saying that should be the conclusion of this... but it would move you forward to find what other issues might exist.
comment:7 Changed 9 years ago by
-- also, what is missing/broken with dojo's loader that you would need to use another? maybe we -- should also be looking to address that?
I thought one of the major selling points of AMD was interoperabilty. This does not help with winning over the AMD naysayers (and there are plenty). Its not so much what is missing but more what keeps getting added as proprietary API's.
comment:8 Changed 9 years ago by
i think proprietary APIs are the only way to innovate in a space that is still maturing. i used to lobby hard to have dojo be completely compatible with requirejs but after seeing things like packageMap
that have come from proprietary APIs in dojo i came to the conclusion that complete interoperability was potentially a hindrance to moving AMD forward. by definition, innovation is doing something someone else isn't doing already.
i'll strongly defend dojo's proprietary API's for as long as they add value to users of dojo - this includes APIs that don't make sense outside of dojo but are needed in order to provide backwards compatibility. even requirejs has proprietary APIs not in other loaders (eg shim). would you ask james to remove this feature from requirejs because dojo doesn't have it? i wouldn't think you should - the feature is valuable to requirejs users and helps move AMD forward when library authors are reluctant to accommodate their own users who have adopted AMD.
comment:9 follow-up: 11 Changed 9 years ago by
Cc: | dylan added |
---|
You can provide or use proprietary APIs, but when you do so, you're not using AMD. If these proprietary APIs are used in core classes, that's going to destroy interop, but it prevents innovation also. Take a look at Zazl. It provides tremendous value. It does something quite different from Dojo's loader, and it's a loader that may only be appropriate for certain use cases.. Maqetta swaps it in to do on-the-fly previews of user applications where a static dojo build doesn't make sense.
Interop with other libraries is key to Dojo's mission. We need to move away from a model where everyone has to adopt Dojo exclusively. We need to have smaller barriers for newcomers who wish to use Dojo who may already be using another library. There are things we will do well, and there are things other toolkits will do well. The loader choice should be arbitrary, and loaders can compete on their own merits.
comment:10 Changed 9 years ago by
I would also separate extensions that deal with resolving string IDs to ones that add new methods on require/define. The string resolution ones are normally workable, where assuming proprietary require APIs (at least without doing a capability check) is more hazardous.
comment:11 Changed 9 years ago by
Replying to peller:
The loader choice should be arbitrary, and loaders can compete on their own merits.
i completely agree and my suggestion for 2.0 is that dojo should separate out it's loader and builder into their own repo(s) in order to even further emphasize that the toolkit is written in AMD and not in "dojo loader" and also, if the motivation is there for someone to promote it, the loader can compete on its own merits outside of dojo. unfortunately, until 2.0, we have to provide backwards compatibility with things that existed before AMD and so interoperability gets 2nd place.
that said, that doesn't necessarily preclude a fix for this specific issue. my first comment in this thread was mostly for the sake of seeing if anyone has looked into further issues - my other comments seem to have become the focus.
comment:12 follow-ups: 13 18 Changed 9 years ago by
I'm open to neonstalwart's suggestion of just mapping dojo/ready to another module as an acceptable alternative for now. I was hoping that just a conditional test for require.on was enough. Although I suppose there needs to be an else branch to at least trigger the callback when domready is available.
I'm also having other trouble building dojo though with r.js, there is something about knowing the right modules to include for a browser build that I cannot determine from executing the has plugin. I did a patch in #15444 to allow me to pass config to dojo/has, but that does not seem to be enough. More info in that other ticket, but I appreciate any pointers. I'm usually in the dojo-meeting irc channel when i'm on IRC.
comment:13 Changed 9 years ago by
Replying to jburke:
I'm also having other trouble building dojo though with r.js, there is something about knowing the right modules to include for a browser build that I cannot determine from executing the has plugin. I did a patch in #15444 to allow me to pass config to dojo/has, but that does not seem to be enough. More info in that other ticket, but I appreciate any pointers. I'm usually in the dojo-meeting irc channel when i'm on IRC.
you might want to look at http://bugs.dojotoolkit.org/browser/dojo/util/trunk/build/plugins for the code that dojo actually executes during a build - it takes a different approach to r.js for plugins.
comment:14 Changed 9 years ago by
harrisreynolds, if you're able to reproduce the problem on trunk, please open a new ticket. (Just noticed that Bill's fixes went into trunk, not 1.8) An example would be especially helpful.
comment:15 Changed 9 years ago by
Summary: | require.on does not exist in some non-dojo loaders → make dojo work with other AMD loaders |
---|
I changed the summary to be hopefully more descriptive.
And yes I think it is the stuff I mentioned in #14587, i.e. all the modules that are currently using dojo/ready.
I'd be happy to review patches but I just don't have time to work on this myself.
Hopefully you could get most of the way there by making modules use dojo/domReady! when the dojo loader is in async mode (or when we are using a different AMD loader). Something like:
define([ ... has("async") ? "dojo/domReady!" : "dojo/ready" ], function(...
(Or alternately have a flag to dojo/domReady! to return instantly when the loader is in sync mode, something like dojo/domReady!asyncOnly)
BTW, there's no issue with the parser's auto-require feature, it was just an issue with the optional parseOnLoad:true djConfig parameter. There's also a problem with dojox.layout.ContentPane?'s executeScripts feature, since it's hard to check what exact modules the embedded script may have tried to require. I don't think anyone cares though if those two features work with other AMD loaders.
comment:17 Changed 9 years ago by
@neonstalwart: ah, thanks, so a different plugin is used for building. I'll see if I can adapt that to work in the requirejs loader. It may be a bit before I can fully try it out. I have a talk deadline and some family visiting.
@rcgill: on the "know when all modules complete". It is always worth bringing it up on amd-implement to see if others are interested in standardizing something to indicate "modules are loading", "all modules finished loading/executing".
I had something in requirejs 1.0 that the domReady plugin could attach to, but removed it in 2.0 as part of trying to consolidate the code, and I figured I would wait to see if people really needed, and if so, look at adding it back then. Particularly since when we talked about it before, ideally this is not needed -- use direct dependencies only. It is also hard for me to see how an ECMAScript module approach would try to allow for it, at least by default without making a custom module loader instance.
comment:18 Changed 9 years ago by
Milestone: | tbd → 1.8.1 |
---|---|
Owner: | Rawld Gill deleted |
Status: | reopened → assigned |
As per the meeting we won't hold up 1.8 for this, but hopefully put into a 1.8.1.
Erasing rawld's name though since there's no loader bug, just limitations in the dojo code.
comment:19 Changed 9 years ago by
Cc: | James Thomas added |
---|
comment:20 Changed 9 years ago by
Cc: | Richard Backhouse added |
---|
comment:21 Changed 9 years ago by
Cc: | Rawld Gill added |
---|
Rawld, as a hack to quickly support this pre-2.0, what do you think about modifying dojo/ready so that when used with other AMD loaders, it runs the priority queue on the DOMReady event? I.E. don't wait for all modules to load. It could be confusing, but could also be expedient.
Another question: I know that dojo/domReady! doesn't work as a dependency when running in sync mode, but what if it's used by itself, i.e. within a module (inside of the define() factory function) there's some code like:
require(["dojo/domReady!"], function(){ // guaranteed delayed until DOMReady even when djConfig="async:false"? })
comment:22 Changed 9 years ago by
PS: not sure my first question is pertinent, since the ready() callbacks could execute in any order if the domReady! happened before the ready() calls. I mean that the order of execution would be dependent on which modules happened to load first, which is somewhat random.
comment:23 Changed 9 years ago by
I'd like to see "dojo/domReady!" deprecated. In our environment we optimize out every other form of plugin so that modules get evaluated immediately (i.e., define(…) immediately resolves dependencies and invokes the callback function. Everything gets sent down in a single script with all the define statements in the correct order.
Abuse of AMD plugins to register for an event is the only case where we wouldn't be able to do this, although I've yet to come across any use of dojo/domReady! that couldn't be converted to the non-plugin form.
comment:24 Changed 9 years ago by
I suppose that's a good idea, assuming that there was a domReady() (or ready()) function that queued callbacks to be run in the order they were registered. That way, a ready() callback in a dependency would be guaranteed to be executed before the ready() callback in the module that depends on it.
comment:25 Changed 9 years ago by
Blocked By: | 15866 added |
---|
comment:5 Changed 9 years ago by
skaegi etc. - can you pull the code from trunk and see if this change is enough to make other AMD loaders work for you?
comment:6 Changed 9 years ago by
PS: my parser example should have been:
require([..., "dojo/parser"], function(..., parser){ require(["dojo/domReady!"], function(){ parser.parse().then(...); }); }
comment:8 Changed 9 years ago by
Replying to bill:
PS: my parser example should have been:
require([..., "dojo/parser"], function(..., parser){ require(["dojo/domReady!"], function(){ parser.parse().then(...); }); }
Please avoid using plugins when there is no reason to do so. Our AMD loader doesn't support plugins because we optimize away all use of plugins on the server. Instead:
require([..., "dojo/parser", "dojo/domReady"], function(..., parser, domReady){ domReady(function(){ parser.parse().then(...); }); }
Which is equivalent, more efficient, and works with loaders that only support server-side/build-time plugins.
comment:9 follow-up: 11 Changed 9 years ago by
I didn't realize that domReady() could be used as a function, rather than a plugin, but looks like you are right. I'll revert my original checkin and do it that way instead.
comment:11 Changed 9 years ago by
Milestone: | 1.8.1 → 1.9 |
---|
Note that [29511] broke _testCommon.js: URL's like dijit/tests/layout/test_TabContainer.html?theme=tundra don't display correctly anymore.
The changes above, plus the discussion to support dojo/domReady! for use with widgets seem a bit dangerous to roll into a point release, and contrary to the idea of point releases, so I'm not going to backport these changesets. If someone else wants to take responsibility for backporting and supporting the feature to 1.8, they can.
comment:12 follow-ups: 13 18 Changed 8 years ago by
Has there been any resolution to this issue? I'd like to use the Dojo charting components within a RequireJS application and I'm getting the about how the load/require object "has no method 'on'".
If there isn't a resolution, is there a workaround? A tweak I can make to either require or dojo to get this use case to work?
Thanks,
-- Harris
comment:13 Changed 8 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
AFAIK it's all working. I asked @skaegi to try it 4 months ago (see above comments) but didn't get a response, so I'll just assume it's working.
comment:14 Changed 8 years ago by
harrisreynolds, if you're using the 1.8 release and you still have an issue, please open a new ticket. An example would be especially helpful.
comment:15 Changed 8 years ago by
The fix is only in trunk though, so you should try against the trunk code.
comment:16 Changed 8 years ago by
With the latest code in trunk require.on still throws an error for me when used with RequireJS.
dojo\dojo - at revision: 30255 dojo\dijit - at revision: 30255 At revision: 30255
TypeError?: require.on is not a function http://localhost:50001/js/vendor/dijit/_Widget.js?bust=v001 Line 130
onMouseDown() http://localhost:50001/js/vendor/dijit/_Widget.js?bust=v001 (line 130)
comment:17 Changed 8 years ago by
There's no require.on() call in _Widget.js. Are you running on a build or something? Or maybe that's just the debugger giving confusing messages.
Anyway, the one require.on() call I see in the current code, besides strangely in dojox/app, is in dojo/ready, which isn't meant to be portable to other AMD loaders.
I'm guessing you aren't using dojox/app?
But also, you shouldn't have any calls to dojo.ready() either, because the code in trunk was changed to use dojo/domReady! exclusively. So not sure what's going on with you getting this error.
comment:18 Changed 8 years ago by
Replying to harrisreynolds:
is there a workaround? A tweak I can make to either require or dojo to get this use case to work?
Thanks,
-- Harris
Harris, I've implemented my own AMD loader, and I had to use the following stub:
require.on = function ready(unused, callback) { callback(); };
comment:19 Changed 8 years ago by
You are right. Firebug was giving a bad stacktrace. Webkit accurately points the finger at dojo/ready. I was trying to use dojo/parser's .parse() method to render bordercontainer/contentpane from existing markup which leads to this error.
This is off topic but is there some other way to use bordercontainer from existing markup?
comment:20 Changed 8 years ago by
Oh, for the parser thing, just stop setting parseOnLoad: true. Instead do:
require([... "dijit/layout/BorderContainer", "dojo/parser", "dojo/domReady!], function(..., BorderContainer, parser){ parser.parse(); });
comment:21 Changed 8 years ago by
I do have parseOnLoad set to false. I added domReady! but still get the require.on error. If there is somewhere else better to discuss this please let me know. I created a post on stackoverflow. http://stackoverflow.com/questions/14074313/using-dijit-without-dojo-parse
comment:22 Changed 8 years ago by
OK, then you need to use the debugger and figure out what's going on. AFAICT the ready() call only fires if parseOnLoad is set to true. The code is simple:
if(config.parseOnLoad){ ready(100, parser, "parse"); }
comment:23 Changed 8 years ago by
FYI, I'm working on a branch of dojo and dijit that's actually (successfully) running against a different loader, see https://github.com/wkeese/dojo/tree/2.0 and https://github.com/wkeese/dijit/tree/2.0.
comment:24 Changed 8 years ago by
bill: neat! I am also curious to try the r.js optimizer as I think from last time I tried a couple months ago that was difficult to do given way loader plugins are used and loaded in the dojo builder. Not surprising as that has been the weakest part of the AMD APIs across AMD implementations, but would be good to sort out, at least for a 2.0 if not in 1.x.
I have been meaning to dive into it more to give more meaningful, targeted feedback, but the day job has been eating more time lately.
comment:27 Changed 8 years ago by
Priority: | undecided → blocker |
---|---|
Resolution: | fixed |
Status: | closed → reopened |
dojo/ready
needs to fall back to a reasonable behaviour for other spec-compliant AMD loaders that do not support the Dojo 1 loader’s custom micro event API rather than just blindly assuming it is there and crashing. Anything that uses dojo/fx
or dojo/parser
does not work right now.
comment:28 Changed 8 years ago by
Owner: | set to Colin Snover |
---|---|
Resolution: | → fixed |
Status: | reopened → closed |
In [31368]:
comment:29 Changed 8 years ago by
Most of the time, ready
is used to continue to support discouraged coding methods. In dojo/fx
, it is used in the sync path to load an extra fx module. In the parser, it’s used to allow parseOnLoad
. Most of the rest of the time it’s just used as a DOM-ready execution queue. At worst, with the patch in [31368], we are no longer any more broken in other AMD loaders than we already were. It does not affect users that use the Dojo loader in any way since it has the methods that are tested for.
Same as #14587?