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

Exposing Bluebird in the API is unrealistic #95

Open
lookfirst opened this issue Aug 9, 2017 · 5 comments
Open

Exposing Bluebird in the API is unrealistic #95

lookfirst opened this issue Aug 9, 2017 · 5 comments
Assignees
Labels
enhancement This will add functionality to the Public API of the library and will result in a minor release
Milestone

Comments

@lookfirst
Copy link
Contributor

There are other closed issues around this, but it still seems to be an open sore point even in 8.x, so let me provide a bit more context.

The fact that you return Bluebird in the interfaces is a real bummer. I don't care what Promise implementation you use under the covers, but I don't want to pollute the rest of my code with BB references.

Your connection example code has issues because connecting to mongo is async... you have to wait for the promise to fulfill before trying to do things with the database...

For example, I have to do this convoluted thing...

	public connect(): Promise<Core> {
		this.dbInstance = new DB(Constants.getMongoURL());

		return new Promise<Core>((resolve, reject) => {
			return this.dbInstance.connect().then((result) => {
				resolve(result);
			}, (err) => {
				reject(err);
			})
		});
	}

Or even this...

	public exists(): Promise<Contractor> {
		return new Promise<Contractor>((resolve, reject) => {
			return this.db.ContractorModel.findOne({ '_id': this.id } ).then((contractor) => {
				resolve(contractor);
			}, (err) => {
				reject(err);
			});
		});
	}

I'm sure I can do a bunch with my own generic helpers to clean this up, but I'd rather your code just help me out. =) Other than this, I'm really happy with your project. So much more sane than the alternatives. Please consider this positive feedback!

@notheotherben
Copy link
Member

Hi @lookfirst,

Thanks for opening this issue. As a matter of fact, the reason 8.x is still in alpha is that I do intend on switching to native Promises before it is released (I simply haven't had the chance to sit down and do the refactor yet).

As you touched on, it is a long running pain point with the library and one which was borne of the early days of Promise implementations where Bluebird really was the best option available. Since then other libraries have matured, Promises have gained further adoption and more people are expecting libraries to provide native Promises out of the box. While I had hoped Bluebird would remain drop-in-compatible on a TypeScript level, that has proven to not be the case.

Rest assured, I have every intention of improving this aspect of the experience and will keep you apprised of my progress on that front.

I'm glad that, with the exception of this, you're enjoying the library and please do open issues if there are any other aspects which rub you the wrong way.

Regards,
Benjamin

@notheotherben notheotherben self-assigned this Aug 9, 2017
@notheotherben notheotherben added this to the Version 8 milestone Aug 9, 2017
@notheotherben notheotherben added the enhancement This will add functionality to the Public API of the library and will result in a minor release label Aug 9, 2017
@notheotherben
Copy link
Member

I've just released v8.0.0-alpha.3 which refactors Iridium to use Native Promises instead of Bluebird. Please can you take a look and let me know if it works as you expect it to, as well as if you encounter any other need for "hackery" to make things work well.

notheotherben added a commit that referenced this issue Aug 14, 2017
@lookfirst
Copy link
Contributor Author

I just tried and yarn is still only grabbing a2 for some reason.

@notheotherben
Copy link
Member

Sorry about that, I published it under the wrong tag (@alpha rather than @next). I've added it to the iridium@next tag now and it should work. Please let me know if you run into any further issues.

@lookfirst
Copy link
Contributor Author

Thanks man. It cleaned up a bunch of code for me. =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This will add functionality to the Public API of the library and will result in a minor release
Projects
None yet
Development

No branches or pull requests

2 participants