Skip to content

lib.dom.d.ts OffscreenCanvas.getContext() not returning type based on context ID #1501

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

Closed
TomasHubelbauer opened this issue Feb 19, 2023 · 8 comments

Comments

@TomasHubelbauer
Copy link

This is a follow-up to microsoft/TypeScript#52831.

I would like to request that OffscreenCanvas.getContext behaves the same way as HTMLCanvasElement.getContext in that the latter returns different concrete context types depending on what context ID is passed in as an argument and the former just returns an umbrella context type that needs to be disambiguated first.

HTMLCanvasElement.getContext:

const canvas = document.createElement('canvas');
const context = canvas.getContext('2d');
// `context` is `CanvasRenderingContext2D`

// lib.dom.d.ts:
getContext(contextId: "2d", options?: CanvasRenderingContext2DSettings): CanvasRenderingContext2D | null;
getContext(contextId: "bitmaprenderer", options?: ImageBitmapRenderingContextSettings): ImageBitmapRenderingContext | null;
getContext(contextId: "webgl", options?: WebGLContextAttributes): WebGLRenderingContext | null;
getContext(contextId: "webgl2", options?: WebGLContextAttributes): WebGL2RenderingContext | null;
getContext(contextId: string, options?: any): RenderingContext | null;

OffscreenCanvas.getContext:

const canvas = new OffscreenCanvas(16, 16);
const context = canvas.getContext('2d');
// `context` is `OffscreenRenderingContext`

// lib.dom.d.ts:
getContext(contextId: OffscreenRenderingContextId, options?: any): OffscreenRenderingContext | null;

I believe it would be beneficial to add overloads with context ID literals which return specific context types.
Also, maybe it would make sense to introduce a context type string literal enum for the normal canvas context types, not just a string?

Documentation Link

@TomasHubelbauer
Copy link
Author

TomasHubelbauer commented Feb 19, 2023

As per: https://github.com/microsoft/TypeScript-DOM-lib-generator#when-the-type-exists-but-is-wrong

https://github.com/microsoft/TypeScript-DOM-lib-generator/blob/main/inputfiles/addedTypes.jsonc

This file doesn't have any mention of OffscreenCanvas.

https://github.com/microsoft/TypeScript-DOM-lib-generator/blob/main/inputfiles/overridingTypes.jsonc

This file does have an entry:

"OffscreenCanvas": {
    "methods": {
        "method": {
            "getContext": {
                "additionalSignatures": [
                    "getContext(contextId: \"2d\", options?: any): OffscreenCanvasRenderingContext2D | null",
                    "getContext(contextId: \"bitmaprenderer\", options?: any): ImageBitmapRenderingContext | null",
                    "getContext(contextId: \"webgl\", options?: any): WebGLRenderingContext | null",
                    "getContext(contextId: \"webgl2\", options?: any): WebGL2RenderingContext | null"
                ]
            }
        }
    }
},

It is missing convertToBlob which I would be happy to add but confusingly it already has the overloads that map literal context IDs to a specific rendering context types. Yet in lid.dom.d.ts these changes are missing.

Does this mean lib.dom.d.ts containing these overloads is just a matter of time (how to determine this amount of time?) or is there more to do I could help with?
Is a contributing for adding convertToBlob welcome?

@WearyMonkey
Copy link

WearyMonkey commented Feb 20, 2023

Prior to version 4.9 we used @types/offscreencanvas for the getContext types. However those types are now merged with the new OffscreenCanvas::getContext method from lib.dom.d.ts so the override narrowing is lost.

Would love to see the overrides added to lib.dom.d.ts so we can remove our dependency on @types/offscreencanvas

@saschanaz
Copy link
Contributor

This is fixed by #1474. Please try @types/web 0.0.91+ (or equivalent for workers) if you need it today.

@github-actions close

@github-actions
Copy link
Contributor

Closing because @saschanaz is one of the code-owners of this repository.

@TomasHubelbauer
Copy link
Author

@saschanaz Awesome! Sorry I searched around in the main TypeScript repository but neglected to search when I moved my issue over to this one after I learnt this is the place to do lib.dom.d.ts changes. Thanks for letting me know about @types/web though I am curious if you can also let me know the approximate timeline for when this change will make it to lib.dom.d.ts if it is possible to say at this moment?

@saschanaz
Copy link
Contributor

That depends very much on the TypeScript release schedule, and I have no idea about that. What I know is TS 5.0 will include it as microsoft/TypeScript#52328 includes the change.

@TomasHubelbauer
Copy link
Author

Perfect, thanks a lot!

@HolgerJeromin
Copy link
Contributor

That depends very much on the TypeScript release schedule, and I have no idea about that.

Will be March 14th

Ref microsoft/TypeScript#51362

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants