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

Opaque types for WebGL #5855

Open
crobi opened this issue Dec 1, 2015 · 9 comments
Open

Opaque types for WebGL #5855

crobi opened this issue Dec 1, 2015 · 9 comments
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

@crobi
Copy link

crobi commented Dec 1, 2015

WebGL types are included in the default lib.d.ts. However, most types are defined as empty interfaces, which means that the compiler doesn't catch many WebGL errors.

Consider the following code:

function foo(gl: WebGLRenderingContext, fbo: WebGLFramebuffer) {
  gl.bindFramebuffer(gl.FRAMEBUFFER, fbo); // Correct call
  gl.bindFramebuffer(gl.FRAMEBUFFER, "abc"); // This should be a compile error!

  fbo = gl.createFramebuffer(); // Correct call
  fbo = new WebGLFramebuffer; // This should be a compile error!
}
  • The second parameter of gl.bindFramebuffer should be a WebGLFramebuffer object, but passing a string does not emit a compile error or warning.
  • The type WebGLFramebuffer can only be constructed via WebGLRenderingContext.createFramebuffer, calling new throws an error at runtime

Ideally, Typescript would know that WebGLFramebuffer, even though it has no public properties, cannot be constructed or converted to any other type.
I assume that this is not easy to specify with structural typing, but having such opaque types could be useful for other libraries as well (e.g., for returning handles).

If that is not possible, can we find a workaround that at least helps catching the above mentioned bugs?

For reference, here's how the above types are currently defined in lib.d.ts:

interface WebGLFramebuffer extends WebGLObject {
}
declare var WebGLFramebuffer: {
    prototype: WebGLFramebuffer;
    new(): WebGLFramebuffer;
}
interface WebGLRenderingContext {
    bindFramebuffer(target: number, framebuffer: WebGLFramebuffer): void;
    // + many other methods
}
@mhegazy
Copy link
Contributor

mhegazy commented Dec 1, 2015

@crobi you raise two issues, the first about WebGLObject being an empty object, and this should be fixed, either by updating the WebGL definitions or by just adding a constructor property to each of these types, this should do the trick. @zhengbli does your update of the spec take care of this? if not can we add the constructor declaration declaration?

interface WebGLFramebuffer extends WebGLObject {
    constructor: WebGLFramebuffer;
}

the second one is about constructors, most of the DOM elements have constructors though they are not callable. the problem is that instanceof relies on constructors, if we remove the constructors we will need to come up with a new way to make instanceof work. the other option is to have some sort of private constructors that are callable but still exist for instanceof checks. i think this is a more general issue and needs a general solution

@mhegazy
Copy link
Contributor

mhegazy commented Dec 1, 2015

@crobi as a work around, you can define the constructor in your files, and this should cause the compiler to flag the error as expected.

@mhegazy mhegazy added the Bug A bug in TypeScript label Dec 1, 2015
@mhegazy mhegazy added this to the TypeScript 1.8 milestone Dec 1, 2015
@mhegazy mhegazy added the Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript label Dec 1, 2015
@mhegazy
Copy link
Contributor

mhegazy commented Dec 1, 2015

sorry, scrap my original comment, the constructor would not do it, we need something not defined on object, some sort of _brand:any.

@crobi
Copy link
Author

crobi commented Dec 1, 2015

the constructor would not do it, we need something not defined on object, some sort of _brand:any.

This crossed my mind as well, but I'm not sure such a workaround would look nice in lib.d.ts. Maybe this is another use case for #202? Structurally, all WebGL objects are identical to the empty interface, but (due to being native browser objects with hidden properties?) nominally they are all distinct types.

the problem is that instanceof relies on constructors, if we remove the constructors we will need to come up with a new way to make instanceof work.

In plain Javascript, instanceof works as expected on WebGL objects (see below). Is there an additional transformation that Typescript performs on instanceof, that would require compile-time information about the constructors?

> gl.createFramebuffer() instanceof WebGLFramebuffer
true

> gl.createFramebuffer() instanceof WebGLTexture
false

> new WebGLFramebuffer
Uncaught TypeError: Illegal constructor

> gl.createFramebuffer()
WebGLFramebuffer {}

@mhegazy
Copy link
Contributor

mhegazy commented Dec 1, 2015

In plain Javascript, instanceof works as expected on WebGL objects (see below). Is there an additional transformation that Typescript performs on instanceof, that would require compile-time information about the constructors?

No run time transformations. the right operand of instanceof is expected to have a constructor, so we will need to remove this restriction, or add a non-callable constructor.

@weswigham
Copy link
Member

@mhegazy A noncallable constructor would be what abstract classes would need as well (both for instanceof and things like passing an abstract class constructor function as a valid type for a dynamic extends clause), so I suspect that would be the more general solution.

@mhegazy mhegazy added the Help Wanted You can do this label Dec 8, 2015
@mhegazy mhegazy modified the milestones: Community, TypeScript 1.8, TypeScript 2.0 Dec 8, 2015
@mhegazy mhegazy modified the milestones: Community, TypeScript 2.0 Feb 24, 2016
@zenmumbler
Copy link

zenmumbler commented Jul 5, 2016

FWIW, I added the full WebGL extensions and simple branding in my project, see:
https://github.com/stardazed/stardazed/blob/master/typings/webgl.d.ts
Updated: link to changed types was changed, GL2 stuff was added and extensions separated.

I could turn this into a PR for lib.d.ts if wanted. This was also submitted in an earlier form to DefinitelyTyped:
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/webgl-ext/webgl-ext.d.ts

That one does not have the branding values, which until #202 is implemented may be preferable for the core lib as the branding types are a workaround.

@sufianrhazi
Copy link

I'd like to propose a compromise until #202 is implemented.

Instead of taking the branded approach which muddies up lib.d.ts with distinct fields for each class, we could declare these WebGL interfaces as classes with private constructors[1] and with a private member named webGL[2]. To demonstrate:

declare class WebGLObject {
  private invalidated: bool; // Why invalidated: bool? See note [3]
}

declare class WebGLFramebuffer extends WebGLObject {
  private constructor(); // Enforce the second class of error: new WebGLFramebuffer
  private webGL: any;
}

Makes the original example produce readable and actionable type errors:

example.ts(3,38): error TS2345: Argument of type '"abc"' is not assignable to parameter of type 'WebGLFramebuffer'.
example.ts(6,9): error TS2673: Constructor of class 'WebGLFramebuffer' is private and only accessible within the class declaration.

This approach also prevents another dangerous surprise which presently typechecks just fine:

function wrongDeletion(gl: WebGLRenderingContext, fbo: WebGLFramebuffer) {
    gl.deleteBuffer(fbo); // This should have been gl.deleteFramebuffer(fbo);
}

With this approach, we'll have a type error:

surprises.ts(2,21): error TS2345: Argument of type 'WebGLFramebuffer' is not assignable to parameter of type 'WebGLBuffer'.
  Types have separate declarations of a private property 'webGL'.

I'll follow up shortly with a PR.


Notes:

  1. Private constructors make the objects unable to be constructed at the typecheck stage. This does have the problem that these classes cannot be subclassed, even in declaration, which means we can't apply it to WebGLObject, so new WebGLObject type-checks. This doesn't seem too bad, as WebGLObject isn't typically used and doesn't even appear on window in Chrome, Firefox, or Safari.
  2. The field name webGL is mostly for error message readability; note how the field name appears in error messages.
  3. The private invalidated: bool; member on WebGLObject is a really convenient reading-between-the-lines of the spec, which claims "Each WebGLObject has an invalidated flag, which is initially unset."

@sufianrhazi
Copy link

Strike the "follow up shortly with a PR" it looks like this may require a change to TSJS-lib-generator to support generating/overriding declare class, I'll have to dig a bit deeper to know how to proceed

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.

7 participants