Skip to content

fix(core): always call callback in DefaultStream._flush#2255

Merged
arcanis merged 1 commit intomasterfrom
merceyz/node-15
Dec 21, 2020
Merged

fix(core): always call callback in DefaultStream._flush#2255
arcanis merged 1 commit intomasterfrom
merceyz/node-15

Conversation

@merceyz
Copy link
Copy Markdown
Member

@merceyz merceyz commented Dec 20, 2020 •
edited
Loading

What's the problem this PR addresses?

DefaultStream._flush doesn't call the callback if the stream isn't empty which, because node 15 waits for it, causes the event loop to become empty and the process exits early during workspaces foreach calls

Note that the tests catch this when they run under node 15 but I can't enable it because of #2232

Fixes #2214
Fixes #2074

How did you fix it?

Always call the callback.

Checklist

  • I have read the Contributing Guide.
  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@merceyz merceyz requested a review from arcanis as a code owner December 20, 2020 00:10
@merceyz merceyz marked this pull request as draft December 20, 2020 00:10
@merceyz merceyz changed the title ci: test node 15 fix(core): always call callback in DefaultStream._flush Dec 20, 2020
@merceyz merceyz marked this pull request as ready for review December 20, 2020 20:21
@arcanis arcanis merged commit 3b8ed0b into master Dec 21, 2020
@arcanis arcanis deleted the merceyz/node-15 branch December 21, 2020 17:56
This was referenced Dec 31, 2020
Closed
Closed
@merceyz merceyz mentioned this pull request Jan 21, 2021
Closed
1 task
@merceyz merceyz mentioned this pull request Feb 13, 2021
Closed
1 task
@merceyz merceyz mentioned this pull request Mar 17, 2021
Closed
1 task
@kiancross kiancross mentioned this pull request Apr 7, 2021
Merged
@merceyz merceyz mentioned this pull request May 7, 2021
Closed
1 task
@AlCalzone
Copy link
Copy Markdown

AlCalzone commented Jun 26, 2021

This may be a stupid question, but how do I get this version? I'm on yarn 2.4.2, freshly installed, freshly imported the workspace-tools, but yarn workspaces foreach still only runs in the root:

$ yarn workspaces list -v
➤ YN0000: .
➤ YN0000: packages/config
➤ YN0000: packages/core
➤ YN0000: packages/maintenance
➤ YN0000: packages/serial
➤ YN0000: packages/shared
➤ YN0000: packages/testing
➤ YN0000: packages/zwave-js
➤ YN0000: Done in 0s 3ms

$ yarn workspaces foreach exec echo HELLO
←[94m➤←[39m ←[90mYN0000←[39m: HELLO

@merceyz
Copy link
Copy Markdown
Member Author

merceyz commented Jun 26, 2021 •
edited
Loading

Update to the latest RC

yarn set version 3.0.0-rc.6 && yarn plugin import workspace-tools

@AlCalzone
Copy link
Copy Markdown

AlCalzone commented Jun 26, 2021

Thanks that seems to work!

@merceyz merceyz mentioned this pull request Jul 16, 2021
Closed
@merceyz merceyz mentioned this pull request Feb 2, 2022
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@arcanis arcanis Awaiting requested review from arcanis

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] workspace-tools foreach doesn't run each workspace for node 15 [Bug] workspace-tools foreach doesn't run each workspace for node 15

3 participants

@merceyz @AlCalzone @arcanis