Skip to content
This repository has been archived by the owner on Nov 20, 2018. It is now read-only.

Add AMD support #690

Closed
wants to merge 5 commits into from
Closed

Add AMD support #690

wants to merge 5 commits into from

Conversation

akre54
Copy link
Contributor

@akre54 akre54 commented Feb 22, 2013

Re-opening accidentally closed #616 for 3.4-WIP

(fyi, you dont need open pull requests to merge in branches, just rebase or cherry-pick the commits off the pull branch and reference the pull in a commit -- i.e. "add AMD support closes #616" and github will handle the rest)

@rnicholus
Copy link
Member

@akre54 Regarding your note on PRs I'm aware. I usually do that anyway instead of simply merging the changes in from the GH UI, but it's nice to have an open PR to remind me about the request.

@akre54
Copy link
Contributor Author

akre54 commented Feb 22, 2013

@rnicholus cool, I thought that might be the case. I'm always amazed how useful github's tools are :)

@rnicholus
Copy link
Member

I plan on looking into this shortly.

@rnicholus
Copy link
Member

A problem I'm running into here: header.js and footer.js have syntax errors. I realize that this is not an issue if all files are combined, but I do not combine these files in my test environment, nor do I combine them on the demo/home page. It's important that I am able to work directly with the individual files in these two environments.

@akre54 What are your thoughts on resolving this?

@rnicholus rnicholus closed this Apr 1, 2013
@rnicholus rnicholus mentioned this pull request Apr 1, 2013
@rnicholus
Copy link
Member

This was closed again, automatically by Github, when I released 3.4 and deleted the 3.4-IP branch. I created a new issue, #776, to track future work on this.

@akre54
Copy link
Contributor Author

akre54 commented Apr 1, 2013

I missed this, sorry. It worked fine for me when I checked the first time,
but if I remember correctly you can't span a closure across multiple files.

I think the main thing I could suggest is that multiple files may not be
the best distribution mechanism for this project, since the individual
pieces, while modular, are still quite interdependent. And given that the
current design exposes everything into the global scope, a closure would
help eliminate some scoping issues too.
On Mar 31, 2013 11:10 PM, "Ray Nicholus" notifications@github.com wrote:

This was closed again, automatically by Github, when I released 3.4 and
deleted the 3.4-IP branch. I created a new issue, #776#776,
to track future work on this.


Reply to this email directly or view it on GitHubhttps://github.com//pull/690#issuecomment-15702784
.

@rnicholus
Copy link
Member

I actually don't intend to distribute multiple files. There is a downloads section where I provide up-to-date code combined into one file. I separate the library into files in the source tree to make it easier for me to test and maintain them. It's quite difficult to work with a 4200+ line javascript file. I carved the giant javascript file into modules starting with version 3.0, which has made it much easier to read and understand the library, IMHO.

@rnicholus
Copy link
Member

I still want to keep this on my radar, in case I come up with a way to make this happen. I've created a feature case: #789.

@neilchaudhuri
Copy link

Did AMD support ever happen for FineUploader?

If not, is it possible to mix RequireJS loading and FineUploader as a "one-off"? How have people made the two coexist?

@rnicholus
Copy link
Member

Is there any reason you cannot access the qq global object/namespace created by Fine Uploader?

@mrjoelkemp
Copy link
Contributor

We're using the old version of qq (fileuploader) and trying to slowly migrate to FineUploader. Since both export qq to the global namespace, any pages that have both versions loaded will throw errors (as both objects get merged together).

Having a UMD wrapping would allow us to pull in the new version without conflicts.

cc @mikesherov

@rnicholus
Copy link
Member

That's going to be quite a migration effort. A lot has changed since valums/file-uploader. That even predates my work on this project.

Are you actually loading both versions on the same page to construct two separate instances of the uploader?

@mrjoelkemp
Copy link
Contributor

Yes. Older features use fileuploader. Newer ones are starting to use Fineuploader.

The migration isn't very painful. The only real changes are to the options object passed to the uploader constructor.

Without amd support, we've had to manually change the export of the new version to something other than qq until we migrate the older features to the new uploader. This is not great as it breaks upgrade paths and forces the backend to adapt to new get params (used to be qqfilename, but is now qqqfilename). Yes, we could carefully find and replace, but there were almost 1000 instances of qq.

@feltnerm
Copy link
Contributor

feltnerm commented Nov 7, 2014

I did some experiments on AMD support for FineUploader a few weeks back.. The most effective solution I found was to wrap the concatenated Fine Uploader sources in a UMD wrapper using grunt-umd. It was pretty easy.

Not sure if this is something we would want to offer by default in the distributed sources, or just allow users to do on their own if they so choose.

@rnicholus
Copy link
Member

If you're making use of templating in both, then you'll have a bit more work since that stuff changed quite a bit in 4.0.

@rnicholus
Copy link
Member

@mrjoelkemp Let us know if @feltnerm's suggestion works for you.

@mrjoelkemp
Copy link
Contributor

@rnicholus Will do. Thanks @feltnerm.

@rnicholus
Copy link
Member

This is in the process of being implemented for 5.8.0 in #1562.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants