search

jscad / OpenJSCAD.org

JSCAD is an open source set of modular, browser and command line tools for creating parametric 2D and 3D designs with JavaScript code. It provides a quick, precise and reproducible method for generating 3D models, and is especially useful for 3D printing applications.
https://openjscad.xyz/
MIT License
2.65k stars 514 forks source link

ZoomToFit Typescript issue #1280

Closed bruceborrett closed 1 month ago

bruceborrett commented 1 year ago

zoomToFit causes a typescript error saying that Geometry[] is not the correct type for the entities argument.

The return type of the function entitiesFromSolids is Geometry[] and zoomToFit is expecting this as input, so I assume this is a bug in the type declarations. If I change the zoomToFit function to expect Geometry[] then everything works as expected.

platypii commented 1 year ago

It looks like you found a bug, but your suggested fix is backwards. The return type of entitiesFromSolids should actually be an array of entities, not an array of geometries. So the bug is in entitiesFromSolids.d.ts, whereas zoomToFit looks correct.

If you look at how it's used in practice, the data type is a list of entities, which contain geometry as one of the properties:

entities

Would you be willing to make a PR with a type definition fix?

bruceborrett commented 1 year ago

Has the entity type been defined anywhere yet?

platypii commented 1 year ago

I don't believe so. The closest is the inline type definition here for zoomToFit: https://github.com/jscad/OpenJSCAD.org/blob/4c1f37271764649e192300c66d263a1fee52303c/packages/utils/regl-renderer/types/controls/orbitControls.d.ts#L100

But I think it would be nicer if we defined the Entity type explicitly.

bruceborrett commented 1 year ago

https://github.com/bruceborrett/OpenJSCAD.org/commit/5e6a792e374fed5c3c88f03b558c1231da2f206d

Is something like this ok? Im not great with typescript and dont really understand the way that you have structured the types, perhaps you can guide me if Im way off.

platypii commented 1 year ago

Yes that's looking very nice, pretty much the same way I would have done it. Notes:

Thanks for looking into this!

bruceborrett commented 1 year ago

https://github.com/jscad/OpenJSCAD.org/pull/1283

z3dev commented 3 months ago

@bruceborrett I believer those changes have been released. If satisfied then please close this issue.