Uploader plugins do not work with AMD

[Adam Peller]

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, {});
        }

[Adam Peller]

Maybe you can do a require() and mixin on the result?

[Mike Wilcox]

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.

[Adam Peller]

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.

[Mike Wilcox]

In [27899]:

Converted Uploader Test to AMD to see if there were any issues. None found. Refs #14811

[Mike Wilcox]

In [27900]:

Uploader Tests - removed redundant addOnload. Refs #14811

[Mike Wilcox]

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.

[Mike Wilcox]

You are requiring Uploader first, then the plugins right? (Although I see the obvious potential for problems there being that the loader is async)

[Adam Peller]

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.

[Mike Wilcox]

In [27901]:

Adding programmatic test for uploader. Refs #14811

[Mike Wilcox]

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.

[Adam Peller]

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.

[Adam Peller]

don't use dojox.form.Uploader to reference the Uploader

[Mike Wilcox]

In [27902]:

Fixed Uploader test to use arg ref instead of global ref. (still works Adam!). Refs #14811

[Mike Wilcox]

Fixed, and I conceded that was a mistake. But it still works.

[Vicente Jiménez Aguilar]

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

[Mike Wilcox]

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.

[Adam Peller]

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.

[Mike Wilcox]

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.

[Mike Wilcox]

In [27920]:

Added workaround for prog uploader in test. Refs #14811

[Vicente Jiménez Aguilar]

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?

[Mike Wilcox]

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.

[Adam Peller]

I'd argue this should be fixed SRTL

[Mike Wilcox]

I don't know how to fix it and be backcompat though.

[Adam Peller]

@neonstalwart suggests it might be possible to use extend, or just mixin right on the object returned by require()?

[ben hockey]

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 :

[Mike Wilcox]

In [27940]:

Fixed code typo. Refs #14811

[Mike Wilcox]

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.

[ben hockey]

a short term fix might be:

Uploader.js

@@ -388 +388 @@
                declare("dojox.form.Uploader", extensions, {});
        }
 
-       return dojox.form.Uploader;
+       return function () {
+               return dojox.form.Uploader.apply(this, arguments);
+       };
 });

it's a hack but at least it works today rather than needing to wait until 2.0

[Mike Wilcox]

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.

[bill]

For the record: (In [30328]) Fixes: #14768, #15151, #16293, #14914, #14811, #11885, #14075 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.