Repository navigation
DEV: Validate packaged frontend assets before release - #268
Merged
Merged
Conversation
davidtaylorhq
force-pushed
the
asset-packaging
branch
from
August 25, 2026 08:37
c8aead3 to
00d8be6
Compare
davidtaylorhq
force-pushed
the
asset-packaging
branch
2 times, most recently
from
August 25, 2026 18:23
3417eb6 to
3400369
Compare
davidtaylorhq
force-pushed
the
asset-packaging
branch
3 times, most recently
from
September 11, 2026 09:26
2042d35 to
74ccc1c
Compare
The gemspec shipped whatever `git ls-files` returned minus `website` and `bin`, so the published gem carried the client app sources, the test suite, the CI workflows and the gemfiles. It also missed two files it did need. `assets/manifest.json` and `assets/logster-config.json` arrived with the Vite build and are ignored by git, so `git ls-files` never saw them and the globs that pick up the built scripts and stylesheets did not cover them; the next release would have shipped a viewer that died looking for its manifest. The gemspec now lists the runtime files on purpose — `lib`, `vendor`, the images, the three top-level documents — and names the two JSON files outright, so `gem build` refuses to package a release without them. That catches a missing file but not a stale one, where the manifest names hashes that no longer exist on disk. `FrontendAssets.verify!` covers that: it checks that every file any chunk names is really in `assets/javascript` or `assets/stylesheets`. Everything the build copies lands in one of those two flat directories, so which chunk imports which does not decide where to look, and there is no graph to walk. It does check that the manifest holds together the way the viewer needs, though, since the viewer reads the same file at runtime and raises if it does not: there has to be exactly one entry chunk, every chunk has to name a file, and every chunk an `imports` or `dynamicImports` list names has to be one the manifest describes. `vendor.js` is checked by name, because the build copies it out of `dist/@embroider/virtual` and no chunk mentions it. Two entry chunks means a development build, where the test bundle is an entry of its own. The viewer renders whichever it finds first, so the check refuses the whole manifest rather than trusting the order. A chunk naming anything under `assets` is refused too: those are the fonts and images a chunk imports, and `build_client_app.sh` copies only scripts and stylesheets, so the build would have to learn to package them first. `rake build_client_app` builds the assets, or skips the build when `LOGSTER_SKIP_ASSET_BUILD` is set, and verifies them either way. The publish job sets the variable because `discourse/publish-rubygems-action` runs `rake release` inside a `ruby:3` image that has no node, so building there is not an option; the job builds the assets in an earlier step and the release consumes them. - `rake build` now depends on `build_client_app`, which puts the check in the path of `rake release`. `/pkg/` is ignored so the gem it writes does not leave the tree dirty and trip `release:guard_clean`. - The gemspec test stages the files that would ship into a temporary directory and verifies those, so a change that quietly stopped packaging the frontend fails instead of passing a list of things it correctly leaves out. - The backend CI job builds through `rake build_client_app`, so the check meets a real build on every pull request rather than for the first time during a release. Extracted from #264 Co-authored-by: Sam Saffron <sam.saffron@gmail.com>
davidtaylorhq
force-pushed
the
asset-packaging
branch
from
September 11, 2026 09:47
74ccc1c to
b3bf4d2
Compare
davidtaylorhq
marked this pull request as ready for review
September 24, 2026 11:27
nattsw
approved these changes
Sep 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Vite migration added
assets/manifest.jsonandassets/logster-config.json, but both are gitignored and were absent from the gemspec's file list. The existing globs included the built JavaScript and CSS, so the next release would have shipped those assets without the JSON files the viewer needs to load them.The gemspec now explicitly includes both JSON files and limits the remaining contents to runtime code, vendor files, images, built scripts and stylesheets, and the README, license and changelog. Client sources, tests, gemfiles and CI configuration no longer ship. Explicitly naming the JSON files also makes
gem buildfail when either is missing.FrontendAssets.verify!checks the built assets before packaging:importsanddynamicImportsreferences must resolve.vendor.jsis checked separately because the build copies it outside the manifest.rake buildnow depends onrake build_client_app, which builds and verifies the frontend.LOGSTER_SKIP_ASSET_BUILD=1reuses existing assets but still verifies them. The publish job sets this flag because it builds the frontend before invokingpublish-rubygems-action, whose Ruby container has no Node.js./pkg/is ignored so building the gem does not trip the release task's clean-tree check.The backend CI job builds through the new task. Tests cover invalid or incomplete manifests and verify a staged copy of the gemspec's file list, so accidentally excluding required frontend files fails CI.
Validation: 181 Ruby tests pass with no skips; building with
LOGSTER_SKIP_ASSET_BUILD=1succeeds. The extracted gem passes asset verification and the viewer's rendered asset URLs return HTTP 200. Existing CI is green, including the production frontend build.Extracted from #264
Co-authored-by: Sam Saffron sam.saffron@gmail.com
Stack created with GitHub Stacks CLI • Give Feedback 💬