fix: parse built asset tags from the document and render every entry stylesheet - #1582
Open
xantorres wants to merge 6 commits into
Open
fix: parse built asset tags from the document and render every entry stylesheet#1582xantorres wants to merge 6 commits into
xantorres wants to merge 6 commits into
Conversation
Two things the server depends on are invisible to the build. Both fail silently, so a green build and a working dev server actively disguise them. The server parses the script and stylesheet paths out of the built index.html and reuses them on every server-rendered page. That parse is coupled to the exact attribute set and attribute order the frontend build writes into those tags. A build that emits a different tag shape still succeeds, the dev server still works, the binary still compiles, and the pages simply render with no scripts and no stylesheet. TestGetStyleResolvesBuiltAssets asserts the parse still finds them. Languages other than the default one are loaded with a template-literal dynamic import through an alias that points outside the frontend root. A bundler that cannot enumerate that pattern still builds and still serves a working app; the resources never arrive, and only for non-default languages, so a smoke test in the default language misses it. check-locale-resolution.js bundles that same import with the project's own configuration, runs it, and requires two languages to resolve to distinct translated content. Both run through make check-ui. check-built-assets.sh --self-check confirms the asset check still fails on tag shapes the parser cannot read, so a check that quietly stopped asserting anything is distinguishable from a passing one. (cherry picked from commit 71cd924, internal/controller/template_controller_test.go only)
GetStyle scraped index.html with regexes matching one exact tag shape: classic scripts with defer first, and stylesheet links with href before rel. Any bundler emitting a different shape returned nothing, and server-rendered pages would load with no JavaScript and no stylesheet while every build step still reported success. The tags are now read from the parsed document, so attribute order, attribute set and quoting no longer matter. header.html emits the scraped paths as script tags itself, and those were classic scripts. A module bundle loaded that way fails on its first import, so fixing only the parsing would have left server-rendered pages broken; the tag is now declared as a module. The self-check fixtures are replaced. The previous two asserted failure on module scripts and on rel-before-href, both of which the parser now accepts, so they would have inverted into false alarms. The replacements cover a stylesheet with no script, a script with no stylesheet, and an inline script with no src. golang.org/x/net moves to a direct requirement, matching its use here. (cherry picked from commit eab6f9d, go.mod and internal/controller/template_controller.go only)
GetStyle returned a single stylesheet path because the previous build emitted exactly one. The current build emits two, so server-rendered pages loaded partially unstyled while every build step still reported success. The stylesheet is now a list, mirroring how script paths are already collected and prefixed, and the template renders one link per entry. This is the same assumption as the tag-shape one fixed earlier: the server encoded a property of one bundler's output, here that there is exactly one entry stylesheet. (cherry picked from commit d66e21b)
The linter configured for this repository flags the slice-building form, so make lint fails on it. (cherry picked from commit 1cfd5da)
Asserting that the parsed stylesheet list is non-empty leaves the exact regression this repository already hit uncovered: a parser that stops at the first stylesheet returns a one element list, satisfies every existing assertion, and silently drops the rest of the page's CSS. Count the declarations again by a cruder method than the parser uses and require the two to agree, so the parser has to be checked against something other than itself. The count is a lower bound: a build that quotes attributes differently drives it to zero and it stops constraining, which is why it supplements the shape-independent assertions rather than replacing them. Verified by reintroducing the truncation and watching this fail. (cherry picked from commit 9232cbd)
The GetStyle comment framed the DOM walk around attribute order, attribute set, and quoting, the same properties the old regex parser depended on, without stating that the new parser ignores all of them. check-built-assets.sh described its self-check as failing when asset tags change shape, but the fixtures test a missing script or stylesheet tag, and the parser accepts any shape as long as the tag is present. Comments now state the actual constraint: tag shape does not affect parsing, and the guarding check fails when a build is present but a required script or stylesheet tag is missing from it. (cherry picked from commit 3bc12e8, internal/controller/template_controller.go only)
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
1 participant
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.
Fixes #1579. Part of #1578. Step 1 of 3; lands while Create React App still builds, and step 2 (the Vite cutover) depends on it.
Three parts of the Go side had one bundler's output format encoded into them. This PR removes two of them ahead of the toolchain change; the third, the classic script tag in
header.html, is the one line step 2 flips together with the build that emits modules, so the cutover itself stays a frontend-only diff plus that line.GetStyle()matched a literal tag shape. It scrapedindex.htmlwith regexes requiring classic scripts withdeferfirst and stylesheet links withhrefbeforerel. Any other shape returned nothing, and server-rendered pages would load with no JavaScript and no stylesheet while every build step still reported success. The tags are now read from the parsed document (golang.org/x/net/html, already in the module graph and promoted from indirect to direct ingo.mod), so attribute order, attribute set and quoting no longer matter, and the next bundler change cannot reintroduce this. The parser accepts any shape as long as the tag is present; what it guards is a missing script or stylesheet tag.GetStyle()returned a single stylesheet. The current build emits exactly one entry stylesheet; the Vite build emits two, so pages would load partially unstyled. It is now a list, mirroring how script paths were already collected and prefixed, andheader.htmlranges over it. With today's build that is a list of one, and the rendered page is unchanged.header.htmlkeeps its classic script tag.<script defer="defer" src="{{$path}}">is untouched here on purpose; it becomes a module tag in step 2, together with the build that emits modules.Test.
TestGetStyleResolvesBuiltAssetsininternal/controller/template_controller_test.gocalls the realGetStyle()against the real embedded build and requires at least one script source and every declared stylesheet to come back. It skips when no frontend build is embedded, which is a constraint for anyone wiring CI later: a barego test ./...reports ok while asserting nothing about the asset paths unless the frontend was built first. The self-check harness that rewrites the builtindex.htmlinto unparseable shapes and asserts the test fails on each follows in step 3.Agentic tooling did the mechanical work in this series; every change was reviewed by a human before being committed.
Verification
On
devat2c0ced32plus these commits, with the current toolchain:pnpm install --frozen-lockfileandpnpm build(react-scripts) complete;go build ./...andgo vet ./...pass;TestGetStyleResolvesBuiltAssetspasses against the Create React App output (--- PASS, not skipped), and as a negative control the same test fails when the builtindex.htmlis rewritten without a scriptsrc, then passes again once the original is restored. The built index.html at that point carries eight classic<script defer="defer" src="/static/js/...">tags, one inline script and one stylesheet link, and the parser returns all eight sources and the one stylesheet.The commits carry
(cherry picked from commit ...)lines pointing at the branch behind #1567, where this change was first reviewed.