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

.subarray method should return Buffer instance #260

Closed
mikeal opened this issue Apr 3, 2020 · 4 comments · Fixed by #263
Closed

.subarray method should return Buffer instance #260

mikeal opened this issue Apr 3, 2020 · 4 comments · Fixed by #263

Comments

@mikeal
Copy link

mikeal commented Apr 3, 2020

Ran across this one today.

Buffer.subarray() in Node.js will return a Buffer instance, but in the browser it returns a Uint8Array.

mikeal added a commit to mikeal/buffer that referenced this issue Apr 3, 2020
@feross
Copy link
Owner

feross commented Apr 14, 2020

I wasn't able to reproduce the issue in Chrome, Brave, Firefox, or Safari on desktop. Which browser did you use?

I tested by browserifying and running:

const Buffer = require('./').Buffer
const a = Buffer.from('test')
console.log(a.subarray(2, 3).constructor.name)

which prints out Buffer.

@mikeal
Copy link
Author

mikeal commented Apr 14, 2020

I’m using polendina for testing, which is a headless chrome that uses webpack (not browserify). I’m also importing the module explicitly rather that relying on globals.

@feross
Copy link
Owner

feross commented Apr 14, 2020

There's some weirdness going on with the Symbol.species code:

buffer/index.js

Lines 111 to 120 in b0a6de5

// Fix subarray() in ES2016. See: https://github.com/feross/buffer/pull/97
if (typeof Symbol !== 'undefined' && Symbol.species != null &&
Buffer[Symbol.species] === Buffer) {
Object.defineProperty(Buffer, Symbol.species, {
value: null,
configurable: true,
enumerable: false,
writable: false
})
}

Need to investigate further. Related: #97

feross added a commit that referenced this issue Apr 14, 2020
We now handle calls like:

new Buffer(arraybuffer, 2, 8) so there's no need for this workaround originally implemented in: #97

More info: https://bugs.chromium.org/p/v8/issues/detail?id=4665#c4

Fixes #260. Supersedes #261
@feross
Copy link
Owner

feross commented Apr 14, 2020

@mikeal Can you try the fix in this PR? #263

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 a pull request may close this issue.

2 participants