purescript-contrib / purescript-arraybuffer Goto Github PK

Describe the bug
polyFill function is not exported from Typed.js, but called at the top-level of Typed.js itself. This seems to cause issues with bundling, where the definition isn't picked up by the bundle, causing the top-level polyFill() call to report Uncaught ReferenceError: polyFill is not defined.

To Reproduce
Initialize a spago project via spago init. Add the following list of dependencies to spago.dhall:

  [ "arraybuffer"
  , "arraybuffer-types"
  , "console"
  , "effect"
  , "prelude"
  , "psci-support"
  ]

Modify src/Main.purs to be as follows:

-- src/Main.purs
module Main where

import Prelude

import Data.ArrayBuffer.Types (ArrayView, Int32)
import Data.ArrayBuffer.Typed as ArrayBuffer
import Effect (Effect)
import Effect.Console (log)

main :: Effect Unit
main = do
  _ :: ArrayView Int32 <- ArrayBuffer.empty 0
  log "hello"

Bundle it and run it via node:

$ spago bundle-app
$ node index.js

Expected behavior
See the output: "hello".

Actual behavior

<path-to-project>\index.js:6
  polyFill();
  ^

ReferenceError: polyFill is not defined

Additional context
Running the bundle through browser or node does not make a difference, failing in both cases.

Shouldn't some of the functions in Data.ArrayBuffer.ArrayBuffer (create in particular, but potentially others too) be Eff computations?

This package doesn't compile with current versions of psc. Will it be supported in the future?
Best regards
Jürgen

The Typed.part function takes an Index as its second argument, in units of the number of typed elements. What if we want to make an ArrayView which doesn't start in the ArrayBuffer at a typed element boundary?

https://github.com/jacereda/purescript-arraybuffer/blob/7f90bc44247dcbeab70caef5d373833286468266/src/Data/ArrayBuffer/Typed.purs#L137-L137

I think this library should also export the Typed.part' function, which is the same but with a ByteOffset type for the second argument.

I find myself needing to do these casts all the time and I think we should add these to the package.

Really these two types are the same type and making them different was a JavaScript design mistake.

fromUint8Array :: Uint8Array -> Effect DataView
fromUint8Array x = DV.part (AT.buffer x) (AT.byteOffset x) (AT.byteLength x)

toUint8Array :: DataView -> Effect Uint8Array
toUint8Array x = AT.part (DV.buffer x) (DV.byteOffset x) (DV.byteLength x)

Sync this repo's labels with the labels we use across the core, contrib, web, and node libraries.

These should be safe, read-only operations. Is wrapping the result in Eff necessary?

The transient dependency on the text encoding library is there to provide a very specific use-case that is not general to all users of this library, and thus makes more sense to be removed. Ideally this could be a two stage process where first a new library is released providing the needed functionality, and then the functionality in this library can be removed, alongside a link and explanation that a new library now provides this behavior.

This issue is being created in response to #19

This is a useful package! Got any plans to register it on bower?

Aren't the comments for slice and subArray reversed?

The current implementations use .fromCharCode and toCharCode which is the first 16 bytes of the utf16 code point space.

To have true support for the UTF-16 character range, and to ensure consistent behaviour across character sets, we should expand the functionality of fromString and decodeToString.

This may require some less cpu efficient js than we currently use, so perhaps we can look at naming the functions or documenting them in such a way that users can make an informed choice about character space versus performance. If they only need the first 65 thousand characters, then they can use the "faster" charCodeAt based implementation. If they need support for more esoteric code points, another function can provide that.

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!

• Run contrib-updater.
• Change default branch to main.
• Confirm CI is working.
• Delete uint and float32 additions in package.dhall.

https://github.com/purescript-contrib/governance/blob/main/library-guidelines.md

My apologies for the many questions, I'm just getting started with this lib and some of the design choices aren't obvious to me.

The fromArray implementation uses a Uint8Array for conversion, which seemingly would have less surprising behavior if it were reading from an Array Int rather than an Array Number, no?

This is more of a conversation than it is an issue, but I think it might be a cool way of interfacing with array buffers:

Basically, in my branch that I'm going to submit, TypedArrays are represented with a typeclass, specifically something that indexes a userland type (like Int or Number (used to be UInt too, but more on that later...)) from a ArrayViewType - similar to Snoyman's mono-traversable package. Right now, most of the arrays' userland types are just Ints, with Number being used for Uint32Arrays, because the values are out of range for traditional purescript Ints.

I'm not using UInts anymore because they clearly encode values between 2^31 and 2^32 - 1 in
the negative range. This won't work for Uint32Arrays because the values it interprets are directly in the positive range (because Ints are just Numbers under-the-hood), therefore UInts are directly not actually in that range and all those values would get lost (probably assigned to 0). Something that I'd like to get added to purescript-math is fround :: Number -> Number in purescript-deprecated/purescript-math#21 and purescript-deprecated/purescript-math#19, which interestingly casts a Float64 to a Float32, but that's an aside on this aside.

Anyway, the conversation is this - purescript-quotient offers a nice newtype'd interface for modulo'ing values - https://pursuit.purescript.org/packages/purescript-quotient/2.0.0/docs/Type.Quotient#t:Canonical

I think it might be nice having something like (Int / Mod256) / Abs as a Byte, for instance. Maybe we should use purescript-typelevel and properly state the type as modulo an exponent of 2, like literally x :: Quotient Int D8 as ````mod2^8```, but that's even more of an aside. This could be really useful for pinning down instances of userland types of arraybuffers to exactly treat values as their bit capacity - a Uint8*Array would be(Int / Mod256) / Abs`, because we want values between `1` and `2^8 - 1`, and only positive values. `Int8` would also be just `Int / Mod128` for instance, because we want the negative values too.

What do you all think?

For moving this package to purescript-contrib, @thomashoneyman has requested that the package has no dependencies outside of purescript-contrib, and the purescript-typelevel package does not belong to purescript-contrib.

The BytesPerValue class with the dependency on purescript-typelevel was introduced in this redesign by @athanclark .

#23

Do you have any advice about this @jacereda @athanclark ? How would you approach this?

This appears to be ignored, but there shouldn't be any reason to send it in the first place.

Currently, the latest published tag is v2.0.0 - see https://github.com/jacereda/purescript-arraybuffer/tags to verify. However, the dependencies have been updated since then - see e.g. https://github.com/jacereda/purescript-arraybuffer/blob/61a273192b889784ddc1c18ad4ad9dd7403249e2/bower.json

Did you mean to publish a new release compatible with 0.10? Since git tags aren't pushed by default, you might have accidentally forgotten to git push origin v3.0.0.

I've learned that V8 throws an exception if you try to create a non-aligned typed array, so I was wrong to ask for #27 .

We should not export Typed.part'; it will fail with a non-aligned ByteOffset.

I believe fixing this would mean doing the following:

• reverting the change
• republishing a new v11.0.3, which is the same as v11.0.1
• adding the change back in, except adding the readme update part, too
• publishing a breaking release as v12.0.0

Ideally, we could avoid this work by just checking out v11.0.1 and publishing that tag as v11.0.3, but I'm not sure if that works or not.

Originally posted by @JordanMartinez in #34 (comment)

Should everything in this package be in the ST monad instead of Effect?

https://pursuit.purescript.org/packages/purescript-st/5.0.1/docs/Control.Monad.ST.Global#v:toEffect

We don't currently support BigInt64Array, BigUint64Array. Should we?

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray#typedarray_objects

Bower components should use semvar. In semvar, 1.0.0 => 1.0.1 indicates a patch update - a backwards compatible bug fix. Changing type signatures is a non-backwards compatible change, so the major version should have been bumped to help avoid type errors when compiling old code.

All of these getters and setters could be reified and would probably be faster if they were.

But the first thing thing to do would be to write a benchmark so that we could observe the difference.