search

ondras / rot.js

ROguelike Toolkit in JavaScript. Cool dungeon-related stuff, interactive manual, documentation, tests!
https://ondras.github.io/rot.js/hp/
BSD 3-Clause "New" or "Revised" License
2.32k stars 254 forks source link

Better signature for optional function parameters #204

Open scambier opened 2 years ago

scambier commented 2 years ago

Hello, I'm just trying rot.js, and I noticed that several functions have optional parameters that are typed as SomeType | null (like here).

The issue is that TypeScript does not consider those parameters as optional: you either give a value, or an explicit null: image

A more correct function signature would be

draw(x: number, y: number, ch?: string | string[], fg?: string, bg?: string) {
/** **/
}

I guess it should be an easy fix (look for all | null in the code), but as I'm literally discovering the lib, I'm not too confident on making a PR for now.

ondras commented 2 years ago

Hi @scambier,

yeah, you are completely correct. I am really not sure why we have these weird null-based signatures - perhaps to comply with existing (pre-TS) code? Passing null to an optional argument would work fine though, provided we test those values with simple if (!fg) ....

scambier commented 2 years ago

Passing null to an optional argument would work fine though, provided we test those values with simple if (!fg) ....

Sure, and you'll have to do the nullish check anyway because the default value for optional arguments is undefined. Calling draw(5, 4, "@") would be the same as calling draw(5, 4, "@", undefined, undefined).