search

gkjohnson / three-mesh-bvh

A BVH implementation to speed up raycasting and enable spatial queries against three.js meshes.
https://gkjohnson.github.io/three-mesh-bvh/example/bundle/raycast.html
MIT License
2.5k stars 259 forks source link

TS type augmentations don't work w/ "moduleResolution": "node16" #682

Closed fabis94 closed 1 month ago

fabis94 commented 3 months ago

Describe the bug

This package augment's some of three.js' types, but it doesn't do it correctly, at least according to modern Node/TypeScript module resolution logic.

Example: https://github.com/gkjohnson/three-mesh-bvh/blob/4a59ceaa3495c1b6f371781e020105fe47cadfc1/src/index.d.ts#L210-L222

The three.js module path is lacking a file extension. Once I added a file extension to those declare module XX import paths manually inside of node_modules, TS picked up on the types and they started working.

More info from TS docs: https://www.typescriptlang.org/tsconfig/#moduleResolution

To Reproduce

Install three.js & three-mesh-bvh in a TS project w/ modern Node module & moduleResolution settings (node16 or nodenext). The three-mesh-bvh type augmentations will not be applied.

Expected behavior

The package should define TS types correctly in accordance to modern Node/TS module resolution logic.

Platform:

gkjohnson commented 3 months ago

Should this line

declare module 'three/src/core/BufferGeometry'

be changed to this?

declare module 'three'

Unfortunately I'm not a typescript expert so please feel free to make a PR if you have a fix.

fabis94 commented 3 months ago

@gkjohnson You'd just need to add file extensions to those relative module specifiers: 'three/src/core/BufferGeometry.js' and 'three/src/core/Raycaster.js' respectively

The reason for this is that native ESM support in browsers & node requires file extensions in ESM import paths. Presumably, because in the browser you can't really guess the extensions like you could on a local file system. Since all of the imports are network resources you have to know the full URL.

gkjohnson commented 3 months ago

Is it possible to use declare module 'three', instead, and put both class modifications in the same block? That would be nicer, I think. If you could test and make a PR that would be much appreciated.