quadstorejs / quadstore

A LevelDB-backed graph database for JS runtimes (Node.js, Deno, browsers, ...) supporting SPARQL queries and the RDF/JS interface.
https://github.com/quadstorejs/quadstore
MIT License
203 stars 14 forks source link

Support Promise interface (in addition to callback) #15

Closed dmitrizagidulin closed 7 years ago

dmitrizagidulin commented 7 years ago

hi @jacoscaz - great project! Having been a fan of levelgraph, I'm really excited to try out node-quadstore.

Do you have any plans to support a Promise-returning interface to your various async methods? Either instead of, or in addition to the old-style callback interface that they're using right now. As in, if there is no callback param, return a promise (otherwise, invoke the callback as usual).

jacoscaz commented 7 years ago

Hello @dmitrizagidulin . We appreciate your kind words! Just a word of caution: node-quadstore is still under heavy development. Expect the unexpected!

We were just wondering about support for Promise(s) within the Graph API (the RDF/JS one will follow the RDF/JS spec). Considering Bluebird's promisifyAll() we opted to focus on a basic, callback-based API to begin with. Do you think we should support Promises(s) natively?

dmitrizagidulin commented 7 years ago

Definitely, please consider supporting Promises natively. A lot of developers are allergic to having to install Bluebird just for the promisify aspect.

I'm fairly sure all of the RDF/JS async functions return Promises. Consider also that W3C recommends using Promises exclusively, in all future JS-related specs. "Going forward, all asynchronous operations of this type should be specified to instead return promises" - from https://www.w3.org/2001/tag/doc/promises-guide

If you want to save labor during initial development, consider just using Promises (and adding callbacks later, if there's demand).

jacoscaz commented 7 years ago

@dmitrizagidulin I see your point. We'll squeeze in support for Promise(s) in the Graph API in as we continue to work on node-quadstore.

Oh the other side, the low-level RDF/JS interface seems to be based upon stream.Readable(s) and events.EventEmitter(s), at least as far as I can understand. No Promise(s).

dmitrizagidulin commented 7 years ago

Yeah, you're right, the store/sink stuff is Stream based instead.

(Thanks!!)

jacoscaz commented 7 years ago

@dmitrizagidulin I'm working on promises here https://github.com/jacoscaz/node-quadstore/tree/promises/ . Shouldn't take long.

jacoscaz commented 7 years ago

Closing this one. Bugs and enhancements should go into dedicated issues.

dmitrizagidulin commented 7 years ago

Awesome, thank you!