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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent PlaneBufferGeometry rename warning #221

Closed

Conversation

wmcmurray
Copy link
Contributor

My suggestion to fix #192 (comment)
(unwanted warning in console : THREE.PlaneBufferGeometry has been renamed to THREE.PlaneGeometry.)

I only tested it with r145, but according to your package.json, troika-three-text supports three.js from r103, so I used this CDN link as a reference to help determine what checks to perform.

Feel free to use this approach or not 馃檪

Thanks !

@Console-buche
Copy link

Console-buche commented Oct 13, 2022

Well done !
Oh man I was about to publish my own PR for this as I stumbled on the issue myself, then just saw yours.
I like the compat function, nice touch. 馃憤

Btw : there's a varity of other files that reference PlaneBufferGeometry. I think we could safely extend your proposal to them.

@CodyJasonBennett
Copy link
Contributor

Following this up with #225. Three deprecation policy means that PlaneBufferGeometry will be removed in r154, or 10 releases after deprecation, so there's not an effective way to shim compatibility here.

@wmcmurray
Copy link
Contributor Author

Nice catch @CodyJasonBennett !
import * as THREE from 'three' would make it possible (but it would be terrrrrible for three-shaking).
I hope Troika is willing to bump the min Threejs revision ! 馃馃徏

@CodyJasonBennett
Copy link
Contributor

CodyJasonBennett commented Nov 14, 2022

Attempting to import a non-existent module through named imports or namespaces will break bundlers (deprecated is effectively removed since three intends to delete in 10 releases). You'd need a dynamic expression that cannot be statically analyzed that is both safe at run-time (despite tree shaking) and not effectful. We employ a similar workaround in @react-three/fiber for progressive support of THREE.ColorManagement, but can't do the same here despite singleton callbacks.

@lojjic
Copy link
Collaborator

lojjic commented Nov 14, 2022

Thanks @wmcmurray for this, and apologies for the silence on my part. I do think bumping the minimum three version is appropriate at this point, so I'm going to close this PR in favor of #225.

@lojjic lojjic closed this Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to latest Three
4 participants