Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mergeBufferGeometries give error for empty array #331

Open
sylee957 opened this issue Jan 3, 2024 · 2 comments
Open

mergeBufferGeometries give error for empty array #331

sylee957 opened this issue Jan 3, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@sylee957
Copy link

sylee957 commented Jan 3, 2024

  • three version: 0.160
  • @types/three version:
  • three-stdlib version:

Problem description:

mergeBufferGeometries gives error Uncaught TypeError: Cannot read properties of undefined (reading 'index')
It is from some sloppy list access

https://github.com/pmndrs/three-stdlib/blame/45e22a1b636241147934e8bd9f22d506ccfc3cf0/src/utils/BufferGeometryUtils.ts#L27

which also exists in three.js upstream

https://github.com/mrdoob/three.js/blob/de6dd45d7e5aa58fed0fbc1dbe53def3402b39cc/examples/jsm/utils/BufferGeometryUtils.js#L109C20-L109C51

Relevant code:

Suggested solution:

I generally would return some reasonable identity object for such cases than raising sloppy errors.
Maybe null can be reasonable.

Or otherwise, it may be useful to denote the types as [BufferGeometry, ...BufferGeometry[]] instead of BufferGeometry[],
where typescript can just warn users that users should pass array with at least one element.

@sylee957 sylee957 added the bug Something isn't working label Jan 3, 2024
@CodyJasonBennett
Copy link
Member

three.js will tell you to validate your inputs, so this is working as intended there. I do agree with your solution, but would prefer to not change the type (rather than adding a runtime guard).

@CodyJasonBennett
Copy link
Member

Related: mrdoob/three.js#27509.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants