gkjohnson / three-bvh-csg

A flexible, memory compact, fast and dynamic CSG implementation on top of three-mesh-bvh
MIT License
552 stars 45 forks source link

Library build fails due to errors in index.d.ts #82

Closed RobertS21 closed 1 year ago

RobertS21 commented 1 year ago

Hi everyone! I'm using the three-mesh-csg inside of the library, that is written in Angular 10. With the recent addition of typescript types it stopped building due to errors in the index.d.ts file of three-bvh-csg. There are two main issues:

  1. Generic type 'Array<T>' requires 1 type argument(s). ts(2314) - The "Array" keyword, which is treated as a generic type and requires at least one type argument. I haven't delved deep into the implementation of each particular method, but unless it is specified differently, the fix would be to either add type to the array generic, at least for now, so that typescript will be okay with that, or to add the target type, that the method related to this issue should return.

    
    export class TriangleIntersectionSets {
    
    addTriangleIntersection( ia: number, tribA: Triangle, ib: number, triB: Triangle ): void;
    getTrianglesAsArray( id?: number ): Array;
    getTriangleIndices(): Array;
    getIntersectionIndices( id: number );
    getIntersectionsAsArray( id?: number, id2: number ): Array;

}


2. `A required parameter cannot follow an optional parameter. ts(1016)` For the declaration of `getIntersectionsAsArray( id?: number, id2: number ): Array` the error is concerning the `id2` argument, which can't be required. Looking at the implementation of this method, this argument could be also marked as optional.
gkjohnson commented 1 year ago

Hi @RobertS21! Would you like to make a PR to fix the issues? The npm run lint command runs the following tsc line:

tsc -p tsconfig.json --noEmit

Perhaps the typescript check can be adjusted to make sure we catch these issues, as well?

Thank you!

RobertS21 commented 1 year ago

I'm afraid that my knowledge of tsconfig is too small to propose some solution that would catch errors like this. I've tested the above mentioned method and there were no issues found. I've also once again checked logs and found out, that the errors were thrown by ngc compiler, not tsc. That said, it could probably be an Angular integration of three-bvh-csg issue.

gkjohnson commented 1 year ago

Interesting - could it be a difference in Typescript version, as well?

RobertS21 commented 1 year ago

Hmm... this is very likely. I've been using Typescript 4.0.6 with Angular 10. Tested it on some other branch with Angular 13 and Typescript 4.6.4 and error is not thrown despite not having the types defined. I'd have to read into Typescript patch notes between those versions.

gkjohnson commented 1 year ago

Great thanks for checking. The more explicit types are better, anyway so we'll get those merged!

gkjohnson commented 1 year ago

Closed via #84