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

Fix implementation for node v21 #7

Closed
wants to merge 2 commits into from

Conversation

airhorns
Copy link
Contributor

@airhorns airhorns commented Dec 17, 2023

On top of #8. Fixes #6.

This fixes support for the latest nodejs version (node 21). Before this change, the tests failed with this kind of message:

/Users/airhorns/Code/stream-shift/index.js:16
    return state.buffer[0].length
                           ^

TypeError: Cannot read properties of null (reading 'length')
    at getStateLength (/Users/airhorns/Code/stream-shift/index.js:16:28)
    at shift (/Users/airhorns/Code/stream-shift/index.js:6:99)
    at Test.<anonymous> (/Users/airhorns/Code/stream-shift/test.js:24:10)
    at Test.bound [as _cb] (/Users/airhorns/Code/stream-shift/node_modules/tape/lib/test.js:89:17)
    at Test.run (/Users/airhorns/Code/stream-shift/node_modules/tape/lib/test.js:108:7)
    at Test.bound [as run] (/Users/airhorns/Code/stream-shift/node_modules/tape/lib/test.js:89:17)
    at Immediate.next (/Users/airhorns/Code/stream-shift/node_modules/tape/lib/results.js:151:7)
    at process.processImmediate (node:internal/timers:478:21)

Node.js v21.3.0

Node made some major changes to the _readableState object for performance here: https://github.com/nodejs/node/pull/50341/files#diff-040c1f5a53844e600d40b33c4624f1fe39fcf2f8d62c76ca3fc5ea5442231469 which broke this package's inspection. buffer[0] is no longer always present, but if it is, it is an array who's .length we can use.

See tests passing here: gadget-inc#2

@airhorns airhorns changed the title Node v21 Fix implementation for node v21 Dec 17, 2023
@airhorns
Copy link
Contributor Author

Tried this in prod at my company and it works just fine!

index.js Outdated
@@ -11,9 +11,9 @@ function getStateLength (state) {
// Since node 6.3.0 state.buffer is a BufferList not an array
if (state.buffer.head) {
return state.buffer.head.data.length
} else if (state.buffer[0]) {
Copy link
Owner

Choose a reason for hiding this comment

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

if (state.buffer.length > 0 && state.buffer[0])

@mafintosh
Copy link
Owner

Thanks, added a comment, otherwise LGTM

[no-changelog-required]
@jamie-phlo
Copy link

Any ETA on when this will be merged/released at all?

@mafintosh
Copy link
Owner

Landed in master

@mafintosh mafintosh closed this Jan 10, 2024
@mafintosh
Copy link
Owner

1.0.2

confused-moniker added a commit to IGUHealth/IGUHealth that referenced this pull request May 16, 2024
@aws/crypto has Dep on Duplexify which uses stream-shift library. This broke on latest Node ver the following bumps dep with fix see issue bellow:

mafintosh/stream-shift#7
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.

Node 21.2 breaking changes
3 participants