Update ArrayBuffer types to support TypeScript 5.9
#1702
Conversation
|
Thanks @jamesbvaughan! I'm wondering what to do with other Uint8Array references moving forward. Assuming that every Uint8Array used in the library is already a
|
|
@donmccurdy That sounds like a reasonable plan to me! |
|
I had targeted this specific function because it's the one that was blocking our upgrade, but I'll take some time today to make this more complete. |
| // @ts-expect-error | ||
| // TODO: Update the types here for TypeScript 5.9: https://github.com/donmccurdy/mikktspace-wasm | ||
| tangents({ generateTangents: mikktspace.generateTangents, ...options }), |
There was a problem hiding this comment.
This should probably be done before this PR is considered ready.
| @@ -172,7 +172,7 @@ export class WriterContext { | |||
| return accessorDef; | |||
| } | |||
|
|
|||
| public createImageData(imageDef: GLTF.IImage, data: Uint8Array, texture: Texture): void { | |||
| public createImageData(imageDef: GLTF.IImage, data: Uint8Array<ArrayBuffer>, texture: Texture): void { | |||
There was a problem hiding this comment.
There were a few cases like this where I either needed to update an argument's type or use a type assertion in order to appease TypeScript.
| @@ -44,22 +44,22 @@ export function prepareAccessor( | |||
| result.byteStride = accessor.getElementSize() * 4; | |||
| result.componentType = FLOAT; | |||
| result.normalized = false; | |||
| result.array = encoder.encodeFilterExp(array, accessor.getCount(), result.byteStride, bits); | |||
| result.array = encoder.encodeFilterExp(array, accessor.getCount(), result.byteStride, bits) as Uint8Array<ArrayBuffer>; | |||
There was a problem hiding this comment.
These lines either require type assertions like this, or an update to the meshoptimizer library's types.
| @@ -70,7 +70,7 @@ export function decodeAttribute( | |||
|
|
|||
| const ptr = decoderModule._malloc(byteLength); | |||
| decoder.GetAttributeDataArrayForAllPoints(mesh, attribute, dataType, byteLength, ptr); | |||
| const array: TypedArray = new ArrayCtor(decoderModule.HEAPF32.buffer, ptr, numValues).slice(); | |||
| const array: TypedArray = new ArrayCtor(decoderModule.HEAPF32.buffer as ArrayBuffer, ptr, numValues).slice(); | |||
There was a problem hiding this comment.
This either requires a type assertion or an update to draco3d's types
| @@ -338,7 +338,7 @@ async function _encodeWithSharp( | |||
| instance.resize(dstSize[0], dstSize[1], { fit: 'fill', kernel: options.resizeFilter }); | |||
| } | |||
|
|
|||
| return BufferUtils.toView(await instance.toBuffer()); | |||
| return BufferUtils.toView((await instance.toBuffer()) as Buffer<ArrayBuffer>); | |||
There was a problem hiding this comment.
This either requires a type assertion or an update to sharp's types.
| @@ -66,13 +66,13 @@ export class DefaultImageProvider implements ImageProvider { | |||
| const image = textureDef.getImage()!; | |||
| const mimeType = textureDef.getMimeType(); | |||
|
|
|||
| let texture = this._cache.get(image); | |||
| let texture = this._cache.get(image.buffer); | |||
There was a problem hiding this comment.
See the note near the bottom of TypeScript 5.9's release notes for more context on these changes: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-9.html#libdts-changes
|
Alright I've updated the types more comprehensively now. My process was to update TypeScript in this repo to 5.9, then to repeatedly run the build and fix errors until I addressed every type error that was breaking the build. I didn't include the TypeScript upgrade in this branch but I can if you'd like me to. A few notes:
I'd understand if you would prefer not to introduce those type assertions and want to hold off on these changes until all of the relevant dependencies have updated. |
jamesbvaughan
changed the title
writeBinary more specificArrayBuffer types to support TypeScript 5.9
When trying to upgrade a project from TypeScript v5.8 to v5.9, I hit the issue described here: https://devblogs.microsoft.com/typescript/announcing-typescript-5-9/#lib.d.ts-changes
This implements the suggested fix described there in order to make the library compatible with TypeScript 5.9.