SourceBuffer.appendBuffer() is missing overload for ArrayBufferView param in lib.d.ts

[philipbulley]

According to the MediaSource Candidate Recommendation Spec, there are two SourceBuffer.appendBuffer() method overloads.

MSDN only mentions the ArrayBuffer overload, even though this is later contradicted in the example code, further down on the same page:

videoSource.appendBuffer(new Uint8Array(xhr.response));

(Uint8Array indeed inherits from ArrayBufferView, not ArrayBuffer).

My tests show that Chrome 36 currently only works with ArrayBufferView (as per the MSDN example code).

Anyway, long story short:

Here is the lib.d.ts change request, as supported by the spec, MSDN example code and Chrome 36:

Existing:

interface SourceBuffer extends EventTarget {
    updating: boolean;
    appendWindowStart: number;
    appendWindowEnd: number;
    buffered: TimeRanges;
    timestampOffset: number;
    audioTracks: AudioTrackList;
    appendBuffer(data: ArrayBuffer): void;
    remove(start: number, end: number): void;
    abort(): void;
    appendStream(stream: MSStream, maxSize?: number): void;
}

Suggested:

interface SourceBuffer extends EventTarget {
    updating: boolean;
    appendWindowStart: number;
    appendWindowEnd: number;
    buffered: TimeRanges;
    timestampOffset: number;
    audioTracks: AudioTrackList;
    appendBuffer(data: ArrayBuffer): void;
    appendBuffer(data: ArrayBufferView): void;
    remove(start: number, end: number): void;
    abort(): void;
    appendStream(stream: MSStream, maxSize?: number): void;
}

[RyanCavanaugh]

Why do we need this? ArrayBufferView is a subtype of ArrayBuffer, so it's already valid to pass in to appendBuffer. There's no need for another overload (in fact, it would never be selected, because any ArrayBufferView would match the first overload).

[philipbulley]

ArrayBufferView contains an ArrayBuffer as buffer, but doesn't inherit from it.

From lib.d.ts:

interface ArrayBufferView {
    buffer: ArrayBuffer;
    byteOffset: number;
    byteLength: number;
}

I believe this is why the spec outlines the two separate overloads.

[RyanCavanaugh]

TypeScript uses a structural type system; explicitly declaring inheritance is unnecessary.

var videoSource: SourceBuffer;
// Does not error
videoSource.appendBuffer(new Uint8Array(null));

[lukehoban]

Note that there is not a subclass relationship between ArrayBuffer and ArrayBufferView in WebIDL. They are unrelated types, but every ArrayBufferView stores a reference to an ArrayBuffer (delegation not inheritance).

It's a coincidence right now that you can successfully pass an ArrayBufferView where an ArrayBuffer is expected. But, for example, if #310 is fixed, this will no longer be true.

[philipbulley]

Uint8Array.prototype instanceof ArrayBuffer
false 

It's for that reason that lib.d.ts requires the overload as outlined in the first post.

As an aside, @lukehoban, why does Typescript even allow for that coincidence?

[RyanCavanaugh]

Let's not get in to why TypeScript uses a structural type system in this thread, please.

We can add the overload to lib.d.ts -- the IE file we generated this from lists this but has it commented out because IE doesn't differentiate between the two overloads.

[philipbulley]

@RyanCavanaugh I'm happy to submit a PR, but need more info on strategy:

• Assuming the IE file comes from the IE devs, the Typescript team wont want to change this? (is this IE file even in the repo?)
• So would we need to do something like prevent SourceBuffer from being generated in dom.generated.d.ts and manually define it in extensions.d.ts?

[danquirk]

We don't need to add any complicated process here with a new .d.ts, let's just add whatever changes are necessary to lib.d.ts and we can manage integrating that with the generated bits from the IE definitions.

[mhegazy]

Sorry for the confusion. we can not accept pull requests to this. we need to update our script. I will take care of this.

[philipbulley]

👍 thanks @mhegazy

[philipbulley]

@mhegazy, @RyanCavanaugh this fix doesn't appear to have successfully made it into 1.1.

The SourceBuffer definition in v1.1's lib.d.ts contains only

appendBuffer(data: ArrayBuffer): void;

it should contain

appendBuffer(data: ArrayBuffer): void;
appendBuffer(data: ArrayBufferView): void;

I'm guessing that you previously updated lib.d.ts manually, but this was later overwritten by the auto generation?

[danquirk]

That is likely the case :(

[zhengbli]

For some issue in my script the overload of "ArrayBufferView" shows up as "any". Will fix soon.

[zhengbli]

Related PR #2827