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

Weird dead code in lib/stream.js #30040

Closed
isaacs opened this issue Oct 20, 2019 · 1 comment
Closed

Weird dead code in lib/stream.js #30040

isaacs opened this issue Oct 20, 2019 · 1 comment
Labels
stream Issues and PRs related to the stream subsystem.

Comments

@isaacs
Copy link
Contributor

isaacs commented Oct 20, 2019

Noticed this bit of code in lib/stream.js on the master branch:

const version = process.version.substr(1).split('.');
if (version[0] === 0 && version[1] < 12) {
  Stream._uint8ArrayToBuffer = Buffer;
} else {
  ...
  1. Since the code internal in node can only ever be the version that it is, it seems odd to be checking the version in this way.
  2. It looks like it's checking for node 0.x < 0.12??
  3. Splitting a string returns an array of strings, so version[0] === 0 would always return false even in node 0.11 and before.

I think this can be removed?

  • Version: 12.12.0
  • Platform: All
  • Subsystem: stream
@lpinca
Copy link
Member

lpinca commented Oct 20, 2019

I think it was added for readable-stream but yeah it seems broken and readable-stream now requires Node.js >= 6.

lpinca added a commit to lpinca/node that referenced this issue Oct 20, 2019
`String.prototype.split()` returns an array of strings so the branch is
never taken.

Fixes: nodejs#30040
@lpinca lpinca mentioned this issue Oct 20, 2019
2 tasks
@lpinca lpinca added the stream Issues and PRs related to the stream subsystem. label Oct 20, 2019
@Trott Trott closed this as completed in 94230d1 Oct 22, 2019
Trott pushed a commit that referenced this issue Oct 22, 2019
The fallback code is no longer used when exporting to readable-stream.

Refs: #29514

PR-URL: #30041
Fixes: #30040
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
MylesBorins pushed a commit that referenced this issue Oct 23, 2019
`String.prototype.split()` returns an array of strings so the branch is
never taken.

Fixes: #30040

PR-URL: #30041
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
MylesBorins pushed a commit that referenced this issue Oct 23, 2019
The fallback code is no longer used when exporting to readable-stream.

Refs: #29514

PR-URL: #30041
Fixes: #30040
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
MylesBorins pushed a commit that referenced this issue Oct 23, 2019
`String.prototype.split()` returns an array of strings so the branch is
never taken.

Fixes: #30040

PR-URL: #30041
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
MylesBorins pushed a commit that referenced this issue Oct 23, 2019
The fallback code is no longer used when exporting to readable-stream.

Refs: #29514

PR-URL: #30041
Fixes: #30040
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit that referenced this issue Nov 8, 2019
`String.prototype.split()` returns an array of strings so the branch is
never taken.

Fixes: #30040

PR-URL: #30041
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit that referenced this issue Nov 8, 2019
The fallback code is no longer used when exporting to readable-stream.

Refs: #29514

PR-URL: #30041
Fixes: #30040
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit that referenced this issue Nov 10, 2019
`String.prototype.split()` returns an array of strings so the branch is
never taken.

Fixes: #30040

PR-URL: #30041
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit that referenced this issue Nov 10, 2019
The fallback code is no longer used when exporting to readable-stream.

Refs: #29514

PR-URL: #30041
Fixes: #30040
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit that referenced this issue Nov 11, 2019
`String.prototype.split()` returns an array of strings so the branch is
never taken.

Fixes: #30040

PR-URL: #30041
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
targos pushed a commit that referenced this issue Nov 11, 2019
The fallback code is no longer used when exporting to readable-stream.

Refs: #29514

PR-URL: #30041
Fixes: #30040
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants