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

fix: add import to lensflare from three addons #585

Open
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

alvarosabu
Copy link
Member

No description provided.

@alvarosabu alvarosabu requested a review from andretchen0 January 8, 2025 16:51
@alvarosabu alvarosabu self-assigned this Jan 8, 2025
@alvarosabu alvarosabu added the bug Something isn't working label Jan 8, 2025
@andretchen0
Copy link
Contributor

andretchen0 commented Jan 9, 2025

I created this issue on three-stdlib:

pmndrs/three-stdlib#396

Until three-stdlib is fixed, how about sticking Lensflare.js from /examples/jsm in Cientos? That avoids build errors and works as expected.

@CodyJasonBennett
Copy link

CodyJasonBennett commented Jan 11, 2025

I fixed the upstream issue but would suggest importing from three/addons if possible since this is an ESM-only package. The benefits of three-stdlib are to support CJS builds and offer backwards compatibility. In time, I hope to remove the need for the package entirely. three/addons (since r158) was a first step towards that.

three-stdlib does tree shake completely, where three/addons or three/examples/jsm/** does not. You can mitigate that with the latter (three/addons is impossible to tree-shake unfortunately; needs mrdoob/three.js#26912) by emitting separate JS files with preserveModules or similar and package.sideEffects = false, so bundlers won't include files unless referenced in code.

Importing from three/examples/jsm/** creates hazards since some bundlers require explicit file extensions, which can be lost during transpilation, as it is technically source code (e.g. mrdoob/three.js#24593).

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

Successfully merging this pull request may close these issues.

3 participants