Skip to content

Proposal: The future of Parse.Promise #57

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
3 of 5 tasks
andrewimm opened this issue Oct 14, 2015 · 25 comments
Closed
3 of 5 tasks

Proposal: The future of Parse.Promise #57

andrewimm opened this issue Oct 14, 2015 · 25 comments
Assignees
Labels
type:feature New feature or improvement of existing feature

Comments

@andrewimm
Copy link
Contributor

This is a public discussion around the future of Parse.Promise. As Promises are made more widely available in browsers and Node, we need to consider evolving the currently implementation to meet developer expectations and avoid fragmentation.

This document is simply a proposal, and is by no means a guarantee. We want to get developer feedback as we move the SDK forward, and we hope these discussion issues are a viable way to collect opinions on our proposals.

Parse.Promise today

Parse.Promise was originally built off of the jQuery Deferred model years ago, which means it's almost – but not quite – Promises/A+ compliant. There are a few differences, but the only one that really affects developers involves the handling of exceptions: A+ will catch an exception, while the Deferred model will propagate it upwards.

It turns out that Parse.Promise contains code to adapt to A+ semantics. Internally, we have a flag called isAPlusCompliant which is disabled by default (to maintain consistency with older versions). It's currently scoped within the module and inaccessible from userspace, but we could expose it through static methods or a CoreManager value. If we wish to move Parse.Promise forward, this would be the first step.

While this would bring Parse Promises in line with the Promises/A+ spec, they would not match the behavior of Promises found in JS. This is because the A+ spec only covers how Promises behave when resolved; it does not dictate how Promises are constructed, resolved, or rejected.
A Parse Promise can be constructed with no parameters, and can be resolved or rejected by any code, as the resolution is performed by instance methods:

let myPromise = new Parse.Promise();
// do something...
myPromise.resolve();

Native JS Promises are constructed with a callback method. It is only within this method that the resolve / reject variables are available:

let myPromise = new Promise((resolve, reject) => {
  // do something...
  resolve();
});

As such, code written for one format may be difficult to rewrite into the other format. We would like to add functionality to Parse.Promise, allowing it to be constructed using the native Promise API. Supporting both styles will maintain functionality with older codebases, but allow new developers to use Parse.Promise in a way that matches the native implementation.

Eventually, we would rewrite the internals of the SDK to match this new format. We would also modify Parse.Promise to be backed by native Promises where supported, and continue to use our polyfill when it is not available.

Proposed Timeline:

  • Before 1.7, expose the isAPlusCompliant flag externally, either through methods or a CoreManager value.
  • In 1.7.0, default to having isAPlusCompliant set to true. This behavior can be disabled for legacy purposes, but it will be the default for all future versions.
  • Additionally in 1.7.0, allow Parse.Promise to be constructed in the same way as native Promises (new Promise(callback))
  • In 1.8.0, let Parse.Promise be backed by native promises where supported; continue to polyfill methods in environments that don't include a native Promise implementation
  • In 1.8.0, rewrite the internals of the SDK to use the new Promise(callback) interface.
@andrewimm andrewimm self-assigned this Oct 14, 2015
@lacker
Copy link
Contributor

lacker commented Oct 21, 2015

I think the A+ error behavior is just better. So I support this migration

@TylerBrock
Copy link
Contributor

Agreed. It was surprising to discover that the Parse JS SDK provides a promise implementation that does not behave the same way as virtually all other promise implementations for Javascript. Perhaps mentioning that the current version is not A+ compliant in the JS Guide someplace would be helpful.

@weiway
Copy link

weiway commented Nov 2, 2015

@TylerBrock Agreed!

@RaitoBezarius
Copy link

Is the isAPlusCompliant flag exposed on the latest version of Parse JS SDK?
I am surprised to see only those keys:
["when","is","isPromisesAPlusCompliant","error","arguments","_continueWhile","as","name","caller","prototype","length"]
Whereas, the 1.6.9 seems to include the method to change them, any update on this?

I could give some help on that, because I am using async/await in my code, and it is really a pain to use wrap Parse.Promise each time.

@andrewimm
Copy link
Contributor Author

@RaitoBezarius I'm not sure I understand the question. The value of the flag can be retrieved by calling Parse.Promise.isPromisesAPlusCompliant(), and the flag can be modified by Parse.Promise.enableAPlusCompliant() / Parse.Promise.enableAPlusCompliant(). The raw boolean controlling all this is made private.

@RaitoBezarius
Copy link

@andrewimm When you do Parse.Promise.enableAPlusCompliant() in a Cloud Code function, it will fails because the Cloud Code's Parse JS SDK does not have this method.

@andrewimm
Copy link
Contributor Author

1.6.9 has not been deployed to cloud code yet. It should be up shortly.

@RaitoBezarius
Copy link

@andrewimm Do you have a timeframe? Another question while I can, is async / await doable with the Parse.Promise implementation (without A+ compliance), in your opinion? (Of course, it is transpiled to a runtime regenerator with Babel).

@andrewimm
Copy link
Contributor Author

It should be doable, since I believe Babel's transpile process works with any thenable (but I can sync up with Sebastian to double-check). If it doesn't, we'll make sure it's supported by 1.7.0.
Keep in mind, Parse Promises are A Compliant by default (not A+ Compliant). The only difference between these is how they handle exceptions. A+ catches an exception, A bubbles it up. The A+ compliance flag shouldn't affect anything related to async/await

@RaitoBezarius
Copy link

Thanks for those informations, @andrewimm ! I appreciate. :)

@cfoulston
Copy link

I vote A+, really needing some exception handling right now

@lrettig
Copy link

lrettig commented Jan 19, 2016

FYI, I noticed that enabling A+ compliance for Parse Promises breaks at least one core piece of code in ParseReact: cf. parse-community/ParseReact#161.

@JeremyPlease
Copy link

If we back Parse.Promise with native promises, then would we be able to use process.on('unhandledRejection'...?

@andrewimm
Copy link
Contributor Author

@JeremyPlease if you could guarantee that you clients all use the native promise and not a polyfill, then yes. But for all clients where we still add the custom implementation because Promise is not supported, we're not going to add an intrusive global event, so it may not be the best thing to depend on. As a developer, I find it's much easier to lint against missing handlers than to use a global catch-all.

@JeremyPlease
Copy link

@andrewimm Thanks for the quick response!
I mostly want to be able to catch unhandledRejection within Parse server (open source) cloud code. So, support within node wouldn't be an issue. I understand that this sdk is shared on browser clients as well.

Linting against missing handlers sounds like a great idea, but I don't see how this would work when returning promises that return and bubble up to a fail/error/catch handler in another file. If you have any suggestions on how linting would work for promises across files, please let me know.

I'll try to take a look at ParsePromise.js to see what kind of effort there would be use native promises when available and polyfill as needed.

@recrsn
Copy link

recrsn commented May 1, 2018

What is the status of these?

@flovilmart
Copy link
Contributor

@JeremyPlease any chance you made progress on that? as it'S 2018, perhaps the promise library should be provided by a 3rd party no?

@JeremyPlease
Copy link

@flovilmart

The Parse JS SDK is built with babel transform runtime which will polyfill Promise. So, we should be able to use Promise without any 3rd party library.

I just made a quick attempt to use native promises. Unfortunately, this breaks all kinds of tests and functionality that rely on ParsePromise resolutions to happen synchronously.

The current ParsePromise implementation executes the promise callbacks immediately. However, native promises will execute the callbacks on "nextTick" (or a similar approach in browsers). So, tests like this one will fail with native promises since it expects the .then callback of the promise to be executed immediately when p.resolve(); is called. It's likely that some actual functionality could be broken beyond just how tests are written.

My recommendation: keep ParsePromise as-is. If there is a big want/need to switch to a native promise implementation I think it should be done as a v2 release.

@flovilmart
Copy link
Contributor

rely on ParsePromise resolutions to happen synchronously.

Thats, unfortunate and I guess it would introcude a lot of changes.

I think it should be done as a v2 release.

That's reasonable and would be very nice!

@recrsn
Copy link

recrsn commented May 2, 2018

My recommendation: keep ParsePromise as-is. If there is a big want/need to switch to a native promise implementation I think it should be done as a v2 release.

This is unfortunate.

However, as promises and async/await are slowly becoming ubiquitous, it will be beneficial for all to be standards compliant. I'm not sure how the synchronous resolution goes along the async/await code.

I'm can help out in porting some of the code, if it is decided to go ahead in this direction.

@flovilmart
Copy link
Contributor

@agathver we’re not against refactoring the code to leverage native promises. As it’s been mentioned, many tests will have to be re-written and we can assist anyone wanting to tackle this task :)

@JeremyPlease
Copy link

Yeah, definitely unfortunate.

It might be good to have ParsePromise subclass Promise (class ParsePromise extends Promise) but babel doesn't support subclassing builtin classes. There are some ways to work around it though.

The biggest challenge will be modifying tests to properly handle asynchronous resolution of promises and then ensuring there isn't anything else in the Parse JS SDK or Parse Server that is unknowingly taking advantage of the current synchronous behavior.

@agathver Please feel free to take a stab at this and post any updates here 😃

@EPILock
Copy link

EPILock commented Jun 12, 2018

Team, is any work in progress on this?

Thanks

@flovilmart
Copy link
Contributor

It seems that it doesn’t really need to be done at that time as no one is rushing for an implementation. Feel free to take a stab at it

@dplewis
Copy link
Member

dplewis commented Aug 10, 2018

Closed via #620

As of 2.0 of this SDK Parse.Promise has been removed.

@dplewis dplewis closed this as completed Aug 10, 2018
@mtrezza mtrezza added type:feature New feature or improvement of existing feature and removed proposal labels Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New feature or improvement of existing feature
Projects
None yet
Development

No branches or pull requests