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

replacing bluebird with native promises #352

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cton
Copy link

@cton cton commented Nov 22, 2019

Because the latest version of this library is incompatible with the angular version 8.3.19 I have replaced bluebird with native promises as proposed in #339. @Keyang please review the changes I've made and merge if everything is working.

@coveralls
Copy link

coveralls commented Nov 22, 2019

Pull Request Test Coverage Report for Build 215

  • 18 of 18 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.05%) to 95.409%

Totals Coverage Status
Change from base Build 212: 0.05%
Covered Lines: 1322
Relevant Lines: 1354

💛 - Coveralls

@jeremyrajan
Copy link
Collaborator

Not sure, if we need this or not. @Keyang thoughts?

@Keyang
Copy link
Owner

Keyang commented Apr 8, 2020

@jeremyrajan Thanks. Bluebird was used because it was fastest promise implementation (faster than native promise) at the time, especially for node 6. If we want to drop Bluebird we probably want to drop support of node 6 as well.
Alternatively, we could expose a package level config field for promise library and allow user to override the underline Promise implementation. Like require("csvtojson").Promise=Promise.
What would you think?

@jeremyrajan
Copy link
Collaborator

jeremyrajan commented Apr 8, 2020

Ah ok! :)
I think a lot of providers have already stopped support for node 6. May be let look at 8+. LTS is at 12 now :D. And then we can get this PR to include native promises :)

@Jolg42
Copy link

Jolg42 commented Nov 5, 2020

It would be great to have this merged!

Node 10 is now in maintenance so older versions should definitely be dropped.
https://nodejs.org/en/about/releases/
Screen Shot 2020-11-05 at 15 03 52

@semla
Copy link

semla commented Dec 17, 2021

afaict rxjs can not be used with bluebird promises. Any chance of merging this?
https://stackoverflow.com/questions/65457823/merge-two-observables-in-angular-for-csv-to-json-conversion

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.

6 participants