fine-uploader

fine-uploader copied to clipboard

The qq.Promise module worked well for a while - it provides simple promise support for Fine Uploader, but this is not even remotely A+ compliant, and it should probably be removed in 6.0 in lieu of support for the native Promise object, or a polyfill. Useful error messaging will have to be part of this change so integrators are notified in an easy-to-understand way when Promise is missing from the global scope, and how they can fix this situation. The requirement of Promise support will also need to be documented in many places throughout the website, docs site, and GitHub repo.

This will also require updating internal code that uses qq.Promise and some of its non-standard methods and behaviors.

Is there anything holding this back?

Yes, available time.

I'm happy to take a stab at this one, I'll raise a PR and see what you think.

Feel free, but do note that any breaking changes must be part of 6.0.0. I don't think it's possible to do this without breaking changes. So it may be best to update the release/6.0.0 branch (merge in master), complete the task in there, and release an alpha version.

Sounds good. I'd suggest creating a 5.x branch off master for bug-fixes, and use master for 6.0. That may have less development overhead involved than maintaining a 6.0 branch and merging master, but it's not something I feel strongly about. I'll go with the release/6.0.0 branch for now.

The convention has been to follow gitflow to some degree, which keeps master the default and stable branch. All features and fixes are branched off of master. I would like to keep it this way as it is otherwise a bit confusing for users when they stumble onto master and are confused to find out that the code/README there does not match the latest stable code.

BTW, I'm quite happy to hear that you're going to attempt this fix, but I consider this to be a very challenging update, so please let me know if you have questions.

It's indeed quite challenging, I suspect we'll probably to do it in a piece-meal fashion, rather than one large PR as I suspect there will be too many changes to comfortably review. But initially I'm just going to make all the changes at once to get a rough sense of the different patterns that need changing.

I've done some investigation and I think a viable path forward may be as follows:

1. Add a requirement on native Promise being available:

   • throw an error during load if typeof Promise === "undefined"
   • Update docs and public API surface area to use native promises (i.e. only return and accept native promises, not qq.Promise, not Q.js)

2. Refactor internal code case-by-case (separate PR each) to switch from qq.Promise to native Promise.

   • Decide on whether all reject() calls use Error (and if specialised Error should be used, e.g. with .xhr property)
   • Decide on new shape for single-value resolve() values.

3. Repeat ⤴ until no remaining usage of qq.Promise

4. For any tests that check qq.Promise and Q.js, refactor to only test native Promise.

5. Delete qq.Promise

This should be achievable in a piece-meal fashion, i.e. not one large pull request. Thoughts?

This sounds like a great start, but I’m a fan of the “band-aid” approach here, and think it will be best to rip out qq.Promise in a single PR, rather than potentially scatter breaking changes and bugs over several PRs and alpha releases. This issue is complex, but still one unit of work.