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

Constructor for DataView is too broad #52815

Closed
ngbrown opened this issue Feb 16, 2023 · 5 comments · Fixed by #53130
Closed

Constructor for DataView is too broad #52815

ngbrown opened this issue Feb 16, 2023 · 5 comments · Fixed by #53130
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Help Wanted You can do this
Milestone

Comments

@ngbrown
Copy link

ngbrown commented Feb 16, 2023

lib Update Request

Configuration Check

My compilation target is ES2021 and my lib is ["DOM", "DOM.Iterable", "ES2021"]. TypeScript version is 4.9.5.

Missing / Incorrect Definition

The DataView constructor parameter type is incorrectly broad by accepting ArrayBufferLike because TypedArray's such as Uint8Array match the ArrayBufferLike type but are not valid parameters. I think the DataView constructor parameter should only accept ArrayBuffer | SharedArrayBuffer.

Sample Code

The following produces no error in TypeScript but does produce an error in Node 16, Chrome 109, and FireFox 111b1:

new DataView(new Uint8Array(32));

The runtime error is:

Uncaught TypeError: First argument to DataView constructor must be an ArrayBuffer
    at new DataView (<anonymous>)

The correct code that does run is:

new DataView(new Uint8Array(32).buffer)

Documentation Link

@fatcerberus
Copy link

ArrayBufferLike is already defined as ArrayBuffer | SharedArrayBuffer. The problem is that TS is structurally typed and typed arrays are structurally compatible with ArrayBuffer:

const foo: ArrayBuffer = new Uint8Array(42);  // no error

@ngbrown
Copy link
Author

ngbrown commented Feb 17, 2023

I knew that to a degree (duck typing and all), and I suppose someone will declare this as duplicate of #202, which I didn't know about. But this seems like these types should be the exception to structural typing because JavaScript is much more strict about the types actually being instanceof.

With lib.es5.d.ts listing ArrayBufferLike in the constructor there's a bit of indirection when looking at the type hints in an IDE, so you aren't clearly told that it should only be ArrayBuffer | SharedArrayBuffer because what is ArrayBufferLike anyways?

@fatcerberus
Copy link

Yeah, I think there's a feature request somewhere to be able to expand types in function hints.

Apparently this is intentional because doing otherwise is a breaking change: #31311 (comment)

@ngbrown
Copy link
Author

ngbrown commented Feb 17, 2023

Interestingly, never and Exclude<T, U> can almost help:

interface DataViewConstructor {
    readonly prototype: DataView;
    new<T extends (ArrayBuffer | SharedArrayBuffer) = ArrayBufferLike>(buffer: T & Exclude<T, {BYTES_PER_ELEMENT: number}>, byteOffset?: number, byteLength?: number): DataView;
}
declare var DataView: DataViewConstructor;

But it is a breaking change for types that inherit from DataView... Maybe someone smarter can make it work right without a generic?

@RyanCavanaugh
Copy link
Member

I think this is actually a lot easier to accomplish:

new(buffer: ArrayBufferLike & { BYTES_PER_ELEMENT?: never }, byteOffset?: number, byteLength?: number): DataView;

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript Help Wanted You can do this Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript labels Feb 17, 2023
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Feb 17, 2023
lpizzinidev added a commit to lpizzinidev/TypeScript that referenced this issue Mar 7, 2023
lpizzinidev added a commit to lpizzinidev/TypeScript that referenced this issue Mar 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Help Wanted You can do this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants