three-ts-types
three-ts-types copied to clipboard
fix wrong typed `Euler.toArray()`, add `EulerOrder` for type safety
Why
array?: number[] of the existing Euler.toArray() method is a typing that does not fit the implementation of the method.
This is the original d.ts code.
toArray(array?: number[], offset?: number): number[];
This is the implementation of three.js Euler.toArray()
toArray( array = [], offset = 0 ) {
array[ offset ] = this._x;
array[ offset + 1 ] = this._y;
array[ offset + 2 ] = this._z;
array[ offset + 3 ] = this._order; //`this._order` type is string so array should not be number[]
return array; //return value is the same as the above.
}
What
The array parameter of Euler.toArray() was changed to array?: any[] and the return type was also matched.
toArray(array?: any[], offset?: number): any[];
In addition, the EulerOrder type was added for type safety of the order and Euler.fromArray() parameter type was changed more correctly
export type EulerOrder = 'XYZ' | 'YXZ' | 'ZXY' | 'ZYX' | 'YZX' | 'XZY';
fromArray(xyzo: [number, number, number, EulerOrder?, ...any[]]): Euler;
Checklist
- [x] Added myself to contributors table
- [x] Ready to be merged
I'm not sure changing the array to be any is correct... 🤔
It is not, but neither is number[].
// https://github.com/mrdoob/three.js/blob/dev/src/math/Euler.js#L267
fromArray( array ) {
this._x = array[ 0 ];
this._y = array[ 1 ];
this._z = array[ 2 ];
if ( array[ 3 ] !== undefined ) this._order = array[ 3 ];
this._onChangeCallback();
return this;
}
toArray( array = [], offset = 0 ) {
array[ offset ] = this._x;
array[ offset + 1 ] = this._y;
array[ offset + 2 ] = this._z;
array[ offset + 3 ] = this._order;
return array;
}
Is this because order could be a string? Or.... it'd be helpful if we had a basic example to review how this is a problem
Isn't the return type something like [number, number, number, string]? Or does the offset arg break that?
Isn't the return type something like
[number, number, number, string]? Or does theoffsetarg break that?
This code is not easy to typed because of offset.
I think realistically it has to be Array<number | string | undefined> in an ideal world it would have been [x:number, y:number, z:number, offset:string], but thats not possible.
Take this example:
toArray( array = [], offset = 0 ) {
array[ offset ] = this._x;
array[ offset + 1 ] = this._y;
array[ offset + 2 ] = this._z;
array[ offset + 3 ] = this._order;
return array;
}
because you can define an offset we have no idea at what index each value type would be so the forth argument could be a number because you set the offset as 1, in addition to this if you passed an empty array but set the offset to 1 then index 0 would be undefined.
I solved the conflict and modified the any[] to Array<number | string | undefined>