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.

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

Removed dependence on "Bluebird". #16185

Closed
wants to merge 5 commits into from
Closed

Removed dependence on "Bluebird". #16185

wants to merge 5 commits into from

Conversation

0x6368656174
Copy link
Contributor

@0x6368656174 0x6368656174 commented Apr 28, 2017

Please fill in this 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.)
  • Follow the advice from the readme.
  • Avoid common mistakes.
  • Run npm run lint package-name (or tsc if no tslint.json is present).

Select one of these and delete the others:

If changing an existing definition:

  • Provide a URL to documentation or source code which provides context for the suggested changes: <>
  • Increase the version number in the header if appropriate.
  • If you are making substantial changes, consider adding a tslint.json containing { "extends": "dslint/dt.json" }.

@dt-bot
Copy link
Member

dt-bot commented Apr 28, 2017

types/sequelize/index.d.ts

to authors (@samuelneff @CodeAnimal @drinchev @babolivier). Could you review this PR?
👍 or 👎?

Checklist

  • pass the Travis CI test?

types/sequelize/v3/index.d.ts

to authors (@samuelneff @CodeAnimal @drinchev). Could you review this PR?
👍 or 👎?

Checklist

  • pass the Travis CI test?

@0x6368656174 0x6368656174 changed the title Change "transaction" method arguments from "Promise" to "PromiseLike" in "sequelize" Removed dependence on "Bluebird". "Promise" replaced by "PromiseLike" in "sequelize" Apr 28, 2017
@drinchev
Copy link
Contributor

This looked like a good idea. Can you mention why you closed this. Did you encounter any problems?

@0x6368656174
Copy link
Contributor Author

@drinchev PromiseLike is not compatible with native Promise=( You can only remove the dependence on Bluebird, so that the sequelize was compatible with native Promise.

@0x6368656174 0x6368656174 reopened this Apr 28, 2017
@0x6368656174 0x6368656174 reopened this Apr 28, 2017
@0x6368656174 0x6368656174 changed the title Removed dependence on "Bluebird". "Promise" replaced by "PromiseLike" in "sequelize" Removed dependence on "Bluebird". Apr 28, 2017
@drinchev
Copy link
Contributor

Got it. Btw keep in mind that sequelize uses Bluebird as promise implementation. That's why I put it as a dependency in first place.

If you remove bluebird dependency you will not have .tap, .reduce, etc.

@0x6368656174
Copy link
Contributor Author

0x6368656174 commented Apr 28, 2017

@drinchev The problem is that if I'm using a global bluebird because of performance:

import * as Bluebird from "bluebird";
global.Promise = Bluebird;

But I do not use methods other than native ones. But because of the dependency of the secularize on the bluebird, the project is not going to. The Bluebird 3.0 is not compatible with the native Promise in the typescript-2.0 (#11027 (comment)).

It turns out that, due to secularise, you have to import bliebird everywhere.

This is a bluebird error, not sequelize!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants