search

dominictarr / JSONStream

rawStream.pipe(JSONStream.parse()).pipe(streamOfObjects)
Other
1.92k stars 165 forks source link

Int32Array bug for some (sometimes older) browser #31

Closed djcoin closed 10 years ago

djcoin commented 11 years ago

Hi,

I have an issue with the Int32Array. If the browser does not support typed array I guess it breaks. In my case (chromium Version 23.0.1271.97 (171054) ), it breaks because the Int32Array does not have the "slice" method. (This breaks in the jsonparse module)

But I was wondering if we couldn't just remove the Int32Array stuff now that Buffer is browserify-able ? If anyone knows a good typed-array polyfill, otherwise, it would be nice to notify it in the README so that you can take into account this requirement and work it around for older/weird browser, avoiding them to break.

Thanks !

djcoin commented 11 years ago

By the way, I got a "Uncaught Error: Unexpected "\u0000" at position 0 in state START " when commenting out the "if(process.browser)" part and using new Buffer (with the browserify-buffer shim) :s

mattsgarlata commented 10 years ago

Here is a fix that worked for us: https://github.com/SpiderStrategies/JSONStream/commit/2544fd0bea4be4cb21dd33937c24c309e577a1cf

dominictarr commented 10 years ago

oh sorry I missed this issue the first time - this shouldn't really be a problem now days because of https://github.com/feross/buffer <- complete polyfil for buffer in all browsers. You should get this if you use a recent browserify

dominictarr commented 10 years ago

@mattsgarlata does you code still work if you use Int8Array ? I it looks like you are allocating 4 bytes per input byte. Also, you could just create a Int8Array with the necessary length and then set it's values instead of creating an array and then turning it into an typed array

nathanbowser commented 10 years ago

@dominictarr We're just relying on using the buffer polyfill in the browser, now that it's supported. I don't think there's a reason to have the if (process.browser) check anymore, right?

dominictarr commented 10 years ago

that is correct, we could remove it.

mattsgarlata commented 10 years ago

I think it would be a good idea to remove it, since it doesn't work with unicode.

dominictarr commented 10 years ago

okay, it's removed in 0.8.3