-
Notifications
You must be signed in to change notification settings - Fork 255
Move 3D module to separate @opentui/3d package #410
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR successfully extracts the 3D rendering module from @opentui/core into a separate @opentui/3d package, making the core package more lightweight. Users who don't need 3D features won't have three, planck, rapier, and bun-webgpu installed as optional dependencies.
Key changes:
- Created new @opentui/3d package with 3D-specific functionality
- Moved optional 3D dependencies (three, planck, rapier, bun-webgpu) from core to 3D package
- Updated imports across 10+ demo files to use @opentui/3d
- Removed the
./3dexport from @opentui/core package.json
Reviewed changes
Copilot reviewed 18 out of 30 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/3d/package.json | New package definition with 3D dependencies and exports |
| packages/3d/tsconfig.json | TypeScript configuration for the 3D package |
| packages/3d/src/index.ts | Main export file for 3D module, re-exports THREE and all 3D functionality |
| packages/3d/src/*.ts | Moved 3D rendering code (WGPURenderer, TextureUtils, SpriteUtils, etc.) |
| packages/3d/src/animation/*.ts | Moved sprite animation and particle system implementations |
| packages/3d/src/physics/*.ts | Moved physics adapter implementations for Rapier and Planck |
| packages/3d/src/shaders/*.wgsl | Moved WGSL shader code for GPU compute operations |
| packages/core/package.json | Removed 3D dependencies, added @opentui/3d as devDependency, removed ./3d export |
| packages/core/src/3d.ts | Deleted file that previously re-exported 3D module |
| packages/core/src/examples/*.ts | Updated imports to use @opentui/3d instead of relative paths |
| packages/core/src/benchmark/*.ts | Updated imports to use @opentui/3d |
| bun.lock | Updated lockfile with new package structure and dependency changes |
Comments suppressed due to low confidence (1)
packages/3d/src/index.ts:1
- Exporting THREE as a namespace from the 3D package creates an inconsistent API pattern. The package is re-exporting the entire "three" library, which:
- Makes it unclear whether consumers should import THREE from "@opentui/3d" or directly from "three"
- Creates a dependency on the exact version of three (0.177.0) specified in this package
- May cause version conflicts if a consumer also has "three" as a dependency with a different version
This is evidenced by the inconsistent usage in the demo files, where some import THREE from "@opentui/3d" while others import types directly from "three".
Consider either:
- Removing this re-export and having consumers import "three" directly
- Documenting clearly that consumers should always use the THREE export from this package
- Only exporting the specific THREE types that are part of the public API
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Could it be that the prebuilt examples now don't contain the 3d examples anymore? |
|
Yeah. Then it's better to keep examples in core. What is the use case for building the examples? |
|
Letting users see what it can do without having to build it themselves and to provide debug tools like the now built in key handling debug demo. |
89445e0 to
6247b6a
Compare
- Move examples back from packages/3d/examples to packages/core/src/examples - Update imports to use @opentui/3d instead of relative ../src paths - Add @opentui/3d as workspace dev dependency in core - Update CI to build 3d package before core - Update root build/publish scripts to include 3d package - Remove 3d.ts re-export and 3d entry point from core build
6247b6a to
e70bef8
Compare
Extracts the 3D rendering module from @opentui/core into a separate @opentui/3d package. This keeps the core package lightweight - users who don't need 3D features won't have three, planck, rapier, and bun-webgpu installed as optional dependencies.
Note: build and publish scripts were copied from other packages. These could be simplified in a future PR for all packages.
Fix #408