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

Inheriting from DataView/TypedArray #39

Open
rotu opened this issue Sep 12, 2023 · 3 comments
Open

Inheriting from DataView/TypedArray #39

rotu opened this issue Sep 12, 2023 · 3 comments

Comments

@rotu
Copy link
Contributor

rotu commented Sep 12, 2023

Most objects in this library inherit from DataView or TypedArray. That means they have a number of methods which don't respect the type abstraction.

e.g. since Uint16View extends DataView, it has a setFloat32 method. Note this also writes outside of the 2 bytes one expects the view to refer to.
e.g. since GridMixin extends a TypedArray, it has map, at, findIndex, which all use linear indexing.

Furthermore, Int32View extends Uint8View. This renders the type annotation x: Uint8View practically meaningless.

It would be nice if this library used less inheritance. It does make sense for binary-backed objects to implement the more minimal ArrayBufferView interface ({buffer, byteLength, byteOffset}).

@rotu
Copy link
Contributor Author

rotu commented Sep 12, 2023

@zandaqo, this seems like a big effort, and I want to know what you think before I start rewriting your whole library!

@zandaqo
Copy link
Owner

zandaqo commented Sep 13, 2023

Yes, typing can probably be improved, but inheriting from the DataView/TypedArray was a deliberate choice. I want to re-use their methods without writing custom logic for binary writing/reading (or keeping an extra hidden DataView/TypedArray around as some other libraries do) and risking performance because of it. ArrayBufferView is a type abstraction, it cannot be instantiated, only ArrayBuffer can, but we cannot read/write into it without the views.

@rotu
Copy link
Contributor Author

rotu commented Sep 13, 2023

Check this out! It took me way too long to figure out but I think this is exactly what you wanted.

// `extends null` isn't necessary but I prefer it.
class BareDataView extends null {
  static fromBuffer(...args) {
    // construct a new object, passing args to the DataView() constructor
    // but use this (i.e. the BareDataView class) to set the prototype
    return Reflect.construct(DataView, args, this)
  }
}

// Select only the properties we actually want to expose
// i.e. those that are part of the ArrayBufferView interface
for (const prop of ['buffer', 'byteLength', 'byteOffset']) {
  const descriptor = Object.getOwnPropertyDescriptor(DataView.prototype,prop)
  Reflect.defineProperty(BareDataView.prototype, prop, descriptor)
}

// Set up some nicely chosen bytes
let myBytes = new Uint8Array([0xff,0xff,0x00,0x84,0x5f,0xed,0xff,0xff])
let b = BareDataView.fromBuffer(myBytes.buffer)

// it has the buffer property!
console.log('buffer' in b) // true
// it doesn't have other DataView member!
console.log('getUint32' in b) // false
// but we can still use DataView methods
console.log(Reflect.apply(DataView.prototype.getUint32, b, [2])) // 8675309

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

No branches or pull requests

2 participants