fix(web): enforce asset compilation at build-time and fix runtime 404s#219
fix(web): enforce asset compilation at build-time and fix runtime 404s#219Jberlinsky wants to merge 2 commits into
Conversation
- Use a sentinel //go:embed for assets/main.js to force compile-time failure if UI is not built - Update hasWebAssets() to verify entry point existence, preventing 404s on empty asset dirs - Update install.md to recommend 'make all' for automated web build and installation - Fix web_test.go to provision dummy assets for stable test execution
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the web asset verification process by introducing a compile-time sentinel to ensure assets are built before embedding and updating the hasWebAssets check to verify the presence of the core entry point. Documentation and tests have been updated to reflect these changes. Review feedback highlights a misleading instruction in the installation guide regarding CLI-only builds and recommends adding error handling for file operations within the test suite.
| os.MkdirAll(assetsDir, 0755) | ||
| os.WriteFile(filepath.Join(assetsDir, "main.js"), []byte("// test"), 0644) |
There was a problem hiding this comment.
Error handling is missing for these file operations. Using require.NoError ensures the test fails immediately with a clear message if the environment setup fails.
| os.MkdirAll(assetsDir, 0755) | |
| os.WriteFile(filepath.Join(assetsDir, "main.js"), []byte("// test"), 0644) | |
| require.NoError(t, os.MkdirAll(assetsDir, 0755)) | |
| require.NoError(t, os.WriteFile(filepath.Join(assetsDir, "main.js"), []byte("// test"), 0644)) |
| os.MkdirAll(assetsDir, 0755) | ||
| os.WriteFile(filepath.Join(assetsDir, "main.js"), []byte("// test"), 0644) |
There was a problem hiding this comment.
Error handling is missing for these file operations. Using require.NoError ensures the test fails immediately with a clear message if the environment setup fails.
| os.MkdirAll(assetsDir, 0755) | |
| os.WriteFile(filepath.Join(assetsDir, "main.js"), []byte("// test"), 0644) | |
| require.NoError(t, os.MkdirAll(assetsDir, 0755)) | |
| require.NoError(t, os.WriteFile(filepath.Join(assetsDir, "main.js"), []byte("// test"), 0644)) |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 participant
This PR ensures that Scion binaries built from source have correctly compiled web assets embedded, or fail at compile-time with a helpful error message. It also fixes a runtime bug where the server would return a 404 error if the web directory was present but empty.
Key Changes
//go:embeddirective inweb/embed.goforassets/main.js. Building the project now fails immediately if the web assets haven't been built (make web), preventing the creation of "broken" binaries.hasWebAssets()inpkg/hub/web.goto verify the presence ofassets/main.jsbefore attempting to serve the SPA shell. This allows the server to gracefully fall back to a "Web UI Not Available" page even if an empty assets directory is provided via--web-assets-dir.install.md) to recommendmake all, which handles both the web build and installation to~/.local/binin a single command.pkg/hub/web_test.goto automatically provision dummy assets for web tests, ensuring CI remains green even when running without a full frontend build (e.g., using-tags no_embed_web).Why this is needed
Previously, because
web/dist/clientcontained a.gitkeepfile, the Go compiler would successfully embed an effectively empty directory. At runtime, the server would find this directory, serve the main HTML shell, and then fail to load the actual JavaScript, leading to a confusing 404 error (visible only in the browser console) and a blank page for users building from source.Testing Performed