Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

1.3 makes $q's deferred.promise methods non-enumerable, breaks angular.extend #9933

Closed
SimplGy opened this issue Nov 6, 2014 · 6 comments
Closed

Comments

@SimplGy
Copy link

SimplGy commented Nov 6, 2014

Recently upgraded from 1.2 to 1.3.1.

In 1.3, promise methods are on the prototype. They didn't used to be in 1.2.

In 1.2, This plunker shows I can extend with a promise.

In 1.3, This plunker shows I can no longer extend with a promise.

This means those methods are not enumerable any more. angular.extend only moves enumerable properties, which is sensible.

Extending an object with a promise may seem like an odd idea, but a modal is a neat use case:

Modal = function(contents) {
  // ...
  this.deferred = $q.defer();
  angular.extend(this, this.deferred.promise);
  return this;
}
warningModal = new Modal('Are you sure?');
warningModal.then(continueWithDeleting);
@SimplGy SimplGy changed the title 1.3 makes $q's deferred.promise methods non-enumerable 1.3 makes $q's deferred.promise methods non-enumerable, breaks angular.extend Nov 6, 2014
@caitp
Copy link
Contributor

caitp commented Nov 6, 2014

yeah this was originally documented as a breaking change, but that version didnt land. however, theres no intention to change this

@lgalfaso
Copy link
Contributor

lgalfaso commented Nov 6, 2014

I understand that this is a breaking change, but this will not change. The
reasons being that promises are not designed (and were never designed) to
be copied around. Not by Angular, Promise A+ nor any of the popular
implementations

Note: In your code you are not trying to extend a promise (this is adding
methods to a promise that this is still possible), you are trying to make
something else extend a promise (and this is what will not be supported)

@SimplGy
Copy link
Author

SimplGy commented Nov 6, 2014

I appreciate the comments, Lucas and Caitlin.

I think it's a sensible change, and don't expect it to roll back. There are other ways for us to have UI components provide a promise API.

You folks are doing what you think is best for the framework, and the framework is great. Lots of people rely on the hard work you guys do. Sometimes there are very large code bases depending on it, and we look over the change logs very carefully when deciding when to upgrade.

@caitp
Copy link
Contributor

caitp commented Nov 6, 2014

@lgalfaso want to amend the changelog to accomodate this though? my original bluebird port patch did include the breaking change notice, but jeff's was missing it.

@lgalfaso
Copy link
Contributor

lgalfaso commented Nov 6, 2014

Will post a patch to fix the changelog in a few

@lgalfaso
Copy link
Contributor

lgalfaso commented Nov 6, 2014

#9947 should take care of documenting the breaking change from #8300. Closing this for now

@lgalfaso lgalfaso closed this as completed Nov 6, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants