Skip to content

Commit

Permalink
Fix duplicated Astro and Vite injected styles (#8706)
Browse files Browse the repository at this point in the history
  • Loading branch information
bluwy authored Sep 29, 2023
1 parent 31c59ad commit 3458081
Show file tree
Hide file tree
Showing 13 changed files with 18 additions and 135 deletions.
5 changes: 5 additions & 0 deletions .changeset/olive-bags-think.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fix duplicated Astro and Vite injected styles
36 changes: 0 additions & 36 deletions packages/astro/e2e/css-sourcemaps.test.js

This file was deleted.

18 changes: 3 additions & 15 deletions packages/astro/e2e/css.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,9 @@ test.describe('CSS HMR', () => {
await expect(h).toHaveCSS('color', 'rgb(0, 128, 0)');
});

test('removes Astro-injected CSS once Vite-injected CSS loads', async ({ page, astro }) => {
test('removes Astro-injected CSS once Vite-injected CSS loads', async ({ astro }) => {
const html = await astro.fetch('/').then((res) => res.text());

// style[data-astro-dev-id] should exist in initial SSR'd markup
expect(html).toMatch('data-astro-dev-id');

await page.goto(astro.resolveUrl('/'));

// Ensure JS has initialized
await page.waitForTimeout(500);

// style[data-astro-dev-id] should NOT exist once JS runs
expect(await page.locator('style[data-astro-dev-id]').count()).toEqual(0);

// style[data-vite-dev-id] should exist now
expect(await page.locator('style[data-vite-dev-id]').count()).toBeGreaterThan(0);
// style[data-vite-dev-id] should exist in initial SSR'd markup
expect(html).toMatch('data-vite-dev-id');
});
});
7 changes: 0 additions & 7 deletions packages/astro/e2e/fixtures/css-sourcemaps/astro.config.mjs

This file was deleted.

8 changes: 0 additions & 8 deletions packages/astro/e2e/fixtures/css-sourcemaps/package.json

This file was deleted.

1 change: 0 additions & 1 deletion packages/astro/e2e/fixtures/css-sourcemaps/src/env.d.ts

This file was deleted.

This file was deleted.

This file was deleted.

2 changes: 1 addition & 1 deletion packages/astro/e2e/view-transitions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ test.describe('View Transitions', () => {
});

test('client:only styles are retained on transition', async ({ page, astro }) => {
const totalExpectedStyles = 8;
const totalExpectedStyles = 7;

// Go to page 1
await page.goto(astro.resolveUrl('/client-only-one'));
Expand Down
45 changes: 3 additions & 42 deletions packages/astro/src/runtime/client/hmr.ts
Original file line number Diff line number Diff line change
@@ -1,46 +1,7 @@
/// <reference types="vite/client" />

if (import.meta.hot) {
// Vite injects `<style data-vite-dev-id>` for ESM imports of styles
// but Astro also SSRs with `<style data-astro-dev-id>` blocks. This MutationObserver
// removes any duplicates as soon as they are hydrated client-side.
const injectedStyles = getInjectedStyles();
const mo = new MutationObserver((records) => {
for (const record of records) {
for (const node of record.addedNodes) {
if (isViteInjectedStyle(node)) {
injectedStyles.get(node.getAttribute('data-vite-dev-id')!)?.remove();
}
}
}
});
mo.observe(document.documentElement, { subtree: true, childList: true });

// Vue `link` styles need to be manually refreshed in Firefox
import.meta.hot.on('vite:beforeUpdate', async (payload) => {
for (const file of payload.updates) {
if (file.acceptedPath.includes('vue&type=style')) {
const link = document.querySelector(`link[href="${file.acceptedPath}"]`);
if (link) {
link.replaceWith(link.cloneNode(true));
}
}
}
});
}

function getInjectedStyles() {
const injectedStyles = new Map<string, Element>();
document.querySelectorAll<HTMLStyleElement>('style[data-astro-dev-id]').forEach((el) => {
injectedStyles.set(el.getAttribute('data-astro-dev-id')!, el);
});
return injectedStyles;
}

function isStyle(node: Node): node is HTMLStyleElement {
return node.nodeType === node.ELEMENT_NODE && (node as Element).tagName.toLowerCase() === 'style';
}

function isViteInjectedStyle(node: Node): node is HTMLStyleElement {
return isStyle(node) && !!node.getAttribute('data-vite-dev-id');
// HMR temporarily not needed for now, but kept here in case we need it again.
// To re-instate this module again, update `vite-plugin-astro-server/route.ts`
// to add this module as a script similar to `/@vite/client`
}
2 changes: 1 addition & 1 deletion packages/astro/src/transitions/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ async function updateDOM(
const devId = el.dataset.viteDevId;
// If this same style tag exists, remove it from the new page
return (
newDocument.querySelector(`style[data-astro-dev-id="${devId}"]`) ||
newDocument.querySelector(`style[data-vite-dev-id="${devId}"]`) ||
// Otherwise, keep it anyways. This is client:only styles.
noopEl
);
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/vite-plugin-astro-server/css.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export async function getStylesForURL(
mode === 'development' && // only inline in development
typeof ssrModule?.default === 'string' // ignore JS module styles
) {
importedStylesMap.set(importedModule.url, ssrModule.default);
importedStylesMap.set(importedModule.id ?? importedModule.url, ssrModule.default);
} else {
// NOTE: We use the `url` property here. `id` would break Windows.
importedCssUrls.add(importedModule.url);
Expand Down
15 changes: 4 additions & 11 deletions packages/astro/src/vite-plugin-astro-server/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { loadMiddleware } from '../core/middleware/loadMiddleware.js';
import { createRenderContext, getParamsAndProps, type SSROptions } from '../core/render/index.js';
import { createRequest } from '../core/request.js';
import { matchAllRoutes } from '../core/routing/index.js';
import { isPage, resolveIdToUrl, viteID } from '../core/util.js';
import { isPage } from '../core/util.js';
import { getSortedPreloadedMatches } from '../prerender/routing.js';
import { isServerLikeOutput } from '../prerender/utils.js';
import { PAGE_SCRIPT_ID } from '../vite-plugin-scripts/index.js';
Expand Down Expand Up @@ -275,13 +275,6 @@ async function getScriptsAndStyles({ pipeline, filePath }: GetScriptsAndStylesPa
props: { type: 'module', src: '/@vite/client' },
children: '',
});
scripts.add({
props: {
type: 'module',
src: await resolveIdToUrl(moduleLoader, 'astro/runtime/client/hmr.js'),
},
children: '',
});
}

// TODO: We should allow adding generic HTML elements to the head, not just scripts
Expand Down Expand Up @@ -322,11 +315,11 @@ async function getScriptsAndStyles({ pipeline, filePath }: GetScriptsAndStylesPa
},
children: '',
});
// But we still want to inject the styles to avoid FOUC
// 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: {
// Track the ID so we can match it to Vite's injected style later
'data-astro-dev-id': viteID(new URL(`.${url}`, settings.config.root)),
'data-vite-dev-id': url,
},
children: content,
});
Expand Down

0 comments on commit 3458081

Please sign in to comment.