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: correctly import webxr types #223

Merged
merged 1 commit into from
May 29, 2022
Merged

fix: correctly import webxr types #223

merged 1 commit into from
May 29, 2022

Conversation

Methuselah96
Copy link
Contributor

@joshuaellis joshuaellis merged commit 13ae1c3 into master May 29, 2022
@joshuaellis joshuaellis deleted the fix-webxr-imports branch May 29, 2022 17:30
@CodyJasonBennett
Copy link
Contributor

CodyJasonBennett commented May 29, 2022

Would this have to be mirrored in other classes that make references to WebXR types like WebXRController?

What would this look like for user-facing code?

@Methuselah96
Copy link
Contributor Author

No, you only have to reference it in one file and then it's globally available. Users won't have to do anything different as far as I'm aware.

@CodyJasonBennett
Copy link
Contributor

CodyJasonBennett commented Jun 21, 2022

Seems this was quite the breaking change since projects using types <141 were importing from three, and in order to support ambient types with 141 they'd require a major bump to hike the peer dep, or also include @types/webxr likewise.

Any chance we can revert or export @types/webxr in a patch release?

@joshuaellis
Copy link
Member

Sorry, I'm not sure what the problem you're trying to say is.

It sounds like to me people are installing the latest version of these types with the non equivalent version of three e.g r139, if this is the case this is an incorrect usage of the library.

Let me know if I've misunderstood?

@CodyJasonBennett
Copy link
Contributor

@types/three no longer exports WebXR types since r141. This is a breaking change.

@joshuaellis
Copy link
Member

Right, yeah if you can open a new issue for this, we can track it better there.

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.

3 participants