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

'Canvas#getContext' almost never returns 'null' #19229

Open
cubedhuang opened this issue Oct 16, 2017 · 14 comments
Open

'Canvas#getContext' almost never returns 'null' #19229

cubedhuang opened this issue Oct 16, 2017 · 14 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Suggestion An idea for TypeScript

Comments

@cubedhuang
Copy link

TypeScript Version: 2.5.3

Code:

let canvas: HTMLCanvasElement = document.createElement("canvas");
let ctx: CanvasRenderingContext2D = canvas.getContext("2d");
ctx.fillRect(0, 0, canvas.width, canvas.height);

Expected behavior:
No errors should occur because the type of canvas is HTMLCanvasElement.

Actual behavior:

test.ts(2,5): error TS2322: Type 'CanvasRenderingContext2D | null' is not assignable to type 'CanvasRenderingContext2D'.
  Type 'null' is not assignable to type 'CanvasRenderingContext2D'.

If I change the type of ctx to CanvasRenderingContext2D | null, then I get

test.ts(3,1): error TS2531: Object is possibly 'null'.
@DanielRosenwasser DanielRosenwasser changed the title HTML type inference 'Canvas#getContext' almost never returns 'null' Oct 16, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Oct 16, 2017

From the spec: https://html.spec.whatwg.org/multipage/canvas.html#dom-canvas-getcontext

context = canvas . getContext(contextId [, ... ] )

Returns an object that exposes an API for drawing on the canvas. The first argument specifies the desired API, either "2d", "bitmaprenderer" or "webgl". Subsequent arguments are handled by that API.
This specification defines the "2d" and "bitmaprenderer" contexts below. There is also a specification that defines a "webgl" context. [WEBGL]
Returns null if the given context ID is not supported, or if the canvas has already been initialized with another context type (e.g. trying to get a "2d" context after getting a "webgl" context).

So the definition for getContext to return CanvasRenderingContext2D | null matches the spec. that said.. i have personally found that annoying as well.. it is possible we change the definition to remove the | null in violation of the spec, for convenience.

@DanielRosenwasser DanielRosenwasser added In Discussion Not yet reached consensus Suggestion An idea for TypeScript labels Oct 16, 2017
@mhegazy mhegazy added Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Suggestion An idea for TypeScript Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature and removed In Discussion Not yet reached consensus Suggestion An idea for TypeScript labels Oct 16, 2017
@weswigham
Copy link
Member

weswigham commented Oct 17, 2017

For reference,

let canvas: HTMLCanvasElement = document.createElement("canvas");
let ctx: CanvasRenderingContext2D | null;
if (!(ctx = canvas.getContext("2d"))) {
    throw new Error(`2d context not supported or canvas already initialized`);
}
ctx.fillRect(0, 0, canvas.width, canvas.height);

Is a decent way to use it in a typesafe way without really getting in the way; but if you must remove null from the type definition, you should probably only remove it from the overloads for "2d" and "webgl" (and I guess there should be one for "bitmaprenderer"), but not the base signature with string.

@NaridaL
Copy link
Contributor

NaridaL commented Oct 18, 2017

This just seems like a perfect use case for !, I don't think changing the dts is necessary:

let canvas: HTMLCanvasElement = document.createElement("canvas");
let ctx: CanvasRenderingContext2D = canvas.getContext("2d")!; // <--
ctx.fillRect(0, 0, canvas.width, canvas.height);

@lzxb
Copy link

lzxb commented Dec 15, 2017

@NaridaL
The first time I saw this write, is there any relevant source?

@NaridaL
Copy link
Contributor

NaridaL commented Dec 15, 2017

@lzxb https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-0.html#non-null-assertion-operator

@lzxb
Copy link

lzxb commented Dec 15, 2017

Thank you

imjasonmiller added a commit to imjasonmiller/image-kernels that referenced this issue Dec 28, 2019
This wrapper handles the possible null value for `getContext()`. Removing the need to check for it inline. See microsoft/TypeScript#19229 for more information.
imjasonmiller added a commit to imjasonmiller/image-kernels that referenced this issue Dec 28, 2019
This wrapper handles the possible null value for `getContext()`. Removing the need to check for it inline. See microsoft/TypeScript#19229 for more information.
agilgur5 added a commit to agilgur5/react-signature-canvas that referenced this issue Feb 6, 2020
- make heavy use of the multiple iterations on types in others' PRs,
  my own iterations on that, and then @types/react-signature-canvas
  - credit where credit is due!

- refactor init logic a bit to get around `| null` everywhere
  - kinda hacky but better than ifs or ! (non-nullable) everywhere
- use ! for canvas.getContext('2d') calls for same reason
  - c.f. microsoft/TypeScript#19229

- refactor naming everywhere to src/index.tsx

- configure tsconfig -- no need for allowJs anymore, but jsx needs to
  be set as TS is actually reading it now
- configure jest -- ts-jest needed to read TSX files

(deps): add DT typings for prop-types, react, and signature_pad
(deps): add a declaration file for trim-canvas as it doesn't have
types built-in yet (TODO for me!)
  - use typings/ to hold external declarations
agilgur5 added a commit to agilgur5/react-signature-canvas that referenced this issue Feb 13, 2020
- make heavy use of the multiple iterations on types in others' PRs,
  my own iterations on that, and then @types/react-signature-canvas
  - credit where credit is due!

- refactor init logic a bit to get around `| null` everywhere
  - kinda hacky but better than ifs or ! (non-nullable) everywhere
- use ! for canvas.getContext('2d') calls for same reason
  - c.f. microsoft/TypeScript#19229

- refactor naming everywhere to src/index.tsx

- configure tsconfig -- no need for allowJs anymore, but jsx needs to
  be set as TS is actually reading it now
- configure jest -- ts-jest needed to read TSX files

(deps): add DT typings for prop-types, react, and signature_pad
(deps): add a declaration file for trim-canvas as it doesn't have
types built-in yet (TODO for me!)
  - use typings/ to hold external declarations
@thw0rted
Copy link

I know this hasn't had any activity in ages, but I noticed that the relevant MDN page had incorrect information on it, saying that null is only returned in the case of an invalid contextType argument. Per the living standard, @mhegazy is correct and null should be returned any time you request a different context type than the first mode requested.

I already put an edit into the MDN page, flagged for technical review (I'm just some guy, you know?) but it sounds to me like this issue can be closed. Basically, the compiler can't know ahead of time if a given call will return null, but you as developer could know that e.g. a given canvas element will only ever be set to 2d context mode, so your call should never return null. In that case, a non-null assertion would make sense.

@ybigus
Copy link

ybigus commented Apr 15, 2022

a given canvas element will only ever be set to 2d context mode, so your call should never return null. In that case, a non-null assertion would make sense.

In theory yes, in reality, it can return null... Here is an example:

We are also experiencing this bug in the same way. After hundreds (or thousands) of small canvas elements being created and successfully drawn to with a 2d context, at some point it just gives up and returns null for the context. In our case we were able to switch to HTMLImageElements and then no problem, which shows that the memory budget doesn't have to be so small. These regression bugs are really frustrating for developers.
Source: https://bugs.webkit.org/show_bug.cgi?id=195325

I just got a similar issue in my project :)

agilgur5 added a commit to agilgur5/react-signature-canvas that referenced this issue Apr 20, 2022
- make heavy use of the multiple iterations on types in others' PRs,
  my own iterations on that, and then @types/react-signature-canvas
  - credit where credit is due!

- refactor init logic a bit to get around `| null` everywhere
  - kinda hacky but better than ifs or ! (non-nullable) everywhere
- use ! for canvas.getContext('2d') calls for same reason
  - c.f. microsoft/TypeScript#19229

- refactor naming everywhere to src/index.tsx

- configure tsconfig -- no need for allowJs anymore, but jsx needs to
  be set as TS is actually reading it now
- configure jest -- ts-jest needed to read TSX files

(deps): add DT typings for prop-types, react, and signature_pad
(deps): add a declaration file for trim-canvas as it doesn't have
types built-in yet (TODO for me!)
  - use typings/ to hold external declarations
agilgur5 added a commit to agilgur5/react-signature-canvas that referenced this issue Apr 20, 2022
- make heavy use of the multiple iterations on types in others' PRs,
  my own iterations on that, and then @types/react-signature-canvas
  - credit where credit is due!

- refactor init logic a bit to get around `| null` everywhere
  - kinda hacky but better than ifs or ! (non-nullable) everywhere
- use ! for canvas.getContext('2d') calls for same reason
  - c.f. microsoft/TypeScript#19229

- refactor naming everywhere to src/index.tsx

- configure tsconfig -- no need for allowJs anymore, but jsx needs to
  be set as TS is actually reading it now
- configure jest -- ts-jest needed to read TSX files

(deps): add DT typings for prop-types, react, and signature_pad
(deps): add a declaration file for trim-canvas as it doesn't have
types built-in yet (TODO for me!)
  - use typings/ to hold external declarations
agilgur5 added a commit to agilgur5/react-signature-canvas that referenced this issue Apr 20, 2022
- make heavy use of the multiple iterations on types in others' PRs,
  my own iterations on that, and then @types/react-signature-canvas
  - credit where credit is due!
  - NOTE: the naming of the props type and the class type do not include
    the word "React" unlike @types/react-signature-canvas
    - it is convention in React components to leave it out and you're
      already importing from a package with "react" in its name

- refactor init logic a bit to get around `| null` everywhere
  - kinda hacky but better than ifs or ! (non-nullable) everywhere
- use ! for canvas.getContext('2d') calls for same reason
  - c.f. microsoft/TypeScript#19229

- refactor naming everywhere to src/index.tsx

- configure tsconfig -- no need for allowJs anymore, but jsx needs to
  be set as TS is actually reading it now
- configure jest -- ts-jest needed to read TSX files

(deps): add DT typings for prop-types, react, and signature_pad
(deps): add a declaration file for trim-canvas as it doesn't have
types built-in yet (TODO for me!)
  - use typings/ to hold external declarations
agilgur5 added a commit to agilgur5/react-signature-canvas that referenced this issue Apr 20, 2022
- make heavy use of the multiple iterations on types in others' PRs,
  my own iterations on that, and then @types/react-signature-canvas
  - credit where credit is due!
  - NOTE: the naming of the props type and the class type do not include
    the word "React" unlike @types/react-signature-canvas
    - it is convention in React components to leave it out and you're
      already importing from a package with "react" in its name

- refactor init logic a bit to get around `| null` everywhere
  - kinda hacky but better than ifs or ! (non-nullable) everywhere
- use ! for canvas.getContext('2d') calls for same reason
  - c.f. microsoft/TypeScript#19229

- refactor naming everywhere to src/index.tsx

- configure tsconfig -- no need for allowJs anymore, but jsx needs to
  be set as TS is actually reading it now
- configure jest -- ts-jest needed to read TSX files

(deps): add DT typings for prop-types, react, and signature_pad
(deps): add a declaration file for trim-canvas as it doesn't have
types built-in yet (TODO for me!)
  - use typings/ to hold external declarations
@mekantor
Copy link

I did this and it is enough to suppress the error in vscode:
let context = canvasElement.getContext('2d') || new CanvasRenderingContext2D();

@snarbies
Copy link

That just throws a confusing error if the canvas can not produce a CanvasRenderingContext2D. Better off keeping it simple and idiomatic and getting a more helpful error if for some reason you can't acquire the context. (mentioned above)

// Use the ! operator to assure TypeScript that
// the value is not null or undefined
let context = canvasElement.getContext('2d')!;

@dvsmc
Copy link

dvsmc commented Feb 21, 2024

Why is this still open? The type CanvasRenderingContext2D | null is correct and using ! is definitely not a solution. You are ignoring potential errors from rendering context.

For example:

const canvas = document.createElement("canvas");
const ctx = canvas.getContext("2d");
ctx.fillRect(...);

Open your app on mac Safari 14 or 15 and you will see nice errors in your console. Safari sometimes can't create canvas element and immediately return its context, it will return null. So your drawing calls will fail and you have to add some check (usually on next tick getting ctx will succeed):

if (!ctx) {
  return;
}

And that's just only one example (usually its a bit more complex, this was just simple out of context code) I can think on top of my head! And as mentioned in previous replies - there can be even more different cases of weird devices, rendering support etc.

@snarbies
Copy link

You misunderstood my comment. The reason the API is typed the way it is and the correct way to handle the situation was already established early in the thread.

I'm not suggesting that ignoring the possibility of null is the "correct" solution. I'm suggesting that if you want to ignore the possibility of null, let context = canvasElement.getContext('2d') || new CanvasRenderingContext2D(); is not the correct way to do it, ! is.

Creating a new rendering context will still fail, but in a more confusing way whereas ! will fail in a way that is very easy to trace back to the problem.

@thw0rted
Copy link

! will fail in a way that is very easy to trace back to the problem.

I'm not sure that willfully overriding type safety makes it "easy" to trace the problem. Experienced devs might look at

const ctx = myCanvas.getContext("2d")!; // Definitely not null, trust me
ctx.fillRect(...); // why did this explode??

with a skeptical eye, but plenty of people would take that at face value. Safer to do

const ctx = myCanvas.getContext("2d");
if (!ctx) { throw new Error("Failed to get context"); } // Should never happen

One extra line of code (two if you count adding /* istanbul ignore if */) and you'll always know exactly what failed and where.

@snarbies
Copy link

I'm not suggesting that ignoring the possibility of null is the "correct" solution. I'm suggesting that if you want to ignore the possibility of null...

The original question has been asked and answered. I think this horse is dead enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests