-
Notifications
You must be signed in to change notification settings - Fork 76
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
chore(hub-embed): auto-build & embed hub instead of pulling from releases #1966
Conversation
Deploying rivet-hub with
|
Latest commit: |
4bce14d
|
Status: | ✅ Deploy successful! |
Preview URL: | https://27662316.rivet-hub-7jb.pages.dev |
Branch Preview URL: | https://01-29-chore-hub-embed-auto-b.rivet-hub-7jb.pages.dev |
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.
PR Summary
This PR modifies the hub-embed package to build and embed the hub UI directly from source during compilation, replacing the previous approach of pulling from releases.
- Changed
packages/common/hub-embed/build.rs
to build hub UI using yarn/turbo instead of downloading pre-built releases - Added new
build:embedded
task infrontend/apps/hub/turbo.json
andpackage.json
with/ui/
base path - Updated placeholder from
%VITE_APP_API_URL%
to__VITE_APP_API_URL__
inpackages/api/ui/src/route.rs
for API URL injection - Security concern: Removed
.env*
from.gitignore
which could expose sensitive information - Added
fs_extra
build dependency inpackages/common/hub-embed/Cargo.toml
for directory operations
6 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile
@@ -61,7 +61,6 @@ tests/basic-game/.env | |||
# Added by cargo | |||
|
|||
/target |
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.
style: Duplicate /target entry - already covered by **/target on line 48
/target |
@@ -61,7 +61,6 @@ tests/basic-game/.env | |||
# Added by cargo | |||
|
|||
/target | |||
.env* | |||
.yarn/cache | |||
.yarn/install-state.gz | |||
.turbo |
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.
style: Duplicate .turbo entry - already defined on line 56
.turbo |
let project_root = PathBuf::from(manifest_dir.clone()).join("../../.."); | ||
let hub_path = project_root.join("frontend/apps/hub"); |
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.
logic: Path traversal could fail if manifest_dir is at root. Consider using canonicalize() to resolve the absolute path.
let dist_path = hub_path.join("dist"); | ||
fs::create_dir_all(out_dir)?; | ||
fs_extra::dir::copy( | ||
dist_path, | ||
out_dir, | ||
&fs_extra::dir::CopyOptions::new().overwrite(true), | ||
)?; |
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.
logic: No check if dist_path exists before copying. Build could silently fail if dist directory wasn't created.
let dist_path = hub_path.join("dist"); | |
fs::create_dir_all(out_dir)?; | |
fs_extra::dir::copy( | |
dist_path, | |
out_dir, | |
&fs_extra::dir::CopyOptions::new().overwrite(true), | |
)?; | |
let dist_path = hub_path.join("dist"); | |
if !dist_path.exists() { | |
return Err("Build failed: dist directory not created".into()); | |
} | |
fs::create_dir_all(out_dir)?; | |
fs_extra::dir::copy( | |
dist_path, | |
out_dir, | |
&fs_extra::dir::CopyOptions::new().overwrite(true), | |
)?; |
Deploying rivet with
|
Latest commit: |
4bce14d
|
Status: | ✅ Deploy successful! |
Preview URL: | https://025b14f6.rivet.pages.dev |
Branch Preview URL: | https://01-29-chore-hub-embed-auto-b.rivet.pages.dev |
Merge activity
|
…ases (#1966) <!-- Please make sure there is an issue that this PR is correlated to. --> ## Changes <!-- If there are frontend changes, please include screenshots. -->
Changes