-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Improve ModelComponent typings #4002
Conversation
/* eslint-disable jsdoc/check-types */ | ||
/** | ||
* @type {Object.<string, number>} | ||
* @private | ||
*/ | ||
_mapping = {}; | ||
/* eslint-enable jsdoc/check-types */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/* eslint-disable jsdoc/check-types */ | |
/** | |
* @type {Object.<string, number>} | |
* @private | |
*/ | |
_mapping = {}; | |
/* eslint-enable jsdoc/check-types */ | |
/** | |
* @type {Record<string, number>} | |
* @private | |
*/ | |
_mapping = {}; |
It takes two little changes in node_modules\@playcanvas\jsdoc-template\publish.js
to make this work without extra steps to ignore eslint
and the resulting types would also look less verbose/strange:
/**
* A dictionary that holds material overrides for each mesh instance. Only applies to model
* components of type 'asset'. The mapping contains pairs of mesh instance index - material
* asset id.
*
* @type {Record<string, number>}
*/
set mapping(arg: Record<string, number>);
get mapping(): Record<string, number>;
vs.
/**
* A dictionary that holds material overrides for each mesh instance. Only applies to model
* components of type 'asset'. The mapping contains pairs of mesh instance index - material
* asset id.
*
* @type {Object.<string, number>}
*/
set mapping(arg: {
[x: string]: number;
});
get mapping(): {
[x: string]: number;
};
In node_modules\@playcanvas\jsdoc-template\publish.js
you can replace this:
// Check for key-value (aka dictionary or index signature) types
match = /^Object.<string,\s*(.*)>$/i.exec(name);
if (match) {
name = match[1];
with:
// Check for key-value (aka dictionary or index signature) types
match = /^(Object|Record).<string,\s*(.*)>$/i.exec(name);
if (match) {
name = match[2]; // changed from [1] to [2] because extra group
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very interesting. However, there are actually several instances of this across the engine codebase now, so rather than just fix this one up here, I think I will open an issue on this and we can address it separately. Thanks for the great suggestion!!
Improve
ModelComponent
typings.I confirm I have read the contributing guidelines and signed the Contributor License Agreement.