Skip to content

Proposal: Deprecate Backbone-style callbacks #56

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
andrewimm opened this issue Oct 14, 2015 · 8 comments
Closed

Proposal: Deprecate Backbone-style callbacks #56

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

Comments

@andrewimm
Copy link
Contributor

For legacy reasons, the SDK currently supports handling asynchronous actions through two separate methods: Backbone-style success/error callbacks, and Promises. We would like to explore the idea of deprecating the callbacks interface in favor of only using Promises, and we would like community input on this decision.

Why deprecate this piece of the SDK?
We believe that the callbacks format is a less-than-optimal approach to structuring asynchronous code. With ES6, Promises are a feature of the JS language, and we want to discourage async formats that allow developers to fall into "callback hell."
From a code perspective, these require internal methods to have a bunch of boilerplate in order to support wrapping controller methods that use Promises. Removing these callbacks would reduce complexity and remove bytes from nearly every publicly-exposed method.

Proposed deprecation path:
In 1.7.0, using Backbone callbacks will log a warning that the format is now deprecated, and will be removed in 1.8. These warning will only be in the non-minified SDK, so they will not affect production.
In 1.8.0, we completely remove callbacks support.

@oriSomething
Copy link

i favor deprecating backbone style callbacks. i even created eslint rule for it. but i think total remove should be in 2.0.0

@oriSomething
Copy link

i have a thought about after deprecation of the backbone style callbacks:
maybe allow cloud functions to return Promise instead of calls to response.success / response.error.

it's a breaking change, but it might be more opinionated and strict and may let for a fewer errors:

// success
Parse.Cloud.define('ƒ', function(request) {
  return {
    "data": {···}
  };
});

// success
Parse.Cloud.define('ƒ', function(request) {
  return Promise.resolve(···);
});

// error
Parse.Cloud.define('ƒ', function(request) {
  throw new Error(···);
});

// error
Parse.Cloud.define('ƒ', function(request) {
  return Promise.reject(···);
});

// maybe error for undefined?
Parse.Cloud.define('ƒ', function(request) {});

also in the future when async functions will be supported, it can lead to a very minimalistic API:

Parse.Cloud.define('ƒ', async function() {
  const data = await ···;
  ···
  return ···;
});

@lacker
Copy link
Contributor

lacker commented Oct 21, 2015

Yeah, I think allowing cloud function to return promises would be an improvement. Would it even be a breaking change?... we could do something tricksy like behaving differently based on whether your function has one or two args. I think there's precedent for doing stuff like that in some node libraries.

@oriSomething
Copy link

@lacker yep. my mistake. i forgot about Function.prototype.length.

i think this thing can be improve for unit testing of the server side and also for deciding when to terminate the process of the function. also this pattern can be applied to other Cloud Code APIs as Parse.Cloud.BeforeSave.

the thing i'm not sure of is the error part. for promises it's better to use a native Error object which is different from Parse.Error. i think this API can be problematic if Parse.Error would stay the same (sadly we still cannot do that class ParseError extend Error {} yet). and there are some questions about the error return value of the cloud function. Will Parse.Promise convert Error to Parse.Error?

@lacker
Copy link
Contributor

lacker commented Oct 21, 2015

It's too bad that ParseError can't just extend Error.

It seems like the promises shouldn't convert anything from one type to another. AFAICT the promise spec doesn't specify Error or anything, the promise spec just specifies that when something goes into "reject" or is thrown, it gets passed along like an error. Promises should work just as well if you just throw a string for example. I could be wrong there.

@oriSomething
Copy link

@lacker it's true, but it will be a very weird behave for JavaScript developers to throw a string instead of Error and it might be more difficult to handle in cases where you want Cloud Code and client side to share some code (specially third party). it's a conversion to throw Error like objects and i can tell that i made lots of this kind mistakes when i've used response.error with Error or even Parse.Error objects.
also throw a string doesn't return a stack, which in many cases is needed (although it might be a security risk as well).
i think converting Errors to the right needed response can be very beneficial specially for beginners

@lacker
Copy link
Contributor

lacker commented Oct 22, 2015

Hmm, well so how about:
1/ Parse promises themselves would not convert data types at all
2/ But if you use cloud code with promises, and you either throw or reject something, when it goes to the client it will get transformed into a Parse.Error no matter what

That seems logical to me.

@dplewis
Copy link
Member

dplewis commented Aug 10, 2018

Closed via #620

As of 2.0 of this SDK Backbone-style callbacks have 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

5 participants