paperjs / paper.js

The Swiss Army Knife of Vector Graphics Scripting – Scriptographer ported to JavaScript and the browser, using HTML5 Canvas. Created by @lehni & @puckey
http://paperjs.org
Other
14.45k stars 1.22k forks source link

[typescript] Raster.onLoad is declared non-nullable #1664

Closed dobesv closed 5 years ago

dobesv commented 5 years ago

Steps to reproduce

Set a raster's onLoad handler to null in typescript code, with "strictNullChecks": true set in tsconfig.json.

Link to reproduction test-case

https://github.com/dobesv/paperjs-ts-repro/blob/master/raster-onload-null.ts

Expected result

Assigning onLoad to null type checks without errors, as this is a valid operation which removes the onLoad handler from the Raster.

Additional information

"strictNullChecks": true is a fairly popular setting in typescript to help detect possible runtime errors involving null or undefined values.

However, with this setting enabled, you cannot set the onLoad handler to null on Raster without getting a typescript error.

sasensi commented 5 years ago

@dobesv, thank you for the report.

@lehni, about this one, an easy and quick solution would be to "mark" all settable properties as "nullable". property: Type => property: Type | null But I wonder if there are settable properties that are not nullable in the library ? And if there are, do we want to provide this extra information in the documentation ?

sasensi commented 5 years ago

This also relates to the problem evoked in https://github.com/paperjs/paper.js/issues/1665 in the sense that declaring an object property nullable means that the compiler will complain when trying to access the keys of this object because it might be null. For example: if we have:

interface Item {
    fillColor: Color | null
}
const item:Item

Doing item.fillColor.red will throw an error in strictNullChecks mode because item.fillColor might be null. So, you will have to do item.fillColor && item.fillColor.red or something like this anyway. @dobesv, what do you think ?

dobesv commented 5 years ago

Well, specifically for this property, onLoad, it makes sense to allow null since it could be null when read or written. For other properties, ideally the types would be as accurate as possible. For fillColor, I think it actually can be null if the shape is not filled. But there might be properties where even if you can assign it to null, the null is translated into some kind of non-null value.