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

Add more JSDoc types #4205

Merged
merged 10 commits into from
May 1, 2022
Merged

Add more JSDoc types #4205

merged 10 commits into from
May 1, 2022

Conversation

kungfooman
Copy link
Collaborator

I like to typify code 😅 I don't know how self-explanatory these types are, I happily answer questions.

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

@willeastcott
Copy link
Contributor

Just a nudge to say that the CI is failing...

@willeastcott
Copy link
Contributor

Can you confirm that npm run docs completes with no errors? I need to add that to the GitHub Actions....

@kungfooman
Copy link
Collaborator Author

kungfooman commented Apr 22, 2022

Can you confirm that npm run docs completes with no errors? I need to add that to the GitHub Actions....

There are eight errors, two related to this PR (should have used @callback)

Currently rewriting this PR to use @callback style instead of TS inline callback type. Remaining errors:

npm run docs

> playcanvas@1.54.0-dev docs
> jsdoc -c conf-api.json

ERROR: Unable to parse a tag's type expression for source file D:\web\playcanvas-engine-jsdoc\src\framework\app-options.js in line 85 with tag title "type" and text "{typeof Lightmapper}": Invalid type expression "typeof Lightmapper": Expected "|" but "L" found.
ERROR: Unable to parse a tag's type expression for source file D:\web\playcanvas-engine-jsdoc\src\framework\app-options.js in line 90 with tag title "type" and text "{typeof Lightmapper}": Invalid type expression "typeof Lightmapper": Expected "|" but "L" found.
ERROR: Unable to parse a tag's type expression for source file D:\web\playcanvas-engine-jsdoc\src\framework\app-options.js in line 92 with tag title "type" and text "{typeof BatchManager}": Invalid type expression "typeof BatchManager": Expected "|" but "B" found.
ERROR: Unable to parse a tag's type expression for source file D:\web\playcanvas-engine-jsdoc\src\framework\app-options.js in line 97 with tag title "type" and text "{typeof BatchManager}": Invalid type expression "typeof BatchManager": Expected "|" but "B" found.
ERROR: Unable to parse a tag's type expression for source file D:\web\playcanvas-engine-jsdoc\src\framework\app-options.js in line 99 with tag title "type" and text "{typeof XrManager}": Invalid type expression "typeof XrManager": Expected "|" but "X" found.
ERROR: Unable to parse a tag's type expression for source file D:\web\playcanvas-engine-jsdoc\src\framework\app-options.js in line 104 with tag title "type" and text "{typeof XrManager}": Invalid type expression "typeof XrManager": Expected "|" but "X" found.
Can't generate type link. Search output for {{TYPE-ERROR}}
Can't generate type link. Search output for {{TYPE-ERROR}}
npm ERR! code 1
npm ERR! path D:\web\playcanvas-engine-jsdoc
npm ERR! command failed

* @callback MakeTickCallback
* @param {number} [timestamp] - The timestamp supplied by requestAnimationFrame.
* @param {*} [frame] - XRFrame from requestAnimationFrame callback.
* @returns {void}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can delete this line right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, thank you!

@kungfooman
Copy link
Collaborator Author

I tried to look into the other errors and it boils down to typeof not being supported:

/**
* The lightmapper.
*
* @type {typeof Lightmapper}
*/
lightmapper;
/**
* The BatchManager.
*
* @type {typeof BatchManager}
*/
batchManager;
/**
* The XrManager.
*
* @type {typeof XrManager}
*/
xr;

However, I don't know a way to fix it in pure JSDoc. It seems like e.g. @lends {XrManager} is just the right solution, but VSCode/TypeScript don't support that:

microsoft/TypeScript#36771

What VSCode/TypeScript likes most seems to be: @type {{new(app: AppBase): XrManager}}

(basically for the lack of a proper ConstructorOf<XrManager> generic type)

But that trips up JSDoc again... so I feel like @lends is still the most correct way (but TypeScript doesn't supports it yet)

Doc on @lends: https://github.com/google/closure-compiler/wiki/Annotating-JavaScript-for-the-Closure-Compiler#lends-objectname

@willeastcott willeastcott merged commit 4bace98 into playcanvas:main May 1, 2022
@kungfooman kungfooman deleted the addTypes branch May 13, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants