Skip to content

Commit

Permalink
Preserve base slash with trailingSlash ignore
Browse files Browse the repository at this point in the history
  • Loading branch information
bluwy committed Aug 4, 2023
1 parent 93ad8b9 commit 5775b4f
Show file tree
Hide file tree
Showing 10 changed files with 38 additions and 40 deletions.
7 changes: 7 additions & 0 deletions .changeset/six-grapes-look.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'astro': major
---

The value of `import.meta.env.BASE_URL`, which is derived from the `base` option, will not be enforced with a trailing slash if `trailingSlash: "ignore"` is set. The existing behaviours of `trailingSlash: "always"` and `trailingSlash: "never"` are unchanged.

To migrate to this new behaviour, if your `base` configuration already has a trailing slash, no change is needed. If your `base` configuration does not have a trailing slash, you can add one to preserve the previous behaviour, or make sure any usages of `import.meta.env.BASE_URL` works without the trailing slash.
4 changes: 2 additions & 2 deletions packages/astro/e2e/astro-envs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ test.describe('Astro Environment BASE_URL', () => {
await page.goto(astro.resolveUrl('/blog/'));

const astroBaseUrl = page.locator('id=astro-base-url');
await expect(astroBaseUrl, 'astroBaseUrl equals to /blog/').toHaveText('/blog/');
await expect(astroBaseUrl, 'astroBaseUrl equals to /blog').toHaveText('/blog');

const clientComponentBaseUrl = page.locator('id=client-component-base-url');
await expect(clientComponentBaseUrl, 'clientComponentBaseUrl equals to /blog').toHaveText(
'/blog/'
'/blog'
);
});
});
5 changes: 3 additions & 2 deletions packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
} from '../../core/build/internal.js';
import {
isRelativePath,
joinPaths,
prependForwardSlash,
removeLeadingForwardSlash,
removeTrailingForwardSlash,
Expand Down Expand Up @@ -437,11 +438,11 @@ function getUrlForPath(
buildPathname = base;
} else if (routeType === 'endpoint') {
const buildPathRelative = removeLeadingForwardSlash(pathname);
buildPathname = base + buildPathRelative;
buildPathname = joinPaths(base, buildPathRelative);
} else {
const buildPathRelative =
removeTrailingForwardSlash(removeLeadingForwardSlash(pathname)) + ending;
buildPathname = base + buildPathRelative;
buildPathname = joinPaths(base, buildPathRelative);
}
const url = new URL(buildPathname, origin);
return url;
Expand Down
27 changes: 12 additions & 15 deletions packages/astro/src/core/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ import path from 'node:path';
import { pathToFileURL } from 'node:url';
import { BUNDLED_THEMES } from 'shiki';
import { z } from 'zod';
import { appendForwardSlash, prependForwardSlash, trimSlashes } from '../path.js';
import {
appendForwardSlash,
prependForwardSlash,
removeTrailingForwardSlash,
trimSlashes,
} from '../path.js';

const ASTRO_CONFIG_DEFAULTS = {
root: '.',
Expand Down Expand Up @@ -366,22 +371,14 @@ export function createRelativeSchema(cmd: string, fileProtocolRoot: string) {
) {
config.build.client = new URL('./dist/client/', config.outDir);
}
const trimmedBase = trimSlashes(config.base);

// If there is no base but there is a base for site config, warn.
const sitePathname = config.site && new URL(config.site).pathname;
if (!trimmedBase.length && sitePathname && sitePathname !== '/') {
config.base = sitePathname;
/* eslint-disable no-console */
console.warn(`The site configuration value includes a pathname of ${sitePathname} but there is no base configuration.
A future version of Astro will stop using the site pathname when producing <link> and <script> tags. Set your site's base with the base configuration.`);
}

if (trimmedBase.length && config.trailingSlash === 'never') {
config.base = prependForwardSlash(trimmedBase);
// Handle `base` trailing slash based on `trailingSlash` config
if (config.trailingSlash === 'never') {
config.base = prependForwardSlash(removeTrailingForwardSlash(config.base));
} else if (config.trailingSlash === 'always') {
config.base = prependForwardSlash(appendForwardSlash(config.base));
} else {
config.base = prependForwardSlash(appendForwardSlash(trimmedBase));
config.base = prependForwardSlash(config.base);
}

return config;
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 @@ -109,15 +109,15 @@ describe('Environment Variables', () => {
expect(res.status).to.equal(200);
let indexHtml = await res.text();
let $ = cheerio.load(indexHtml);
expect($('#base-url').text()).to.equal('/blog/');
expect($('#base-url').text()).to.equal('/blog');
});

it('does render destructured builtin SITE env', async () => {
let res = await fixture.fetch('/blog/destructured/');
expect(res.status).to.equal(200);
let indexHtml = await res.text();
let $ = cheerio.load(indexHtml);
expect($('#base-url').text()).to.equal('/blog/');
expect($('#base-url').text()).to.equal('/blog');
});
});
});
6 changes: 3 additions & 3 deletions packages/astro/test/astro-global.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ describe('Astro Global', () => {
const html = await fixture.readFile('/index.html');
const $ = cheerio.load(html);

expect($('#pathname').text()).to.equal('/blog/');
expect($('#pathname').text()).to.equal('/blog');
expect($('#searchparams').text()).to.equal('{}');
expect($('#child-pathname').text()).to.equal('/blog/');
expect($('#nested-child-pathname').text()).to.equal('/blog/');
expect($('#child-pathname').text()).to.equal('/blog');
expect($('#nested-child-pathname').text()).to.equal('/blog');
});

it('Astro.site', async () => {
Expand Down
8 changes: 4 additions & 4 deletions packages/astro/test/dev-routing.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,9 @@ describe('Development Routing', () => {
expect(response.status).to.equal(200);
});

it('404 when loading subpath root without trailing slash', async () => {
it('200 when loading subpath root without trailing slash', async () => {
const response = await fixture.fetch('/blog');
expect(response.status).to.equal(404);
expect(response.status).to.equal(200);
});

it('200 when loading another page with subpath used', async () => {
Expand Down Expand Up @@ -163,9 +163,9 @@ describe('Development Routing', () => {
expect(response.status).to.equal(200);
});

it('404 when loading subpath root without trailing slash', async () => {
it('200 when loading subpath root without trailing slash', async () => {
const response = await fixture.fetch('/blog');
expect(response.status).to.equal(404);
expect(response.status).to.equal(200);
});

it('200 when loading another page with subpath used', async () => {
Expand Down
8 changes: 4 additions & 4 deletions packages/astro/test/preview-routing.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,9 @@ describe('Preview Routing', () => {
expect(response.status).to.equal(200);
});

it('404 when loading subpath root without trailing slash', async () => {
it('200 when loading subpath root without trailing slash', async () => {
const response = await fixture.fetch('/blog');
expect(response.status).to.equal(404);
expect(response.status).to.equal(200);
});

it('200 when loading another page with subpath used', async () => {
Expand Down Expand Up @@ -345,9 +345,9 @@ describe('Preview Routing', () => {
expect(response.status).to.equal(200);
});

it('404 when loading subpath root without trailing slash', async () => {
it('200 when loading subpath root without trailing slash', async () => {
const response = await fixture.fetch('/blog');
expect(response.status).to.equal(404);
expect(response.status).to.equal(200);
});

it('200 when loading another page with subpath used', async () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/test/public-base-404.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ describe('Public dev with base', () => {
expect(response.status).to.equal(404);
const html = await response.text();
$ = cheerio.load(html);
expect($('a').first().text()).to.equal('/blog/');
expect($('a').first().text()).to.equal('/blog');
});

it('default 404 page when loading /none/', async () => {
Expand Down
7 changes: 0 additions & 7 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 5775b4f

Please sign in to comment.