Skip to content
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

Missing styles since Astro 5 #12616

Closed
1 task
lilnasy opened this issue Dec 3, 2024 · 10 comments
Closed
1 task

Missing styles since Astro 5 #12616

lilnasy opened this issue Dec 3, 2024 · 10 comments
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) feat: styling Related to styles (scope)

Comments

@lilnasy
Copy link
Contributor

lilnasy commented Dec 3, 2024

Astro Info

Astro                    v5.0.2
Node                     v18.20.3
System                   Linux (x64)
Package Manager          npm
Output                   static
Adapter                  none
Integrations             none

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

Stylesheet generated by vite plugins is no longer added to the page.

image

What's the expected result?

Same result that astro 4 generates.

image

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-ojxby9

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Dec 3, 2024
@bozdevs
Copy link

bozdevs commented Dec 4, 2024

In V5, this error is also thrown if reference("collection") is used in z.array.

[UnhandledRejection] Astro detected an unhandled rejection. Here's the stack trace:
TypeError: Cannot destructure property 'type' of 'lookupMap[collection]' as it is undefined.
    at Object.eval [as transform] (/node_modules/astro/dist/content/runtime.js:487:17)
    at async Promise.all (index 0)

Update

You should flush the folder named .astro after upgrade.

@ascorbic
Copy link
Contributor

ascorbic commented Dec 4, 2024

@bozdevs can you submit a separate issue with its own repro please

@ascorbic
Copy link
Contributor

ascorbic commented Dec 4, 2024

@lilnasy Even though this is a regression with Astro 5, it would be a good idea to submit this as an issue to ecsstatic too. It's hard to say if this is an issue with that plugin + Vite 6, or if it's Astro-specific, as the plugin hasn't been updated for Vite 6 yet.

@ascorbic ascorbic added feat: styling Related to styles (scope) - P3: minor bug An edge case that only affects very specific usage (priority) and removed needs triage Issue needs to be triaged labels Dec 4, 2024
@ascorbic
Copy link
Contributor

ascorbic commented Dec 4, 2024

This does only seem to be an issue in dev too: when I run a build, it works correctly.

@lilnasy
Copy link
Contributor Author

lilnasy commented Dec 4, 2024

The issue is astro specific. The styles added onto astro components are missing. They still load with framework components.

Although, the use with framework components changed in a more subtle way: the stylesheet is not added immediately in the initial html, rather it is added by vite client js dynamically. This affects vite 6 too but it does not block upgrading, the entirely missing styles in astro components do.

That might mean vite no longer adds the css to the module graph (not immediately, at least), and that's why astro fails to retrieve them in getStylesForURL():

// Pass framework CSS in as style tags to be appended to the page.
const links = new Set<SSRElement>();
const { urls, styles: _styles } = await getStylesForURL(filePath, loader);
for (const href of urls) {
links.add({ props: { rel: 'stylesheet', href }, children: '' });
}
const styles = new Set<SSRElement>();
for (const { id, url: src, content } of _styles) {
// Vite handles HMR for styles injected as scripts
scripts.add({ props: { type: 'module', src }, children: '' });
// But we still want to inject the styles to avoid FOUC. The style tags
// should emulate what Vite injects so further HMR works as expected.
styles.add({ props: { 'data-vite-dev-id': id }, children: content });
}

/** Given a filePath URL, crawl Vite’s module graph to find all style imports. */
export async function getStylesForURL(
filePath: URL,
loader: ModuleLoader,
): Promise<{ urls: Set<string>; styles: ImportedStyle[]; crawledFiles: Set<string> }> {
const importedCssUrls = new Set<string>();
// Map of url to injected style object. Use a `url` key to deduplicate styles
const importedStylesMap = new Map<string, ImportedStyle>();
const crawledFiles = new Set<string>();
for await (const importedModule of crawlGraph(loader, viteID(filePath), true)) {

...and css for astro components is added only on initial page load, unlike for framework components, relying on full page refresh to update them, which seems to be another regression.

@bluwy
Copy link
Member

bluwy commented Dec 5, 2024

I have not took a deeper look, but my hunch is that @acab/ecsstatic is not handling this Vite breaking change which indirectly affects how Astro searches for CSS.

@lilnasy
Copy link
Contributor Author

lilnasy commented Dec 5, 2024

It uses side effect imports, not css modules.

https://github.com/mayank99/ecsstatic/blob/3c75f63414e7a7ee88305ff3731f9a2109806fd5/package/vite.ts#L162

Would you expect a difference between astro and framework components with the breaking change you linked?

@bluwy
Copy link
Member

bluwy commented Dec 5, 2024

Yes as framework components are rendered client side which Vite takes over and dynamically adds CSS by itself. The breaking change I linked affects all kinds of CSS, not only CSS modules.

@lilnasy
Copy link
Contributor Author

lilnasy commented Dec 5, 2024

If that is the relevant breaking change, how would the plugin migrate? Would it be by hooking the load of the css file with export default ${JSON.stringify(css)} for the server consumer?

@bluwy
Copy link
Member

bluwy commented Dec 6, 2024

I've sent a fix downstream. The idea is for virtual module CSS to support the ?inline query when Astro tries to add it to be able to get the processed CSS. I'll close this for now as it isn't a bug in Astro. mayank99/ecsstatic#25

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) feat: styling Related to styles (scope)
Projects
None yet
Development

No branches or pull requests

4 participants