Skip to content

Commit

Permalink
Preserve base slash when trailingSlash ignore (#7878)
Browse files Browse the repository at this point in the history
  • Loading branch information
bluwy committed Aug 9, 2023
1 parent 0f1e9d8 commit 0f637c7
Show file tree
Hide file tree
Showing 10 changed files with 42 additions and 39 deletions.
15 changes: 15 additions & 0 deletions .changeset/six-grapes-look.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
'astro': major
---

The value of `import.meta.env.BASE_URL`, which is derived from the `base` option, will no longer have a trailing slash added by default or when `trailingSlash: "ignore"` is set. The existing behavior of `base` in combination with `trailingSlash: "always"` or `trailingSlash: "never"` is unchanged.

If your `base` already has a trailing slash, no change is needed.

If your `base` does not have a trailing slash, add one to preserve the previous behaviour:

```diff
// astro.config.mjs
- base: 'my-base',
+ base: 'my-base/',
```
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'
);
});
});
7 changes: 1 addition & 6 deletions packages/astro/src/@types/astro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -572,12 +572,7 @@ export interface AstroUserConfig {
*
* When using this option, all of your static asset imports and URLs should add the base as a prefix. You can access this value via `import.meta.env.BASE_URL`.
*
* By default, the value of `import.meta.env.BASE_URL` includes a trailing slash. If you have the [`trailingSlash`](https://docs.astro.build/en/reference/configuration-reference/#trailingslash) option set to `'never'`, you will need to add it manually in your static asset imports and URLs.
*
* ```astro
* <a href="/docs/about/">About</a>
* <img src=`${import.meta.env.BASE_URL}image.png`>
* ```
* The value of `import.meta.env.BASE_URL` respects your `trailingSlash` config and will include a trailing slash if you explicitly include one or if `trailingSlash: "always"` is set. If `trailingSlash: "never"` is set, `BASE_URL` will not include a trailing slash, even if `base` includes one.
*/
base?: string;

Expand Down
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
22 changes: 7 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,7 @@ 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 } from '../path.js';

const ASTRO_CONFIG_DEFAULTS = {
root: '.',
Expand Down Expand Up @@ -366,22 +366,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

0 comments on commit 0f637c7

Please sign in to comment.