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

as_Array is broken #19

Closed
athanclark opened this issue Dec 2, 2018 · 6 comments
Closed

as_Array is broken #19

athanclark opened this issue Dec 2, 2018 · 6 comments

Comments

@athanclark
Copy link
Collaborator

Here's a minimal example:

arr :: DataView
arr = whole (fromArray [12.0, 23.0, 53.0, 0.0]) -- we need one number for each byte

getUint32le arr 0 -- Just 3479308u

getUint32le (dataView (asUint32Array arr)) 0 -- runtime exception - cannot read property `call` of undefined

I noticed that the implementation for fromArray simply builds it out of a Uint8Array, where each Number is one byte. However, asUint32Array builds a new typed array using the previous buffer. I wonder if there's just some faulty logic going on bit-wise in regards to the buffer offset or bytelength or something. If this library is no longer being maintained, I might redesign some of it if that's alright - I'm probably not gonna touch the type designs you have, but I might make the actual functions a little more true to the js api.

Thanks for making this regardless!

@jacereda
Copy link
Collaborator

jacereda commented Dec 2, 2018

Thanks for the report.

If you want to enhance the implementation, PRs are welcome. I'm not specially attached to the API either, so if you think something should be some other way we can discuss it.

Also, for other use cases, something like this should be much faster and close to the JS API:

https://github.com/mgmeier/purescript-typedarray

@athanclark
Copy link
Collaborator Author

I think that because this package is technically the mainstream package for arraybuffer / typedarray support, because it's published on pursuit, should definitely be at least fully funcitonal - I have a pretty convenient design in mind that's a little higher level than the package you referenced that uses typeclasses for the create mechanism. I'll also have a thorough test suite that proves the consistency of the logic too.

@athanclark
Copy link
Collaborator Author

I'm also going to cut the dependency on purescript-text-encoding - encoders and decoders for various implementations should be separate from the actual arraybuffer machinery, plus it's an extra npm dependency for something that could be pure.

@jacereda
Copy link
Collaborator

jacereda commented Dec 6, 2018

I also think the encoding/decoding should be moved to a separate library. There're lots of use cases where you just want binary data. @AlexaDeWit should probably be aware of this... Alexa, could you move that functionality to another library on top?

@AlexaDeWit
Copy link
Contributor

I might have some time this weekend. I'll take a look.

@athanclark
Copy link
Collaborator Author

I agree 100%, and it's already been ripped out. I also want to rip out the Data.ArrayBuffer.Show module - I don't think there should be any npm packages involved, just pure javascript. I also wanted to make arraybuffer-crypto for basic crypto implementations, and arraybuffer-algorithms as a sandbox for sorting algorithms and other cool stuff

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants