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

Feature request: allow awaiting event, reject on "error" #21339

Closed
dead-claudia opened this issue Jun 15, 2018 · 3 comments
Closed

Feature request: allow awaiting event, reject on "error" #21339

dead-claudia opened this issue Jun 15, 2018 · 3 comments
Labels
duplicate Issues and PRs that are duplicates of other issues or PRs. events Issues and PRs related to the events subsystem / EventEmitter. feature request Issues that request new features to be added to Node.js.

Comments

@dead-claudia
Copy link

dead-claudia commented Jun 15, 2018

Edit: Added errorEvent option to configure error events

99% of my event adaptation of promises end up looking something close to this:

function open(file) {
    return new Promise((resolve, reject) => {
        const stream = fs.createWriteStream(file)

        function ready() {
            stream.removeListener("ready", ready)
            stream.removeListener("error", fail)
            resolve(stream)
        }

        function fail(e) {
            reject(e)
            stream.removeListener("ready", ready)
            stream.removeListener("error", fail)
        }

        stream.on("ready", ready)
        stream.on("error", fail)
    })
}

This is most frequently with streams, but it also occasionally shows up with other things, including with non-core libraries. What I'd prefer to write is something closer to this:

async function open(file) {
    const stream = fs.createWriteStream(file)
    await stream.awaitEvent("ready")
    return stream
}

My proposed API is this:

  • ee.awaitEvent("name", {includeAll = false, errorEvent = "event"} = {}): Return a promise fulfilled with an array of arguments once "name" is emitted, rejected if it's beat by an "error" event. An extra options object is permitted, and it accepts two options: includeAll, which resolves with an array of arguments intead, and errorEvent, which allows configuring what event is considered the "error" event.

The naïve implementation is pretty simple, and looks very similar to above.

EventEmitter.prototype.awaitEvent = function awaitEvent(event, {includeAll, errorEvent = "event"} = {}) {
    return new Promise((resolve, reject) => {
        const success = (...args) => {
            this.removeListener(event, success)
            this.removeListener(errorEvent, fail)
            if (includeAll) resolve(...args)
            else resolve(args[0])
        }

        const fail = e => {
            reject(e)
            this.removeListener(event, success)
            this.removeListener(errorEvent, fail)
        }

        this.on(event, success)
        this.on(errorEvent, fail)
    })
}
@Trott Trott added the feature request Issues that request new features to be added to Node.js. label Jun 15, 2018
@devsnek
Copy link
Member

devsnek commented Jun 15, 2018

related to #20909 and #20893

@devsnek devsnek added the events Issues and PRs related to the events subsystem / EventEmitter. label Jun 15, 2018
@dead-claudia
Copy link
Author

Yeah, my search wouldn't have found those. It's a near-duplicate, but only different in that it's actually error-tolerant and it doesn't leak. (Those leak if only one fires, which is the common case.)

@bnoordhuis
Copy link
Member

I'm going to close this as a duplicate because otherwise we're going to get scattered discussion. I see @isiahmeadows already chimed in on #20909 so that's probably a good a place as any to continue the discussion.

@bnoordhuis bnoordhuis added the duplicate Issues and PRs that are duplicates of other issues or PRs. label Jun 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate Issues and PRs that are duplicates of other issues or PRs. events Issues and PRs related to the events subsystem / EventEmitter. feature request Issues that request new features to be added to Node.js.
Projects
None yet
Development

No branches or pull requests

4 participants