[p5.js 2.0 Bug Report]: Typescript declarations missing overloads/optional parameters
Most appropriate sub-area of p5.js?
- [ ] Accessibility
- [ ] Color
- [ ] Core/Environment/Rendering
- [ ] Data
- [ ] DOM
- [ ] Events
- [ ] Image
- [ ] IO
- [ ] Math
- [ ] Typography
- [ ] Utilities
- [ ] WebGL
- [ ] Build process
- [ ] Unit testing
- [ ] Internationalization
- [ ] Friendly errors
- [x] Other (specify if possible)
p5.js version
v2.0.3
Web browser and version
No response
Operating system
No response
Steps to reproduce this
There are many missing overloads in the generated TypeScript definitions, most of which are functions with runtime optional parameters that are not declared as optional.
Usage of these missing overloads is an error when using "strict": true in tsconfig.json.
Some examples in p5.d.ts (not an exhaustive list):
// alpha should be optional
color(v1: number, v2: number, v3: number, alpha: number): p5.Color;
background(colorstring: string, a: number): void;
background(gray: number, a: number): void;
background(v1: number, v2: number, v3: number, a: number): void;
background(image: p5.Image, a: number): void;
clear(r: number, g: number, b: number, a: number): void;
/* correct */ fill(v1: number, v2: number, v3: number, alpha?: number): void;
fill(gray: number, alpha: number): void;
stroke(v1: number, v2: number, v3: number, alpha: number): void;
stroke(gray: number, alpha: number): void;
// options should be optional
beginClip(options: object): void;
clip(callback: Function, options: object): void;
// n should be optional
redraw(n: number): void;
// both params should be optional
erase(strengthFill: number, strengthStroke: number): void;
// these empty overloads are missing
print(): void;
fullscreen(): boolean;
// these fields are missing
width: number;
height: number;
// etc
Tested using p5 instance mode.
To actually use the generated types I had to change this part of p5/package.json.
".": "./dist/app.js",
// ->
".": {
"types": "./types/p5.d.ts",
"default": "./dist/app.js"
},
I'm compiling to an HTML file that can be opened offline with file://, but because of CORS the HTML file cannot use JS modules and I must disable the friendly error system.
To do this I've used Parcel to bundle the files with settings "outputFormat": "global", "scopeHoist": false, but it requires me to comment out a block if (typeof P5_DEV_BUILD !== 'undefined') { inside lib/p5.js (p5.min.js works fine).
This is my current working setup (suggestions welcome):
import type * as p5 from "p5";
// for some reason p5.default has different constructor signature than p5.p5
const P5 = (globalThis.p5 as any as typeof p5.default);
// this declaration is missing
(P5 as any).disableFriendlyErrors = true;
new P5(p5 => { /* ... */ });
Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, please make sure to fill out the inputs in the issue forms. Thank you!
Types are automatically generated from the inline documentation in the codebase, so these overloads or missing optionals will need to be corrected there (this means the documented function signatures are also slightly incorrect.) If anyone is interested in going through the list here (thanks for compiling it!) that would be appreciated!
I don't mind giving it a try. I'll also look for more discrepancies as the above list is not complete.
Thanks @SoundOfScooting , I have assign you this one. Thanks for the help :)
I'm starting work on this and notice some other issues.
As mentioned in the issue, TypeScript errors at import ... from 'p5' because exports in package.json doesn't specify the path to the type files. This issue also occurs inside the generated declarations themselves (for example, p5 constants don't appear to resolve).
I plan to include the following fix in my PR:
(The other files in this list don't have corresponding declaration files to point to, so I'm not sure what to do about them.)
- ".": "./dist/app.js",
+ ".": {
+ "types": "./types/p5.d.ts",
+ "default": "./dist/app.js"
+ },
- "./core": "./dist/core/main.js",
+ "./core": {
+ "types": "./types/core/main.d.ts",
+ "default": "./dist/core/main.js"
+ },
vscode(ium) also reports these syntax errors which can be made into other issues if necessary.
math.js:37:14: A rest parameter must be last in a parameter list. src:* @param {...Number} x Zero or more numbers, representing each component of the vector.structure.d.ts:181:12: '(' expected. src:function p5: typeof p5p5.Element.d.tsFunctions and parameters namedclassare marked as syntax errors (methods namedclassare fine).2d_primitives.d.ts&3d_primitives.d.tsexport default function(...)in JS generates the declarationexport default function 2/3d_primitives(...), which is marked a syntax error.- #6660 (which has had multiple PRs attempted and none merged)
Oh, it appears that I was mistaken that one of the fill overloads was correct.
I've looked into it more thoroughly. The documentation comments correctly mark parameters as optional with [], and types/color/setting.d.ts has optional parameters, but types/p5.d.ts and types/global.d.ts do not, so the issue appears to be with those portions of the generator scripts.
Thanks for narrowing it down @SoundOfScooting! Are you interested in trying to fix the optional handling in that file too? It looks like it is currently trying to be handled here (possibly the broken cases are going through a different code path?): https://github.com/processing/p5.js/blob/2606c21ef4924bd995053193c1c8b66f3e014d25/utils/helper.mjs#L310
For reference, for the documentation on the website and for FES, it happens here: https://github.com/processing/p5.js/blob/2606c21ef4924bd995053193c1c8b66f3e014d25/utils/convert.mjs#L64-L65
https://github.com/processing/p5.js/issues/6660 (which has had multiple PRs attempted and none merged)
I think none of them ended up merged because someone was working on it and we didn't want people to jump the queue, but then they later closed their PR without making revisions. So I think that one is open again to be worked on, I've marked it as Help Wanted and Good First Issue.
Okay, I've fixed the main issue of optional parameters.
Both generateFunctionDeclaration and generateMethodDeclarations call the function generateParamDeclaration, but the former is using funcDoc (from "unorganized" data) and the latter is using item (from "organized" data), which the function is not correctly written to handle.
Organized parameters have a different format than regular ones:
https://github.com/processing/p5.js/blob/2606c21ef4924bd995053193c1c8b66f3e014d25/utils/helper.mjs#L286-L290
generateTypeFromTag actually strips OptionalType, so the detection in generateParamDeclaration fails. A simple fix is to change the detection to param.optional || param.type?.type === 'OptionalType'.
hi @ksen0 is this open, should i give i a shot or not?
The main issue here of missing overloads is already fixed in my PR (#7863), but it's not merged yet as I was trying to fix more problems with the TypeScript declarations and haven't yet commited the big changes or decided on the final design. I had taken a break from working on the PR for a while but plan to resume sometime soon to finalize the changes. See also #7938.
Thanks for the update @SoundOfScooting !
Since someone is already working on it, @NalinDalal maybe you can find another open PR that does not have ongoing work yet? Thank you!
I am using p5 for the first day, so sorry, if I am wrong... Beside missing fields like disableFriendlyErrors, mouseX, mouseY, ... I found, that compiler complains about:
- using keywod "class" as variable name at several places in p5.Element.d.ts file,
- using variable name beginning with digit in:
-- 2d_primitives.d.ts (
export default function 2d_primitives(p5: any, fn: any): void;), -- 3d_primitives.d.ts (export default function 3d_primitives(p5: any, fn: any): void;)