Skip to content

Conversation

@martinheidegger
Copy link
Contributor

This PR adds the option to pass-in open, destroy, read, write, writev, and final as async function instead of a callback.

@martinheidegger martinheidegger force-pushed the async-options branch 5 times, most recently from f28abf7 to e7b48f2 Compare November 26, 2020 12:54
@phated
Copy link

phated commented Dec 7, 2020

This gets really tricky if someone makes a function that takes a callback and returns a promise. We've struggled with this same issue in async-done and I don't think the complexity of the API is worth it. It almost seems like it should just be a wrapper of some sort.

@mafintosh
Copy link
Owner

I’m leaning towards just making a major that impls the internal _read/write etc with promises instead of cbs. Luckily we don’t have any public apis with cbs exposes on the stream itself so interop is trivial

@phated
Copy link

phated commented Dec 7, 2020

@mafintosh have you done any profiling on creating so many intermediate promises? I've not, so I'm just curious.

@martinheidegger
Copy link
Contributor Author

This gets really tricky if someone makes a function that takes a callback and returns a promise.

It needs to be mutually exclusive. In the implementation as-is I relied on the skill of the programmer (either pass-in a regular fn or a fn returning a promise). I see that this might be a bit naive, so how about adding a guard for regular function to exit with an error or show a warning if a regular function returns a promise (wrong API use).

@martinheidegger
Copy link
Contributor Author

martinheidegger commented Jan 11, 2022

Rebased to match recent changes. In order to address the recent comment: if an async function is detected but no promise is returned, an error will be thrown.

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.

3 participants