Three.js: TS: Vector3 interface not descriptive enough to support this valid usage

[Antony74]

Description of the problem

The following code works, but TypeScript does not compile with the present Vector3.d.ts

  // vector between center of head and center of ear
  const earVector = new THREE.Vector3(1.44, 0.6, -0.3);

  const leftEar = new THREE.SphereGeometry(1, 32, 32).applyMatrix4(new THREE.Matrix4().makeScale(0.4, 0.4, 0.25));
  const rightEar = leftEar.clone();

  leftEar.translate(...earVector.toArray());
  earVector.x = -earVector.x;
  rightEar.translate(...earVector.toArray());

BUG (compile error):
Expected 3 arguments, but got 0 or more. ts(2556)

EXPECTED:
Given that the code behaves as expected, I would expect it to compile.
(I would also expect the aesthetics of this use of the spread operator to divide opinion)

Three.js version

• [ ] Dev
• [x] r116
• [ ] ...

Browser

• [x] All of them
• [ ] Chrome
• [ ] Firefox
• [ ] Internet Explorer

OS

• [x] All of them
• [ ] Windows
• [ ] macOS
• [ ] Linux
• [ ] Android
• [ ] iOS

Hardware Requirements (graphics card, VR Device, ...)

N/A

[Antony74]

This was my attempt make the toArray interface sufficiently descriptive in this respect. I am hestiatant because I do not have much experience of TypeScript definition files.

7a5c9d7

toArray( array?: [number, number, number], offset?: 0 ): [number, number, number];

[Mugen87]

TBH, such a definition will look awful when implementing it for Matrix4.toArray(). Since the toArray() methods should be equally defined for all math classes, it would be great to find a way that scales better. If possible...

[Antony74]

Thanks for the feedback!

For reference, I guess Matrix4.toArray() would look like this:

toArray(
    array?: [
        number, number, number, number,
        number, number, number, number,
        number, number, number, number,
        number, number, number, number
    ],
    offset?: 0
): [
    number, number, number, number,
    number, number, number, number,
    number, number, number, number,
    number, number, number, number
];

And at this point in time I don't think any of the generics cleverness for defining a FixedLengthArray<number, 16> really aids clarity.

And it's worth pointing out, the set method already looks like this:

/**
 * Sets all fields of this matrix.
 */
set(
    n11: number,
    n12: number,
    n13: number,
    n14: number,
    n21: number,
    n22: number,
    n23: number,
    n24: number,
    n31: number,
    n32: number,
    n33: number,
    n34: number,
    n41: number,
    n42: number,
    n43: number,
    n44: number
): Matrix4;

[Mugen87]

Urgh! Let's see if other TS devs figure out a different solution. If not, we can still make this change (and never look into the d.ts files again^^).

[gkjohnson]

From this stackoverflow answer it sounds like you could define fixed array types of a specific length and reuse them but I haven't tested it:

// ReusableTypes.d.ts
export type Length3Array = [ number, number, number ];
export type Length4Array = [ number, number, number, number ];
export type Length16Array = [ /* ... */ ];

// Vector3.d.ts
class Vector3 {

    // ...

    toArray() : Length3Array;

}

[gkjohnson]

It might also be possible to clean up the set function definition for Matrix4 using the "rest" parameters notation but again I haven't tested it:

set( ...values : Length16Array ) : Matrix4

[pailhead]

The SO answer though raises an interesting point which i don't understand :) The difference between a list and tuple. Defining these lengtX arrays would be defining common tuples that appear in vector and matrix math for example?

I wonder how many of these methods are on the receiving end. Ie. if translate() can take an array as well, would it make sense to be a list, or it'd still make sense to be this specific tuple? (if it is a tuple)

[donmccurdy]

I've used this syntax elsewhere:

export type vec2 = [number, number];

export type vec3 = [number, number, number];

export type vec4 = [number, number, number, number];

export type mat3 = [
    number, number, number,
    number, number, number,
    number, number, number,
];

export type mat4 = [
    number, number, number, number,
    number, number, number, number,
    number, number, number, number,
    number, number, number, number,
];

There is no list/tuple distinction at runtime, this is just for typechecking.

[Antony74]

I believe gkjohnson's approach of defining tuple types for re-use is a good way forwards. Were this issue assigned to me I am confident I could provide an adequate pull request.

Pailhead writes:

The SO answer though raises an interesting point which i don't understand :) The difference between a list and tuple. Defining these lengtX arrays would be defining common tuples that appear in vector and matrix math for example?

I don't believe TypeScript has list as a fundamental type. The difference between an array and a tuple is... well actually a tuple is an array, except that it has a fixed number of elements whose types are known. As such there are no tuples in JavaScript, only arrays.

I wonder how many of these methods are on the receiving end. Ie. if translate() can take an array as well, would it make sense to be a list, or it'd still make sense to be this specific tuple? (if it is a tuple)

If translate could take a Vector then there wouldn't be an issue... at least not one with a use-case. I'm not proposing an api change, just a change to the TypeScript definition files to better reflect and utilize the existing api.

gkjohnson writes

It might also be possible to clean up the set function definition for Matrix4 using the "rest" parameters notation but again I haven't tested it:

set( ...values : Length16Array ) : Matrix4

While I am deeply impressed, and never cease to be amazed at what the TypeScript type-system can do, I fear this would make the definitions file harder rather than easier to read. In particularly when entering values for a call to set the variable names that an IDE such as Visual Studio Code displays are very helpful for verifying you're actually in the row and column you believe, and therefore makes it easier to enter the correct data first time.

I am personally not persuaded of the need to implement this additional type information for matrices, as there is at present no use case. What method takes the same four, nine, or sixteen parameters as a Matrix2, Matrix3, and Matrix4? Other than another such matrix?

In terms of initializing one matrix from another, I believe, if you're being extremely conscious of performance you should not be using an intermediate array, and if you are not trying to wring every last performance improvement out the system you should use the clone method.

Therefore no use case. However, I am more than happy to defer to the judgement of others and implement the improved definitions uniformly across vectors and matrices if that is desired.

[donmccurdy]

@Antony74 it sounds like you like @gkjohnson's proposal, but want to apply it only to Vectors and not to Matrices. Is that correct?

The syntax above is one I have used with gl-matrix, see index.d.ts, where arrays are the basic unit of math operations, and used widely.

I agree the case for a special type for matrices is weaker than the case for vectors. Mostly it's just for consistency. But either way, I think this is a very minor syntax difference and should make no difference to the user's code, as long as the case you described in the original post is handled?

[Antony74]

That looks like a fairly conclusive vote in favor of consistency across vectors and matrices. Naturally my offer to work on this still stands. Unless someone else wants it, of course.