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

Make trailingSlash: 'always' default #11922

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 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
33 changes: 31 additions & 2 deletions packages/astro/src/core/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,20 @@ type RehypePlugin = ComplexifyWithUnion<_RehypePlugin>;
type RemarkPlugin = ComplexifyWithUnion<_RemarkPlugin>;
type RemarkRehype = ComplexifyWithOmit<_RemarkRehype>;

const suggestedTrailingSlashToBuildFormat = {
always: 'directory',
never: 'file',
ignore: 'preserve',
} as const;

export const ASTRO_CONFIG_DEFAULTS = {
root: '.',
srcDir: './src',
publicDir: './public',
outDir: './dist',
cacheDir: './node_modules/.astro',
base: '/',
trailingSlash: 'ignore',
trailingSlash: 'always',
build: {
format: 'directory',
client: './dist/client/',
Expand Down Expand Up @@ -540,6 +546,8 @@ export const AstroConfigSchema = z.object({
export type AstroConfigType = z.infer<typeof AstroConfigSchema>;

export function createRelativeSchema(cmd: string, fileProtocolRoot: string) {
let originalBuildFormat: 'file' | 'directory' | 'preserve' | undefined;

// We need to extend the global schema to add transforms that are relative to root.
// This is type checked against the global schema to make sure we still match.
const AstroConfigRelativeSchema = AstroConfigSchema.extend({
Expand All @@ -566,10 +574,14 @@ export function createRelativeSchema(cmd: string, fileProtocolRoot: string) {
.transform((val) => resolveDirAsUrl(val, fileProtocolRoot)),
build: z
.object({
// NOTE: The `format` default will be set later as it's based on the `trailingSlash` value
format: z
.union([z.literal('file'), z.literal('directory'), z.literal('preserve')])
.optional()
.default(ASTRO_CONFIG_DEFAULTS.build.format),
.transform((val) => {
originalBuildFormat = val;
return val ?? ASTRO_CONFIG_DEFAULTS.build.format;
}),
client: z
.string()
.optional()
Expand Down Expand Up @@ -660,6 +672,23 @@ export function createRelativeSchema(cmd: string, fileProtocolRoot: string) {
config.base = prependForwardSlash(config.base);
}

// Handle `build.format` default based on `trailingSlash` config
const suggestedBuildFormat = suggestedTrailingSlashToBuildFormat[config.trailingSlash];
if (originalBuildFormat === undefined) {
config.build.format = suggestedBuildFormat;
} else if (config.build.format !== suggestedBuildFormat) {
const suggestedTrailingSlash = Object.entries(suggestedTrailingSlashToBuildFormat).find(
([_, format]) => format === config.build.format,
)?.[0];
// Use console.warn instead of logger because it's hard to pass the logger here
// eslint-disable-next-line no-console
console.warn(
`The trailingSlash option is set as "${config.trailingSlash}", but build.format is set as "${config.build.format}". ` +
`This will likely cause inconsistencies in relative paths when developing your app. To fix this, remove the build.format ` +
`option so it defaults to "${suggestedBuildFormat}", or set trailingSlash to "${suggestedTrailingSlash}".`,
);
}

return config;
})
.refine((obj) => !obj.outDir.toString().startsWith(obj.publicDir.toString()), {
Expand Down
20 changes: 15 additions & 5 deletions packages/astro/src/types/public/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,18 +170,27 @@ export interface AstroUserConfig {
* @docs
* @name trailingSlash
* @type {('always' | 'never' | 'ignore')}
* @default `'ignore'`
* @default `'always'`
* @see build.format
* @description
*
* Set the route matching behavior of the dev server. Choose from the following options:
* Set the route matching behavior of the dev server and serving of on-demand routes in production. Choose from the following options:
* - `'always'` - Only match URLs that include a trailing slash (ex: "/foo/")
* - `'never'` - Never match URLs that include a trailing slash (ex: "/foo")
* - `'ignore'` - Match URLs regardless of whether a trailing "/" exists
*
* Use this configuration option if your production host has strict handling of how trailing slashes work or do not work.
* For pages that are prerendered (e.g. `src/pages/foo.html`), they will be written as `.html` files like below:
* | `trailingSlash` | `src/pages/about.astro` | `src/pages/about/index.astro` |
* | - | - | - |
* | `'always'` | `dist/about/index.html` | `dist/about/index.html` |
* | `'never'` | `dist/about.html` | `dist/about.html` |
* | `'ignore'` | `dist/about.html` | `dist/about/index.html` |
*
* Using `'ignore'` is not recommended as it allows the same page to exist in two paths and affects SEO. See
* https://www.seroundtable.com/google-trailing-slashes-url-24943.html for more details.
*
* When set to `'always'`, URL paths already contains an extension, e.g. endpoints from `src/pages/api.json.ts`, do not need a trailing slash.
*
* You can also set this if you prefer to be more strict yourself, so that URLs with or without trailing slashes won't work during development.
*
* ```js
* {
Expand Down Expand Up @@ -527,7 +536,8 @@ export interface AstroUserConfig {
* @docs
* @name build.format
* @typeraw {('file' | 'directory' | 'preserve')}
* @default `'directory'`
* @default `'directory'` if `trailingSlash: 'always'`, `'file'` if `trailingSlash: 'never'`, `'preserve'` if `trailingSlash: 'ignore'`
* @deprecated Use the `trailingSlash` option instead.
* @description
* Control the output file format of each page. This value may be set by an adapter for you.
* - `'file'`: Astro will generate an HTML file named for each page route. (e.g. `src/pages/about.astro` and `src/pages/about/index.astro` both build the file `/about.html`)
Expand Down
3 changes: 2 additions & 1 deletion packages/astro/src/vite-plugin-utils/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { fileURLToPath } from 'node:url';
import path from 'node:path'
import ancestor from 'common-ancestor-path';
import {
appendExtension,
Expand All @@ -19,7 +20,7 @@ export function getFileInfo(id: string, config: AstroConfig) {
.replace(/^.*?\/pages\//, sitePathname)
.replace(/(?:\/index)?\.(?:md|markdown|mdown|mkdn|mkd|mdwn|astro)$/, '')
: undefined;
if (fileUrl && config.trailingSlash === 'always') {
if (fileUrl && config.trailingSlash === 'always' && !path.basename(fileUrl).includes('.')) {
fileUrl = appendForwardSlash(fileUrl);
}
if (fileUrl && config.build.format === 'file') {
Expand Down
12 changes: 6 additions & 6 deletions packages/astro/test/actions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe('Astro Actions', () => {
}),
}),
);
const res = await fixture.fetch('/subscribe-prerendered', {
const res = await fixture.fetch('/subscribe-prerendered/', {
headers: {
Cookie: cookie.toString(),
},
Expand Down Expand Up @@ -197,7 +197,7 @@ describe('Astro Actions', () => {
});

it('Response middleware fallback', async () => {
const req = new Request('http://example.com/user?_astroAction=getUser', {
const req = new Request('http://example.com/user/?_astroAction=getUser', {
method: 'POST',
body: new FormData(),
headers: {
Expand All @@ -213,11 +213,11 @@ describe('Astro Actions', () => {
});

it('Respects custom errors', async () => {
const req = new Request('http://example.com/user-or-throw?_astroAction=getUserOrThrow', {
const req = new Request('http://example.com/user-or-throw/?_astroAction=getUserOrThrow', {
method: 'POST',
body: new FormData(),
headers: {
Referer: 'http://example.com/user-or-throw',
Referer: 'http://example.com/user-or-throw/',
},
});
const res = await followExpectedRedirect(req, app);
Expand All @@ -230,7 +230,7 @@ describe('Astro Actions', () => {
});

it('Ignores `_astroAction` name for GET requests', async () => {
const req = new Request('http://example.com/user-or-throw?_astroAction=getUserOrThrow', {
const req = new Request('http://example.com/user-or-throw/?_astroAction=getUserOrThrow', {
method: 'GET',
});
const res = await app.render(req);
Expand Down Expand Up @@ -338,7 +338,7 @@ describe('Astro Actions', () => {
});

it('Is callable from the server with rewrite', async () => {
const req = new Request('http://example.com/rewrite');
const req = new Request('http://example.com/rewrite/');
const res = await app.render(req);
assert.equal(res.ok, true);

Expand Down
4 changes: 2 additions & 2 deletions packages/astro/test/astro-basic.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ describe('Astro basic development', () => {
});

it('Renders markdown in utf-8 by default', async () => {
const res = await fixture.fetch('/chinese-encoding-md');
const res = await fixture.fetch('/chinese-encoding-md/');
assert.equal(res.status, 200);
const html = await res.text();
const $ = cheerio.load(html);
Expand All @@ -214,7 +214,7 @@ describe('Astro basic development', () => {
});

it('Renders MDX in utf-8 by default', async () => {
const res = await fixture.fetch('/chinese-encoding-mdx');
const res = await fixture.fetch('/chinese-encoding-mdx/');
assert.equal(res.status, 200);
const html = await res.text();
const $ = cheerio.load(html);
Expand Down
32 changes: 16 additions & 16 deletions packages/astro/test/astro-cookies.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('Astro.cookies', () => {
});

it('is able to get cookies from the request', async () => {
const response = await fixture.fetch('/get-json', {
const response = await fixture.fetch('/get-json/', {
headers: {
cookie: `prefs=${encodeURIComponent(JSON.stringify({ mode: 'light' }))}`,
},
Expand All @@ -42,7 +42,7 @@ describe('Astro.cookies', () => {
});

it('can set the cookie value', async () => {
const response = await fixture.fetch('/set-value', {
const response = await fixture.fetch('/set-value/', {
method: 'POST',
});
assert.equal(response.status, 200);
Expand All @@ -54,26 +54,26 @@ describe('Astro.cookies', () => {
});

it('can set cookies in a rewritten page request', async () => {
const response = await fixture.fetch('/from');
const response = await fixture.fetch('/from/');
assert.equal(response.status, 200);

assert.match(response.headers.get('set-cookie'), /my_cookie=value/);
});

it('overwrites cookie values set in the source page with values from the target page', async () => {
const response = await fixture.fetch('/from');
const response = await fixture.fetch('/from/');
assert.equal(response.status, 200);
assert.match(response.headers.get('set-cookie'), /another=set-in-target/);
});

it('allows cookies to be set in the source page', async () => {
const response = await fixture.fetch('/from');
const response = await fixture.fetch('/from/');
assert.equal(response.status, 200);
assert.match(response.headers.get('set-cookie'), /set-in-from=yes/);
});

it('can set cookies in a rewritten endpoint request', async () => {
const response = await fixture.fetch('/from-endpoint');
const response = await fixture.fetch('/from-endpoint/');
assert.equal(response.status, 200);
assert.match(response.headers.get('set-cookie'), /test=value/);
});
Expand All @@ -93,7 +93,7 @@ describe('Astro.cookies', () => {
}

it('is able to get cookies from the request', async () => {
const response = await fetchResponse('/get-json', {
const response = await fetchResponse('/get-json/', {
headers: {
cookie: `prefs=${encodeURIComponent(JSON.stringify({ mode: 'light' }))}`,
},
Expand All @@ -106,7 +106,7 @@ describe('Astro.cookies', () => {
});

it('can set the cookie value', async () => {
const response = await fetchResponse('/set-value', {
const response = await fetchResponse('/set-value/', {
method: 'POST',
});
assert.equal(response.status, 200);
Expand All @@ -116,7 +116,7 @@ describe('Astro.cookies', () => {
});

it('app.render can include the cookie in the Set-Cookie header', async () => {
const request = new Request('http://example.com/set-value', {
const request = new Request('http://example.com/set-value/', {
method: 'POST',
});
const response = await app.render(request, { addCookieHeader: true });
Expand All @@ -127,7 +127,7 @@ describe('Astro.cookies', () => {
});

it('app.render can exclude the cookie from the Set-Cookie header', async () => {
const request = new Request('http://example.com/set-value', {
const request = new Request('http://example.com/set-value/', {
method: 'POST',
});
const response = await app.render(request, { addCookieHeader: false });
Expand All @@ -136,7 +136,7 @@ describe('Astro.cookies', () => {
});

it('Early returning a Response still includes set headers', async () => {
const response = await fetchResponse('/early-return', {
const response = await fetchResponse('/early-return/', {
headers: {
cookie: `prefs=${encodeURIComponent(JSON.stringify({ mode: 'light' }))}`,
},
Expand All @@ -151,7 +151,7 @@ describe('Astro.cookies', () => {
});

it('API route can get and set cookies', async () => {
const response = await fetchResponse('/set-prefs', {
const response = await fetchResponse('/set-prefs/', {
method: 'POST',
headers: {
cookie: `prefs=${encodeURIComponent(JSON.stringify({ mode: 'light' }))}`,
Expand All @@ -167,29 +167,29 @@ describe('Astro.cookies', () => {
});

it('can set cookies in a rewritten page request', async () => {
const request = new Request('http://example.com/from');
const request = new Request('http://example.com/from/');
const response = await app.render(request, { addCookieHeader: true });
assert.equal(response.status, 200);

assert.match(response.headers.get('Set-Cookie'), /my_cookie=value/);
});

it('overwrites cookie values set in the source page with values from the target page', async () => {
const request = new Request('http://example.com/from');
const request = new Request('http://example.com/from/');
const response = await app.render(request, { addCookieHeader: true });
assert.equal(response.status, 200);
assert.match(response.headers.get('Set-Cookie'), /another=set-in-target/);
});

it('allows cookies to be set in the source page', async () => {
const request = new Request('http://example.com/from');
const request = new Request('http://example.com/from/');
const response = await app.render(request, { addCookieHeader: true });
assert.equal(response.status, 200);
assert.match(response.headers.get('Set-Cookie'), /set-in-from=yes/);
});

it('can set cookies in a rewritten endpoint request', async () => {
const request = new Request('http://example.com/from-endpoint');
const request = new Request('http://example.com/from-endpoint/');
const response = await app.render(request, { addCookieHeader: true });
assert.equal(response.status, 200);
assert.match(response.headers.get('Set-Cookie'), /test=value/);
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/test/astro-envs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,15 +110,15 @@ describe('Environment Variables', () => {
assert.equal(res.status, 200);
let indexHtml = await res.text();
let $ = cheerio.load(indexHtml);
assert.equal($('#base-url').text(), '/blog');
assert.equal($('#base-url').text(), '/blog/');
});

it('does render destructured builtin SITE env', async () => {
let res = await fixture.fetch('/blog/destructured/');
assert.equal(res.status, 200);
let indexHtml = await res.text();
let $ = cheerio.load(indexHtml);
assert.equal($('#base-url').text(), '/blog');
assert.equal($('#base-url').text(), '/blog/');
});
});
});
Loading
Loading