three-ts-types icon indicating copy to clipboard operation
three-ts-types copied to clipboard

Draft: sketch out approach to better BufferAttribute typings #35

Open kmannislands opened this issue 3 years ago • 2 comments

This is a draft to collect feedback on the idea of improving specificity of BufferAttribute typings, especially as they relate to BufferGeometry typings

Why

The current typing of attributes on BufferGeometries is too wide. It's not possible to specify what attributes are actually defined and what "flavor" of TypedArray actually underlies them.

What

I've made the BufferAttribute type generic so that the backing TypedArrays can be specified. I've leveraged this in the definition of BufferGeometry to make it possible to specify the attributes and their underlying types.

With this change, the following code works:

type geom = BufferGeometry<{ normal: Float32Array,  position: Uint16Array }>
type normal = geom['attributes']['normal'] // InterleavedBufferAttribute | BufferAttribute<Float32Array>
type position = geom['attributes']['position'] // InterleavedBufferAttribute | BufferAttribute<UInt16Array>

// @ts-expect-error Property 'foo' does not exist on type 'AttributesFor<{ normal: Float32Array; position: Uint16Array; }>'
type foo = geom['attributes']['foo'] // type error!

Using the previous wide type as default allows backwards compatibility:

type geom = BufferGeometry
type foo = geom['attributes']['foo']; // InterleavedBufferAttribute | BufferAttribute<TypedArray>

If there's an appetite for this sort of change, I'll button up the PR here with:

  • Handling for all variants of TypedArray
  • Handling of all methods of BufferGeometry
  • Better handling/generic typing of InterleavedBufferAttribute in the same vein as the way BufferAttribute is handled

Checklist

  • [ ] Checked the target branch (current goes master, next goes dev)
  • [ ] Added myself to contributors table
  • [ ] Ready to be merged

kmannislands avatar Aug 05 '22 20:08 kmannislands

I like where this is going. Could it be expanded to let the user say that an attribute is not a GLBufferAttribute (see https://github.com/three-types/three-ts-types/issues/389)?

Methuselah96 avatar Mar 30 '23 11:03 Methuselah96

Thanks for your work on this, sorry for the radio silence. I'm looking into building off of this to allow distinguishing between the different buffer attribute types (as discussed in https://github.com/three-types/three-ts-types/issues/389). Let me know if you would rather do that work yourself, and I'd be happy to review it.

Methuselah96 avatar Mar 30 '23 13:03 Methuselah96