Skip to content
This repository was archived by the owner on Jun 26, 2020. It is now read-only.

add getPromise call to access removed then method of Deferred #1130

Merged
merged 1 commit into from
Aug 28, 2018

Conversation

noahlemen
Copy link
Contributor

@noahlemen noahlemen commented Aug 28, 2018

The then method of Deferred was removed (see D9308668 internally) -- the RelayMutationRequest (extending Deferred) request now must call getPromise() to access the underlying Promise methods

note that this change to fbjs/Deferred has not yet been synced to Github, so this only occurs internally -- error was reported in T33316897

The `then` method of `Deferred` was removed (see D9308668 internally) -- the `RelayMutationRequest` (extending `Deferred`) `request` now must call `getPromise()` to access the underlying Promise methods

note that this change to fbjs/Deferred has not yet been synced to Github, so this only occurs internally -- was reported in T33316897
@bvaughn
Copy link
Contributor

bvaughn commented Aug 28, 2018

note that this change to fbjs/Deferred has not yet been synced to Github

I think you're saying that removing .then() hasn't yet been synced, but the getPromise() API has already been release (and so is safe to switch to)?

@noahlemen
Copy link
Contributor Author

noahlemen commented Aug 28, 2018

yup! the getPromise method was not added, it is already in the class. just that then has not yet been removed.

https://github.com/facebook/fbjs/blob/cfd39964ba4b9ce351c314ed512e654ffa9cad26/packages/fbjs/src/core/Deferred.js#L35-L37

@bvaughn
Copy link
Contributor

bvaughn commented Aug 28, 2018

Cool. Just checked and it looks like you're right that the earliest version of fbjs we've pinned to does include getPromise() so... 👍

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a safe change to me 👍

@bvaughn bvaughn merged commit 7205587 into facebook:master Aug 28, 2018
@noahlemen noahlemen deleted the deferred-fix branch August 28, 2018 20:36
@noahlemen
Copy link
Contributor Author

thanks @bvaughn!

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.

3 participants