#14811 closed defect (fixed)
Uploader plugins do not work with AMD
Reported by: | Adam Peller | Owned by: | Mike Wilcox |
---|---|---|---|
Priority: | high | Milestone: | 1.9 |
Component: | DojoX Uploader | Version: | 1.7.1 |
Keywords: | Cc: | ben hockey | |
Blocked By: | Blocking: |
Description
The declare
call in Uploader.js redefines the global dojox.form.Uploader. If the return value of require()
is used to load the Uploader rather than the global dojox.form.Uploader
the original Uploader code will be used, and the upload() method will be a no-op without any plugin code.
I don't think it's possible to redefine the module this way. Instead, some sort of factory may need to be used within the Uploader to call out to the plugin.
dojox.form.addUploaderPlugin = function(plug){ // summary: // Handle Uploader plugins. When the dojox.form.addUploaderPlugin() function is called, // the dojox.form.Uploader is recreated using the new plugin (mixin). // extensions.push(plug); declare("dojox.form.Uploader", extensions, {}); }
Attachments (1)
Change History (34)
comment:1 Changed 9 years ago by
Milestone: | tbd → 1.7.3 |
---|---|
Owner: | changed from dante to Mike Wilcox |
Priority: | undecided → high |
Status: | new → assigned |
comment:2 Changed 9 years ago by
comment:3 Changed 9 years ago by
Is this speculation or something you ran into? I'm using the Uploader in an app at work, and it works fine.
I wouldn't mind an excuse to change the plugin system though. It confuses too many people. But not sure how quick I could do that.
comment:4 Changed 9 years ago by
Something I ran into, using the new loader -- if you switch to using require() to load the Uploader instead of dojo.require and use the reference for Uploader passed in through the callback in define, instead of referencing it as dojox.form.Uploader. You could start by converting the tests in dojox/form/tests to use require instead of dojo.require.
comment:7 Changed 9 years ago by
I converted one of the two tests (the other one is pretty convoluted and needs time). It works fine. Are you doing something out of the ordinary? I couldn't really follow your description.
comment:8 Changed 9 years ago by
You are requiring Uploader first, then the plugins right? (Although I see the obvious potential for problems there being that the loader is async)
comment:9 Changed 9 years ago by
I am, though it seems to be loading in order, perhaps by chance. If the order is important, perhaps the plugin should reference the base Uploader as a dependency?
So I need to revise my description one more time, sorry. I'm not using markup. I'm instantiating the Uploader programmatically from JS. By using markup, you're still going through the global dojox.form.Uploader
reference, which has the mixin.
comment:11 Changed 9 years ago by
Working programmatic test added.
I bet you forgot to call startup() on it.
And oh, I can't pass this up... Don't get me "started" on "startup". Heh.
comment:12 Changed 9 years ago by
Huh. I never call startup() on Uploader (perhaps I should?) either way, and it hasn't been a problem. With this particular method, though, I assure you, uploader is an empty function. No startup is going to fix it. See patch, attached, which describes the usage I was going for. No dojox.* reference, but a direct reference to the module from the loader.
Changed 9 years ago by
don't use dojox.form.Uploader to reference the Uploader
comment:15 Changed 9 years ago by
I'm having some issues too with dojox.form.Uploader. The "upload" method seems to do nothing. I'm going to do some more checks later. But there a list of things that I'm doing differently:
- Used Google CDN (maybe I need a local blank.html)
- HTML5 plugin instead of Flash.
- Not used for multiple files but trying to upload on select: multiple: false, uploadOnSelect: true
comment:16 Changed 9 years ago by
vice, this is not a support forum. Please be sure you're carefully following the docs. If this is in fact a bug, open a new ticket and attach a test.
comment:17 Changed 9 years ago by
It sounds like vice may be experiencing the same problem I am. Sorry I didn't create and attach a test in the first place, but when I try the patched programmatic test you made, select a file, and click on the upload button, nothing happens.
comment:18 Changed 9 years ago by
Ok. Sorry vice, I didn't realize you had the same problem. FYI, the uploader with CDN has historically been dicey due to the external resources like the SWF and the iFrame.
Adam, I see what you were talking about now (finally!). I'll add comments in the prog test to emphasize that you have to use the full path and not the require reference.
comment:20 Changed 9 years ago by
Apologies accepted.
My intention was to bring some light detailing my particular use case because I have the same issue.
Thanks for the clarification in the example comments! This must resolve my problems. I'll try it later. It would be nice if this information is also included in the documentation.
By the way, the use of 'dojox.form.Uploader' instead of the reference is needed always or only when using CDN?
comment:21 Changed 9 years ago by
Resolution: | → wontfix |
---|---|
Status: | assigned → closed |
The Docs have been updated with the AMD issue. I've also set this to wontfix, but it will be fixed before 2.0.
Vice, the need for the global reference is an AMD issue, not a CDN issue. The CDN issue is pretty much based on loading the iframe and SWF resources, which, at least in the case of the SWF (and probably for the iframe), need to be hosted on the same domain as your application. You should find how to reference those resources in the dojoConfig in the code files.
The good news is if you can enforce HTML5 (ergo, Firefox and Safari) you don't have to worry about that. I know it's a tall order, but it's a step forward.
comment:22 Changed 9 years ago by
Milestone: | 1.7.3 → 2.0 |
---|---|
Resolution: | wontfix |
Status: | closed → reopened |
comment:24 Changed 9 years ago by
Cc: | ben hockey added |
---|
comment:26 Changed 9 years ago by
@neonstalwart suggests it might be possible to use extend, or just mixin right on the object returned by require()?
comment:27 Changed 9 years ago by
i looked into it and it isn't going to work the way i thought it would. using extend
overrides the current methods rather than "extending" them but there's got to be a better way to do these plugins. redeclaring a class every time doesn't seem like a good idea.
btw, mike there's a typo in your test page now - it doesn't load.
you have a ;
after innerHTML
when it should be a :
comment:29 Changed 9 years ago by
Thanks, bug fixed.
So, short story long:
The reason it was built the way it is was an attempt to make it easy for the dev to just declare his plugins and use the Uploader otherwise the same way. Well, good intentions gone astray, this didn't really work. I know a lot of devs use this, so it should be considered an overall success, but I get a lot of people inquiring because they get confused by the concept (which I confess follows no Dojo standards).
My new way of thinking I guess isn't that new at all, because it's much like what Dijit does. Provide a: Flash, HTML5, iFrame, and Form uploader, and also an Uploader, which is the master that mixes them all in. That way, the uninitiated can use Uploader with a bit of code bloat, and those who are more advanced can load the plugins they want and or mix them. FWIW, this is what I have planned for Video, which will have several plugin options.
As you might imagine, this is not an easy change. And it also breaks backcompat.
comment:30 Changed 9 years ago by
a short term fix might be:
-
Uploader.js
388 388 declare("dojox.form.Uploader", extensions, {}); 389 389 } 390 390 391 return dojox.form.Uploader; 391 return function () { 392 return dojox.form.Uploader.apply(this, arguments); 393 }; 392 394 });
it's a hack but at least it works today rather than needing to wait until 2.0
comment:31 Changed 9 years ago by
Component: | DojoX Form → DojoX Uploader |
---|
comment:32 Changed 8 years ago by
Milestone: | 2.0 → 1.9 |
---|---|
Resolution: | → fixed |
Status: | reopened → closed |
This is fixed. I converted Uploader to use AMD properly which required discarding the plugin approach. All functionality is now included in Uploader. plugins remain for backcompat in order to throw warnings.
Maybe you can do a require() and mixin on the result?