Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/all-badgers-peel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes `getRelativeLocaleUrl`, `getAbsoluteLocaleUrl`, and `getAbsoluteLocaleUrlList` to strip trailing slashes when `trailingSlash: 'never'` is configured
5 changes: 5 additions & 0 deletions .changeset/itchy-snails-march.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes CSS from `client:only` islands leaking to unrelated pages when Rollup bundles non-CSS-importing modules into the same chunk as CSS-importing modules
5 changes: 5 additions & 0 deletions .changeset/long-cloths-smoke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes HMR not triggering for files inside the `src/middleware/` directory during dev
5 changes: 5 additions & 0 deletions packages/astro/src/core/build/plugins/plugin-css.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,11 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] {
// client:only component and if so, add its CSS to the page it belongs to.
if (this.environment?.name === ASTRO_VITE_ENVIRONMENT_NAMES.client) {
for (const id of Object.keys(chunk.modules)) {
// Only walk from CSS modules to find client:only parents. When Rollup
// merges unrelated modules into the same chunk, walking from every module
// would incorrectly attribute the chunk's CSS to pages reached through
// modules that have no CSS dependency.
if (!isCSSRequest(id)) continue;
for (const pageData of getParentClientOnlys(id, this, internals)) {
for (const importedCssImport of meta.importedCss) {
const cssToInfoRecord = (pagesToCss[pageData.moduleSpecifier] ??= {});
Expand Down
10 changes: 8 additions & 2 deletions packages/astro/src/core/middleware/vite-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ export const MIDDLEWARE_MODULE_ID = 'virtual:astro:middleware';
const MIDDLEWARE_RESOLVED_MODULE_ID = '\0' + MIDDLEWARE_MODULE_ID;
const NOOP_MIDDLEWARE = '\0noop-middleware';

export function isMiddlewarePath(relativePath: string): boolean {
return (
relativePath.startsWith(`${MIDDLEWARE_PATH_SEGMENT_NAME}.`) ||
relativePath.startsWith(`${MIDDLEWARE_PATH_SEGMENT_NAME}/`)
);
}

export function vitePluginMiddleware({ settings }: { settings: AstroSettings }): VitePlugin {
let resolvedMiddlewareId: string | undefined = undefined;
const hasIntegrationMiddleware =
Expand All @@ -43,8 +50,7 @@ export function vitePluginMiddleware({ settings }: { settings: AstroSettings }):
// Check if the changed file is a middleware file under srcDir
if (!normalizedPath.startsWith(normalizedSrcDir)) return;
const relativePath = normalizedPath.slice(normalizedSrcDir.length);
// Dot ensures we match "middleware.ts" but not e.g. "middleware-utils.ts"
if (!relativePath.startsWith(`${MIDDLEWARE_PATH_SEGMENT_NAME}.`)) return;
if (!isMiddlewarePath(relativePath)) return;

for (const name of [
ASTRO_VITE_ENVIRONMENT_NAMES.ssr,
Expand Down
8 changes: 6 additions & 2 deletions packages/astro/src/i18n/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { appendForwardSlash, joinPaths } from '@astrojs/internal-helpers/path';
import {
appendForwardSlash,
joinPaths,
removeTrailingForwardSlash,
} from '@astrojs/internal-helpers/path';
import type { RoutingStrategies } from '../core/app/common.js';
import type { SSRManifest } from '../core/app/types.js';
import { shouldAppendForwardSlash } from '../core/build/util.js';
Expand Down Expand Up @@ -106,7 +110,7 @@ export function getLocaleRelativeUrl({
if (shouldAppendForwardSlash(trailingSlash, format)) {
relativePath = appendForwardSlash(joinPaths(...pathsToJoin));
} else {
relativePath = joinPaths(...pathsToJoin);
relativePath = removeTrailingForwardSlash(joinPaths(...pathsToJoin));
}

if (relativePath === '') {
Expand Down
39 changes: 39 additions & 0 deletions packages/astro/test/client-only-css-chunk-leak.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import assert from 'node:assert/strict';
import { before, describe, it } from 'node:test';
import { load as cheerioLoad } from 'cheerio';
import { type Fixture, loadFixture } from './test-utils.ts';

describe('client:only CSS chunk leak', () => {
let fixture: Fixture;

before(async () => {
fixture = await loadFixture({
root: './fixtures/client-only-css-chunk-leak/',
});
await fixture.build();
});

it('does not leak CSS to pages that do not use CSS-importing client:only components', async () => {
const html = await fixture.readFile('/about/index.html');
const $ = cheerioLoad(html);

const stylesheets = $('link[rel=stylesheet]');
// The about page only uses CurrentTime (no CSS imports).
// It should have no stylesheets from the shared chunk.
assert.equal(stylesheets.length, 0, 'About page should have no stylesheet links');
});

it('includes CSS on the page that uses the CSS-importing client:only component', async () => {
const html = await fixture.readFile('/index.html');
const $ = cheerioLoad(html);

const stylesheets = await Promise.all(
$('link[rel=stylesheet]').map((_, el) => {
return fixture.readFile(el.attribs.href);
}),
);
const css = stylesheets.join('');

assert.match(css, /\.heavy-widget/, 'Home page should include .heavy-widget CSS');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { defineConfig } from 'astro/config';
import react from '@astrojs/react';

export default defineConfig({
integrations: [react()],
build: { inlineStylesheets: 'never' },
vite: {
build: {
rollupOptions: {
output: {
// Force StyledPanel (which imports CSS) and formatLabel (a pure utility)
// into the same chunk. This simulates what Rollup's default heuristics
// do naturally in large apps.
manualChunks(id) {
if (id.includes('StyledPanel') || id.includes('formatLabel')) {
return 'shared-utils';
}
},
},
},
},
},
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "@test/client-only-css-chunk-leak",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*",
"@astrojs/react": "workspace:*",
"react": "^19.0.0",
"react-dom": "^19.0.0"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { formatLabel } from './formatLabel';

export default function CurrentTime() {
return <span>{formatLabel('time')}</span>;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import StyledPanel from './StyledPanel';
import { formatLabel } from './formatLabel';

export default function HeavyWidget() {
return <StyledPanel>{formatLabel('Widget')}</StyledPanel>;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
.heavy-widget { color: red; font-size: 72px; background: limegreen; padding: 2rem; }
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import './StyledPanel.css';

export default function StyledPanel({ children }: { children: React.ReactNode }) {
return <div className="heavy-widget">{children}</div>;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function formatLabel(text: string): string {
return `[ ${text.toUpperCase()} ]`;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
import CurrentTime from '../components/CurrentTime';
---

<html lang="en">
<head>
<meta charset="utf-8" />
<title>CSS Leak Test</title>
</head>
<body>
<CurrentTime client:only="react" />
<slot />
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
import Layout from '../layouts/Layout.astro';
---

<Layout>
<h1>About</h1>
</Layout>
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
import Layout from '../layouts/Layout.astro';
import HeavyWidget from '../components/HeavyWidget';
---

<Layout>
<h1>Home</h1>
<HeavyWidget client:only="react" />
</Layout>
82 changes: 82 additions & 0 deletions packages/astro/test/units/i18n/astro_i18n.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,88 @@ describe('getLocaleRelativeUrl', () => {
);
});

it('should not add trailing slash when trailingSlash is "never" and path is empty or "/" (#17034)', () => {
const config = {
i18n: {
defaultLocale: 'en',
locales: ['en', 'pl'],
},
};

// path omitted vs path='' should produce the same result
assert.equal(
relativeUrl({
locale: 'pl',
base: '/',
...config.i18n,
trailingSlash: 'never',
format: 'directory',
}),
'/pl',
);
assert.equal(
relativeUrl({
locale: 'pl',
base: '/',
...config.i18n,
trailingSlash: 'never',
format: 'directory',
path: '',
}),
'/pl',
);
assert.equal(
relativeUrl({
locale: 'pl',
base: '/',
...config.i18n,
trailingSlash: 'never',
format: 'directory',
path: '/',
}),
'/pl',
);

// prependWith + trailing slash in path
assert.equal(
relativeUrl({
locale: 'pl',
base: '/',
...config.i18n,
trailingSlash: 'never',
format: 'directory',
path: 'docs/setup/',
prependWith: 'blog',
}),
'/blog/pl/docs/setup',
);

// absolute URL should also strip trailing slash
assert.equal(
absoluteUrl({
locale: 'pl',
base: '/',
...config.i18n,
trailingSlash: 'never',
format: 'directory',
path: '',
site: 'https://example.com',
}),
'https://example.com/pl',
);

// absoluteUrlList should be consistent across all locales
const list = absoluteUrlList({
base: '/',
...config.i18n,
trailingSlash: 'never',
format: 'directory',
path: '',
site: 'https://example.com',
});
assert.deepEqual(list, ['https://example.com', 'https://example.com/pl']);
});

it('should normalize locales by default', () => {
const config = {
base: '/blog',
Expand Down
37 changes: 37 additions & 0 deletions packages/astro/test/units/middleware/middleware-hmr.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import assert from 'node:assert/strict';
import { describe, it } from 'node:test';
import { isMiddlewarePath } from '../../../dist/core/middleware/vite-plugin.js';

describe('middleware HMR path matching', () => {
it('matches middleware.ts (single-file pattern)', () => {
assert.ok(isMiddlewarePath('middleware.ts'));
});

it('matches middleware.js', () => {
assert.ok(isMiddlewarePath('middleware.js'));
});

it('matches middleware/index.ts (directory pattern)', () => {
assert.ok(isMiddlewarePath('middleware/index.ts'));
});

it('matches middleware/test.ts (file inside middleware directory)', () => {
assert.ok(isMiddlewarePath('middleware/test.ts'));
});

it('matches middleware/nested/deep.ts (nested file)', () => {
assert.ok(isMiddlewarePath('middleware/nested/deep.ts'));
});

it('does not match middleware-utils.ts (similarly named file)', () => {
assert.ok(!isMiddlewarePath('middleware-utils.ts'));
});

it('does not match pages/middleware.ts (wrong directory)', () => {
assert.ok(!isMiddlewarePath('pages/middleware.ts'));
});

it('does not match other unrelated files', () => {
assert.ok(!isMiddlewarePath('components/Header.astro'));
});
});
Loading
Loading