Skip to content

refactor: Change three to a peer dependency#406

Merged
ShrimpCryptid merged 1 commit into
mainfrom
refactor/three-as-peer-dependency
May 18, 2026
Merged

refactor: Change three to a peer dependency#406
ShrimpCryptid merged 1 commit into
mainfrom
refactor/three-as-peer-dependency

Conversation

@ShrimpCryptid
Copy link
Copy Markdown
Contributor

@ShrimpCryptid ShrimpCryptid commented May 8, 2026

Problem

I ran into a bug in TFE where 3D views would fail, but only in built files. I eventually traced this to changes made in #395, and discovered that it was caused by having conflicting versions of three installed in TFE vs. vole-core.

Changing three to a peer dependency in vole-core would help prevent this in the future, so that client libraries only have one version of three installed.

Couple questions:

  • Should we set a maximum upper bound for what version of three we support, in case there are breaking changes on a minor version in the future?

@ShrimpCryptid ShrimpCryptid requested review from frasercl and toloudis May 8, 2026 18:31
@ShrimpCryptid ShrimpCryptid self-assigned this May 8, 2026
@ShrimpCryptid ShrimpCryptid marked this pull request as ready for review May 8, 2026 18:38
@ShrimpCryptid ShrimpCryptid requested a review from a team as a code owner May 8, 2026 18:38
Copy link
Copy Markdown
Contributor

@toloudis toloudis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Copy Markdown
Collaborator

@frasercl frasercl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left and then deleted a comment thinking this was TFE, about how I'd discovered that my understanding of peerDependencies was wrong and that library code needed to peer-depend on packages that provided members of its public interface rather than downstream client packages. You were clearly one step ahead of me here.

I was thrown off by the fact that vole-app currently peer-depends on three which it shouldn't have to do... but it shouldn't have to depend on three at all after #405 merges.

@ShrimpCryptid ShrimpCryptid merged commit cacdeaf into main May 18, 2026
3 checks passed
@ShrimpCryptid ShrimpCryptid deleted the refactor/three-as-peer-dependency branch May 18, 2026 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants