-
-
Notifications
You must be signed in to change notification settings - Fork 3k
fix: resolve environment-api build issues for client & CSS #14770
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
Conversation
Fix 4 critical issues with the environment-based build system: 1. Client build now conditionally runs only when client input exists, preventing rollup errors when no client scripts are present 2. Client environment rollup config now specifies output file naming (entryFileNames, chunkFileNames, assetFileNames) to place JS bundles in _astro folder instead of root 3. Manifest serialization refactored to read pageData.styles immediately when serializing each route, ensuring CSS collected during build is captured before prerender static files are added 4. BuildPipeline.retrieveRoutesToGenerate() now iterates manifest.routes directly (which has complete style info) instead of reconstructing from internals This fixes cascading test failures in CSS collection, asset paths, and manifest generation.
|
ematipico
left a comment
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.
The extraction of the prerender entry point needs a better check
| if (routeIsRedirect(pageData.route)) { | ||
| pages.set(pageData, pageData.component); | ||
| } else if ( | ||
| routeIsFallback(pageData.route) && | ||
| (i18nHasFallback(this.config) || | ||
| (routeIsFallback(pageData.route) && pageData.route.route === '/')) | ||
| ) { |
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.
Here we removed essential logic that hasn't been placed anywhere else. Why? Maybe we should update the docs of this method to better align it with the new logic
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.
We now add all routes. This function is quite different because it now operates on the list of routes in the manifest rather than these other maps we used to use.
Can you explain what this logic was doing? It's filtering out fallback routes unless i18n has fallback configuration or the route happens to be the base?
I was of course trying to be careful to preserve logic as much as possible but given that prerendering works much differently now (being based on the manifest) it was difficult to preserve everything, and I figured as we went through the tests we would fix these kinds of things.
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.
Ok, then maybe it's best if later you can share with us how the prerendering works. Not a blocker
Co-authored-by: Emanuele Stoppa <[email protected]> Co-authored-by: Samuel Macleod <[email protected]> Co-authored-by: Alexander Niebuhr <[email protected]> Co-authored-by: ascorbic <[email protected]> Co-authored-by: matthewp <[email protected]> Co-authored-by: Florian Lefebvre <[email protected]> Co-authored-by: Matt Kane <[email protected]> Co-authored-by: florian-lefebvre <[email protected]> Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Matthew Phillips <[email protected]> Co-authored-by: Matthew Phillips <[email protected]> Co-authored-by: Sarah Rainsberger <[email protected]> Co-authored-by: Erika <[email protected]> Co-authored-by: Hayato Hasegawa <[email protected]> fix: creation of routes list in build (#14325) Fix unit tests for environment API (#14394) fixture (#14599) fix: organize imports and formatting in cloudflare handler (#14697) fix: resolve environment-api build issues for client & CSS (#14770) Fixes CSS when both static/client styles exist (#14790) fix: flag `serverLike` incorrectly computed (#14792) fix: redirects and i18n routing (#14797) fix: logger level and static paths params (#14799) Fix image tests crashing (#14803) fix(environment): Error overlay not working (#14816) fix: hashing of chunks (#14811) Fix before-hydration script emission for environment-api (#14818) fix: node leak into content collections (#14820) fix: avoid `node:path` (#14827) Fix API route tests (#14825) Fix unit tests (#14832) fix: server islands build (#14830) Fix astro-component-bundling tests (#14840) Fix astro-component-bundling tests (#14837) Fix config alias by using resolve.alias config (#14828) fix: container and routing tests (#14852) fix: respect user vite config on sourcemaps (#14847) fix: remove useless tests (#14854) Fixes assetQueryParams and public folder tests (#14853) fix: renderError fallback (#14855) Fix server-islands data being used across tests (#14857) fix: incorrect configuration in tests (#14858) Fixes for tests in environment branch (#14865) Fixes for e2e tests in environment branch (#14870) fix(test): svelte async rendering (#14886) fix(cloudflare/vercel/netlify): some tests pass and excluded others (#14892) Fixes for node integrations tests (#14900) Fix @astrojs/db tests (#14899) fix(environment): Prevent Vite from interpreting injected sourcemap comments as actual sourcemaps (#14921) Fix remaining cloudflare tests (#14912) fix(vite): import mergeConfig from direct file to avoid dynamic import being processed by Vite (#14939) Fix astro:env tests in Cloudflare (#14925)
Fix 4 critical issues with the environment-based build system:
Client build now conditionally runs only when client input exists, preventing rollup errors when no client scripts are present
Client environment rollup config now specifies output file naming (entryFileNames, chunkFileNames, assetFileNames) to place JS bundles in _astro folder instead of root, this fixes various tests that expect assets to be in
_astro.Manifest serialization refactored to read pageData.styles immediately when serializing each route, ensuring CSS collected during build is captured before prerender static files are added
BuildPipeline.retrieveRoutesToGenerate() now iterates manifest.routes directly (which has complete style info) instead of reconstructing from internals
This fixes cascading test failures in CSS collection, asset paths, and manifest generation.