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

Preserve base slash when trailingSlash ignore #7878

Merged
merged 1 commit into from
Aug 9, 2023
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
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.`);
}
bluwy marked this conversation as resolved.
Show resolved Hide resolved

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);
});
bluwy marked this conversation as resolved.
Show resolved Hide resolved

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