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

fs: don't emit 'close' twice if emitClose enabled #31383

Closed
wants to merge 3 commits into from

Conversation

ronag
Copy link
Member

@ronag ronag commented Jan 16, 2020

fs streams have some backwards compat behavior that does not
behave well if emitClose: true is passed in options. This
fixes this edge case until the backwards compat is removed.

Fixes: #31366

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

fs streams have some backwards compat behavior that does not
behave well if emitClose: true is passed in options. This
fixes this edge case until the backwards compat is removed.

Fixes: nodejs#31366
@nodejs-github-bot nodejs-github-bot added the fs Issues and PRs related to the fs subsystem / file system. label Jan 16, 2020
@ronag ronag requested a review from lundibundi January 16, 2020 17:53
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I'd suggest a comment in the test to make it clear that it's testing that close is emitted once and not twice when emitClose is true. (But that's a nit. I'm fine with it as-is too.)

@nodejs-github-bot
Copy link
Collaborator

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 18, 2020
@nodejs-github-bot
Copy link
Collaborator

Trott pushed a commit that referenced this pull request Jan 20, 2020
fs streams have some backwards compat behavior that does not
behave well if emitClose: true is passed in options. This
fixes this edge case until the backwards compat is removed.

PR-URL: #31383
Fixes: #31366
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@Trott
Copy link
Member

Trott commented Jan 20, 2020

Landed in 7d5a86c

@Trott Trott closed this Jan 20, 2020
codebytere pushed a commit that referenced this pull request Feb 17, 2020
fs streams have some backwards compat behavior that does not
behave well if emitClose: true is passed in options. This
fixes this edge case until the backwards compat is removed.

PR-URL: #31383
Fixes: #31366
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@codebytere codebytere mentioned this pull request Feb 17, 2020
codebytere pushed a commit that referenced this pull request Mar 14, 2020
fs streams have some backwards compat behavior that does not
behave well if emitClose: true is passed in options. This
fixes this edge case until the backwards compat is removed.

PR-URL: #31383
Fixes: #31366
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
codebytere pushed a commit that referenced this pull request Mar 17, 2020
fs streams have some backwards compat behavior that does not
behave well if emitClose: true is passed in options. This
fixes this edge case until the backwards compat is removed.

PR-URL: #31383
Fixes: #31366
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@codebytere codebytere mentioned this pull request Mar 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Event 'close' emits twice (fs.createWriteStream)
4 participants