[dev-2.0] Fix optional and rest parameters in TypeScript class method declarations

[SoundOfScooting]

Addresses #7862 (fixes overloads but not missing fields or other syntax errors)

Changes:

• Adds type declaration filepaths to package.json so they can be imported.
• Fixes class method declarations not emitting optional and rest parameters.
• Fixes @chainable in class method declarations

Screenshots of the change:

PR Checklist

• [x] npm run lint passes
• [ ] Inline reference is included / updated
• [ ] Unit tests are included / updated

[welcome[bot]]

🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. And be sure to add yourself to the list of contributors on the readme page!

[SoundOfScooting]

Working on fixing more type resolution and syntax errors, plus a few reference tag corrections. I've not been organized and will have to divide all the changes into separate commits.

One question I have is what I should do about @private classes whose types are referenced in public methods - some examples in p5.d.ts are Renderer, RendererGL, Texture, FramebufferTexture, Quat, Cubic, ImageInfo[] (which is wrong and I've corrected it to ImageInfos, although still private). Since the class is not emitted it will fail to resolve the type (even after the resolution fixes), and the class's public methods are incorrectly emitted as regular functions.

For some of these I'm also wondering if they should even be private in the first place. For instance, the reference page for image() links to missing pages for private classes. Alternatively we could implement a new tag (e.g. @internal) which would hide a class from the reference but not the declaration files?

[davepagurek]

I think these are intentionally private, as "private" for us means "doesn't get its own reference page," and I think it still makes sense to treat things like Renderer, an abstract base class, don't need their own user-facing reference page even if they are mentioned by other references. Ideally the text of the description would link to the thing that returns it, even if it doesn't need a full class description.

For the .d.ts though, it sounds like we do need to export anything that is referenced by a type even if it's marked as @private to get the type checks to work. That's also kinda how FES works -- FES typechecking also compares against the real class instances if they are encountered in a type, even if they're private.

[SoundOfScooting]

Some of the private types like ImageInfos were only being used in local functions that were erroneously detected to be methods of p5, so I've currently marked them @private and removed @method. Not sure what to do about the abstract classes (I think anything private doesn't even get seen by helper.mjs?) so I'm leaving them as-is.

To fix type resolution, I have tried two potential avenues so far:

1. Emit references to p5.Type as Type instead (p5 ends up refering to a class and not the module). This will fail in global.d.ts since it's declarations aren't inside the module, so this fix should not be applied to that file, but I didn't succeed in conditionally applying it and switched to fix 2 (see below for a fix that would work though).
2. Inspired by @types/p5, in p5.d.ts change export default p5 to export = p5. I haven't really tested this for ESM imports (I don't think they'd even work to begin with since you're supposed to import from the export subpaths directly?), but unlike 1 it works correctly in non-module <script> blocks (which I've changed my game to use so I could drop its dependency on a bundler). Previously, you would have a globally declared namespace p5 that contains the p5 class as a default export, so only this would work at runtime:

let pInst: p5.default = new (p5 as unknown as typeof p5.default)(p => {...})

Another change I currently have applied is changing global.d.ts to not redeclare everything from scratch but instead copy them from the p5 import. This seems like a lot less work and bypasses the issue with fix 1. For example:

/** imagine docs here */
function describe(text: string, display?: p5.FALLBACK | p5.LABEL): void;
// -->
/** imagine docs here */
let describe: p5["describe"]; // syntax for fix 2, different for 1

This works as long as the p5 class has no accessors (which I see no way to declare inside declare global but could still be in interface Window).

There's also the issue of both p5.d.ts and core/main.d.ts declaring class p5 (the former being complete and latter incomplete). We could just hardcode main.d.ts to not declare anything, but I say we again look to @types/p5 here and have p5.d.ts declare an empty class p5 with interface p5 extends some_merged_interface {} to remove this duplication as well as align p5 method declarations with their corresponding JS files.

[davepagurek]

Hey @SoundOfScooting! Just wanted to check in again, are you still working on this and is there anything we can do to help if you're blocked?

[SoundOfScooting]

Hi @davepagurek, sorry for the slow responses. I've still been focused on other projects and never set aside time to finish working on this. If someone else wants to take lead on the typescript declarations they're welcome to. I didn't decide on the final design for the type resolution changes, but I'll push the smaller fixes I completed that may be useful.