Skip to content

Last two type issues (scroll view component + core/string fromCodePoint) #7616

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

Closed
kungfooman opened this issue Apr 26, 2025 · 3 comments · Fixed by #7618
Closed

Last two type issues (scroll view component + core/string fromCodePoint) #7616

kungfooman opened this issue Apr 26, 2025 · 3 comments · Fixed by #7618

Comments

@kungfooman
Copy link
Collaborator

/**
* @param {string} onOrOff - 'on' or 'off'.
* @param {ScrollViewComponentSystem} system - The ComponentSystem that
* created this Component.
* @private
*/
_toggleLifecycleListeners(onOrOff) {
this[onOrOff]('set_horizontal', this._onSetHorizontalScrollingEnabled, this);
this[onOrOff]('set_vertical', this._onSetVerticalScrollingEnabled, this);
this.entity[onOrOff]('element:add', this._onElementComponentAdd, this);
}

(system isn't an argument)

engine/src/core/string.js

Lines 218 to 238 in d076f26

/**
* Get the string for a given code point or set of code points. Polyfill for
* [`fromCodePoint`]{@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/fromCodePoint}.
*
* @param {...number} args - The code points to convert to a string.
* @returns {string} The converted string.
*/
fromCodePoint(/* ...args */) {
const chars = [];
let current;
let codePoint;
let units;
for (let i = 0; i < arguments.length; ++i) {
current = Number(arguments[i]);
codePoint = current - 0x10000;
units = current > 0xFFFF ? [(codePoint >> 10) + 0xD800, (codePoint % 0x400) + 0xDC00] : [current];
chars.push(String.fromCharCode.apply(null, units));
}
return chars.join('');
}
};

(still just accessing arguments)

I've seen a lot of types being fixed recently, so it would be nice if they could all be fixed (getting rid of warning messages) 💯

@willeastcott
Copy link
Contributor

So I have a PR for this...but I'm curious, why do you say these are the 'last two'? I think there are a large number of type warnings/errors still reported by Intellisense across the codebase.

@kungfooman
Copy link
Collaborator Author

but I'm curious, why do you say these are the 'last two'?

Good question! In RTI.js, I have to loop over JSDoc's documented parameters to check for their actual existence. If a parameter doesn't exist, I log a warning and those were the last two warnings that appeared (so I just meant to refer to a proper mapping from JSDoc to JS of this style).

@willeastcott
Copy link
Contributor

Ah, ok, that makes sense! Thanks so much for reporting these problems. 🙌

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 a pull request may close this issue.

2 participants