Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[bluebird] Restore assignability to native Promises (fixes #11027) #34805

Merged
merged 3 commits into from May 24, 2019
Merged

[bluebird] Restore assignability to native Promises (fixes #11027) #34805

merged 3 commits into from May 24, 2019

Conversation

lhecker
Copy link
Contributor

@lhecker lhecker commented Apr 17, 2019

Intention

Recently Bluebird added support for Symbol.toStringTag in petkaantonov/bluebird#1421.
This PR adds typings for this new "feature", in turn restores assignability of Bluebird to native Promise types and thus fixes the longstanding issue #11027.

This Bluebird feature has not been released yet, but I expect this to happen relatively soon.
We'll have to decide whether we wait or not for Bluebird to release the changes in the above PR.
I've inquired @petkaantonov whether we intends to release a minor version for this here.
In that case we could safely release this PR as part of seperate Bluebird 3.6 typings.
If it'll be a patch release we can merge this PR at any point in time instead.

Code

The PR is split up in 2 commits. You should review them seperately.
The first commit only renames the Bluebird import for tests, in order for us to more easily have access to the native Promise class (including in the future).

cc: @tmueller

Template

  • Use a meaningful title for the pull request. Include the name of the package modified.
  • Test the change in your own code. (Compile and run.)
  • Add or edit tests to reflect the change. (Run with npm test.)
  • Follow the advice from the readme.
  • Avoid common mistakes.
  • Run npm run lint package-name (or tsc if no tslint.json is present).
  • Provide a URL to documentation or source code which provides context for the suggested changes: Added Symbol.toStringTag support to Promise petkaantonov/bluebird#1421
  • Increase the version number in the header if appropriate.
  • If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }.

@lhecker
Copy link
Contributor Author

lhecker commented Apr 17, 2019

Bluebird has an absolute massive amount of dependents which must be upgraded for this to work.
(The TSC version upgrade is necessary because Symbol.toStringTag is incompatible otherwise.)
I'm not sure if there's any better way about this. 🤔

@typescript-bot typescript-bot added this to Waiting for Reviewers in Pull Request Status Board Apr 17, 2019
@typescript-bot typescript-bot added Popular package This PR affects a popular package (as counted by NPM download counts). Awaiting reviewer feedback labels Apr 17, 2019
@typescript-bot
Copy link
Contributor

typescript-bot commented Apr 17, 2019 •
edited

@lhecker Thank you for submitting this PR!

🔔 @tkqubo @rogierschouten @mnahkies @abreits @nfantone @Zelein @tkqubo @sodatea @d-ph @pvomhoff @arcticwaters @vesse @soywiz @theosherry @nicolashenry @nickiannone @mtraynham @micksatana @midknight41 @jasonswearingen @HiromiShikata @segayuu @philipisapain @plantain-00 @chrisleck @aliarham11 @br8h @shaharmor @palindrom615 @reconbot @funthing @iamolegga @tingwai-to @pettyalex @elvisvoer @tkrotoff @43081j @devoto13 @mgroenhoff @kataras @saeedtabrizi @scsouthw @coolreader18 @chrootsu @BendingBender @cglantschnig @joeskeen @AyaMorisawa @mastermatt @Loghorn @Raigen @samuelneff @codeanimal @drinchev @babolivier @kukoo1 @oktapodia @morpheusxaut @TitaneBoy @zjy01 @nidzov @todd @nrschultz @thomas-b @Antoine38660 @smff @truongkhanhduy95 @emmanuelgautier @dancrumb @torhal @pilagod @cschwarz @mad-mike @troy-lamerton @netux @LinusU @mlamp @trodi @arvitaly - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

@jasonswearingen
Copy link
Contributor

glad to finally see bb made compatable with native promises. props for seeing this through!

@typescript-bot typescript-bot moved this from Waiting for Reviewers to Check and Merge in Pull Request Status Board Apr 17, 2019
@typescript-bot typescript-bot added Owner Approved A listed owner of this package signed off on the pull request. Merge:Express and removed Awaiting reviewer feedback labels Apr 17, 2019
@typescript-bot
Copy link
Contributor

A definition owner has approved this PR ⭐️. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped!

@armanio123
Copy link
Contributor

@lhecker As you mentioned this has not been release yet, I'll keep from merging until you let me know otherwise.

@typescript-bot typescript-bot moved this from Check and Merge to Needs Author Attention in Pull Request Status Board May 4, 2019
@typescript-bot typescript-bot added Has Merge Conflict This PR can't be merged because it has a merge conflict. The author needs to update it. and removed Merge:Express labels May 4, 2019
@typescript-bot
Copy link
Contributor

@lhecker Unfortunately, this pull request currently has a merge conflict 😥. Please update your PR branch to be up-to-date with respect to master. Have a nice day!

@lhecker
Copy link
Contributor Author

lhecker commented May 4, 2019

To be completely honest I didn’t expect the release of this feature to take quite that long.
I‘m not sure what to do about it though.

I‘ll fix the merge conflict once Bluebird was released with this change.

@lhecker lhecker mentioned this pull request May 10, 2019
Closed
@typescript-bot typescript-bot moved this from Needs Author Attention to Check and Merge in Pull Request Status Board May 24, 2019
@typescript-bot typescript-bot added Merge:Express and removed Has Merge Conflict This PR can't be merged because it has a merge conflict. The author needs to update it. labels May 24, 2019
@lhecker
Copy link
Contributor Author

lhecker commented May 24, 2019

@armanio123 Bluebird 3.5.5 was released just now with this particular feature.
I've had to rebase this PR to resolve a minor conflict as can be seen above.

You should be able to safely merge this PR now. 🙂

@RyanCavanaugh RyanCavanaugh merged commit 3c7c5b7 into DefinitelyTyped:master May 24, 2019
@typescript-bot
Copy link
Contributor

I just published @types/db-migrate-pg@0.0.7 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/dts-generator@2.1.4 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/epilogue@0.7.2 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/fs-extra-promise@1.0.8 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/hexo-fs@0.2.8 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/inline-css@0.0.32 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/ioredis@3.2.20 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/jackrabbit@4.3.2 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/karma@3.0.3 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/karma@1.7.8 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/karma-coverage@1.1.1 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/karma-webpack@2.0.7 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/move-concurrently@1.0.2 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/node-mysql-wrapper@0.0.34 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/project-oxford@0.1.33 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/promise-ftp@1.3.2 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/promise-sftp@1.3.2 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/redlock@3.0.3 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/redlock@2.0.3 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/request-promise@4.1.44 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/restling@0.9.5 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/sequelize@4.28.2 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/sequelize@3.30.10 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/sequelize-cursor-pagination@1.2.1 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/simple-oauth2@1.6.1 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/supertest-as-promised@2.0.38 to npm.

@typescript-bot
Copy link
Contributor

I just published @types/umzug@2.2.2 to npm.

@lhecker lhecker mentioned this pull request May 24, 2019
Closed
iRON5 pushed a commit to iRON5/DefinitelyTyped that referenced this pull request Aug 13, 2019
@lhecker
[bluebird] Restore assignability to native Promises (fixes Definitely…
1747f33 
…Typed#11027) (DefinitelyTyped#34805)

* [bluebird] Rename import to Bluebird for tests

* [bluebird] Restore assignability to native Promises

* [bluebird] Upgrade TypeScript Versions of all dependents
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasonswearingen jasonswearingen jasonswearingen approved these changes

@mastermatt mastermatt mastermatt approved these changes

@plantain-00 plantain-00 plantain-00 approved these changes

Assignees
No one assigned
Labels
Owner Approved A listed owner of this package signed off on the pull request. Popular package This PR affects a popular package (as counted by NPM download counts).
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@lhecker @typescript-bot @jasonswearingen @armanio123 @mastermatt @plantain-00 @RyanCavanaugh