0xs1r4t / sirat.xyz

my website. under construction 🔧
https://sirat.xyz
0 stars 0 forks source link

[@types/p5] Vector has incorrect return types #182

Closed 0xs1r4t closed 1 week ago

0xs1r4t commented 4 weeks ago

References this issue DefinitelyTyped/DefinitelyTyped#69284 opened earlier this year. There's no fix in place so issues come up whenever the canvas is built and deployed.

A temporary fix I found in my local environment was to just go to the node_modules/p5/src/math/p5.Vector.d.ts file and manually change the types of mult from void to Vector.

I will need to find a more permanent fix for when I merge this with the main branch. Until then, this is what the canvas looks like.

0xs1r4t commented 3 weeks ago

It took me a while to understand the code implemented in the library and found that after last year's changes in #https://github.com/DefinitelyTyped/DefinitelyTyped/pull/66294, there are 2 separate implementations of the 4 different methods of methods used for p5.Vector.mult().

The first method has a Vector return type variable in both cases, but the other three methods have a void return type in their static versions since they all have a target variable as an optional parameter in these methods that the result of the operation can be stored in.

When not mentioned, it should default to the non-static versions of the same methods, but that isn't the case. I will investigate further to find out why. The changes implemented in the mentioned PR were blind-reviewed since there was no one to approve of them from the p5 types team, so that may be why this discrepency exists.

0xs1r4t commented 3 weeks ago

The above issue has been fixed in p5-types #https://github.com/p5-types/p5.ts/issues/123 via a fix in p5.js #https://github.com/processing/p5.js/pull/6831, but this change hasn't been pushed to @types/p5.

Previously, I was using the mult() function like this:

// incorrect version

velocity = Vector.mult(distance, easing);
// this should return velocity with a vector type
// but gives an error because the return type is void

This version expects an optional target variable to be declared to store the value in as the third parameter. Whether a user adds this target variable or not, the return type of this mult() function remains void instead of Vector.

I converted it to the statement below, since there is another scalar multiplication function that expects the output to be a Vector when called on the vector value itself.

// correct version

velocity = distance.mult(ease);
// this returns a vector and stores it in velocity
// the result after scalar multiplication is:
// [velocity.x, velocity.y] = [easing * distance.x, easing * distance.y]
0xs1r4t commented 1 week ago

🌈 Fixed in #184