summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Matthew Phillips <matthew@skypack.dev> 2022-11-11 11:34:34 -0800
committerGravatar GitHub <noreply@github.com> 2022-11-11 11:34:34 -0800
commitbee8c14afd475ad053b9fdf78a2d29bebdd86926 (patch)
tree0c6bcc50b58a8ec378298d8e32d890a59090b439
parentf47fb995c0dd5943deffbab082abbfb3977d062b (diff)
downloadastro-bee8c14afd475ad053b9fdf78a2d29bebdd86926.tar.gz
astro-bee8c14afd475ad053b9fdf78a2d29bebdd86926.tar.zst
astro-bee8c14afd475ad053b9fdf78a2d29bebdd86926.zip
Use base rather than site to create subpath for links/scripts (#5371)
* Use base rather than site to create subpath for links/scripts * Adding a changeset * Warn when the site has a pathname but not using base * fix asset test * fix asset inlining behavior
-rw-r--r--.changeset/thirty-cheetahs-repeat.md5
-rw-r--r--packages/astro/src/core/app/index.ts4
-rw-r--r--packages/astro/src/core/build/generate.ts10
-rw-r--r--packages/astro/src/core/build/vite-plugin-hoisted-scripts.ts5
-rw-r--r--packages/astro/src/core/build/vite-plugin-ssr.ts11
-rw-r--r--packages/astro/src/core/config/schema.ts20
-rw-r--r--packages/astro/src/core/render/ssr-element.ts24
-rw-r--r--packages/astro/test/asset-url-base.test.js52
-rw-r--r--packages/astro/test/astro-scripts.test.js80
-rw-r--r--packages/astro/test/fixtures/asset-url-base/package.json11
-rw-r--r--packages/astro/test/fixtures/asset-url-base/src/pages/index.astro13
-rw-r--r--packages/astro/test/fixtures/ssr-request/src/pages/request.astro3
-rw-r--r--packages/astro/test/ssr-request.test.js24
-rw-r--r--pnpm-lock.yaml6
14 files changed, 206 insertions, 62 deletions
diff --git a/.changeset/thirty-cheetahs-repeat.md b/.changeset/thirty-cheetahs-repeat.md
new file mode 100644
index 000000000..60c44fe7b
--- /dev/null
+++ b/.changeset/thirty-cheetahs-repeat.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Prefer the `base` config rather than site config for creating URLs for links and scripts.
diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts
index d06ff5280..188562e9d 100644
--- a/packages/astro/src/core/app/index.ts
+++ b/packages/astro/src/core/app/index.ts
@@ -170,7 +170,7 @@ export class App {
const url = new URL(request.url);
const manifest = this.#manifest;
const info = this.#routeDataToRouteInfo.get(routeData!)!;
- const links = createLinkStylesheetElementSet(info.links, manifest.site);
+ const links = createLinkStylesheetElementSet(info.links);
let scripts = new Set<SSRElement>();
for (const script of info.scripts) {
@@ -182,7 +182,7 @@ export class App {
});
}
} else {
- scripts.add(createModuleScriptElement(script, manifest.site));
+ scripts.add(createModuleScriptElement(script));
}
}
diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts
index 039e8283c..a1e6e6489 100644
--- a/packages/astro/src/core/build/generate.ts
+++ b/packages/astro/src/core/build/generate.ts
@@ -291,14 +291,8 @@ async function generatePath(
debug('build', `Generating: ${pathname}`);
- // If a base path was provided, append it to the site URL. This ensures that
- // all injected scripts and links are referenced relative to the site and subpath.
- const site =
- settings.config.base !== '/'
- ? joinPaths(settings.config.site?.toString() || 'http://localhost/', settings.config.base)
- : settings.config.site;
- const links = createLinkStylesheetElementSet(linkIds, site);
- const scripts = createModuleScriptsSet(hoistedScripts ? [hoistedScripts] : [], site);
+ const links = createLinkStylesheetElementSet(linkIds, settings.config.base);
+ const scripts = createModuleScriptsSet(hoistedScripts ? [hoistedScripts] : [], settings.config.base);
if (settings.scripts.some((script) => script.stage === 'page')) {
const hashedFilePath = internals.entrySpecifierToBundleMap.get(PAGE_SCRIPT_ID);
diff --git a/packages/astro/src/core/build/vite-plugin-hoisted-scripts.ts b/packages/astro/src/core/build/vite-plugin-hoisted-scripts.ts
index 23230035c..7a90f00bb 100644
--- a/packages/astro/src/core/build/vite-plugin-hoisted-scripts.ts
+++ b/packages/astro/src/core/build/vite-plugin-hoisted-scripts.ts
@@ -40,7 +40,10 @@ export function vitePluginHoistedScripts(
},
async generateBundle(_options, bundle) {
- let assetInlineLimit = settings.config.vite?.build?.assetsInlineLimit || 4096;
+ let assetInlineLimit = 4096;
+ if(settings.config.vite?.build && settings.config.vite.build.assetsInlineLimit !== undefined) {
+ assetInlineLimit = settings.config.vite?.build.assetsInlineLimit;
+ }
// Find all page entry points and create a map of the entry point to the hashed hoisted script.
// This is used when we render so that we can add the script to the head.
diff --git a/packages/astro/src/core/build/vite-plugin-ssr.ts b/packages/astro/src/core/build/vite-plugin-ssr.ts
index 33784083c..8cbc0a1b6 100644
--- a/packages/astro/src/core/build/vite-plugin-ssr.ts
+++ b/packages/astro/src/core/build/vite-plugin-ssr.ts
@@ -133,17 +133,22 @@ function buildManifest(
staticFiles.push(entryModules[PAGE_SCRIPT_ID]);
}
+ const bareBase = removeTrailingForwardSlash(removeLeadingForwardSlash(settings.config.base));
+ const joinBase = (pth: string) => (bareBase ? bareBase + '/' + pth : pth);
+
for (const pageData of eachPageData(internals)) {
const scripts: SerializedRouteInfo['scripts'] = [];
if (pageData.hoistedScript) {
- scripts.unshift(pageData.hoistedScript);
+ scripts.unshift(Object.assign({}, pageData.hoistedScript, {
+ value: joinBase(pageData.hoistedScript.value)
+ }));
}
if (settings.scripts.some((script) => script.stage === 'page')) {
scripts.push({ type: 'external', value: entryModules[PAGE_SCRIPT_ID] });
}
- const bareBase = removeTrailingForwardSlash(removeLeadingForwardSlash(settings.config.base));
- const links = sortedCSS(pageData).map((pth) => (bareBase ? bareBase + '/' + pth : pth));
+
+ const links = sortedCSS(pageData).map((pth) => joinBase(pth));
routes.push({
file: '',
diff --git a/packages/astro/src/core/config/schema.ts b/packages/astro/src/core/config/schema.ts
index ac70ec337..30b9f65dd 100644
--- a/packages/astro/src/core/config/schema.ts
+++ b/packages/astro/src/core/config/schema.ts
@@ -323,11 +323,23 @@ export function createRelativeSchema(cmd: string, fileProtocolRoot: URL) {
config.build.client = new URL('./dist/client/', config.outDir);
}
const trimmedBase = trimSlashes(config.base);
- if (trimmedBase.length && config.trailingSlash === 'never') {
- config.base = prependForwardSlash(trimmedBase);
- } else {
- config.base = prependForwardSlash(appendForwardSlash(trimmedBase));
+
+ // 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);
+ } else {
+ config.base = prependForwardSlash(appendForwardSlash(trimmedBase));
+ }
+
return config;
});
diff --git a/packages/astro/src/core/render/ssr-element.ts b/packages/astro/src/core/render/ssr-element.ts
index 4828583ee..63c02ff3e 100644
--- a/packages/astro/src/core/render/ssr-element.ts
+++ b/packages/astro/src/core/render/ssr-element.ts
@@ -3,34 +3,34 @@ import type { SSRElement } from '../../@types/astro';
import npath from 'path-browserify';
import { appendForwardSlash } from '../../core/path.js';
-function getRootPath(site?: string): string {
- return appendForwardSlash(new URL(site || 'http://localhost/').pathname);
+function getRootPath(base?: string): string {
+ return appendForwardSlash(new URL(base || '/', 'http://localhost/').pathname);
}
-function joinToRoot(href: string, site?: string): string {
- return npath.posix.join(getRootPath(site), href);
+function joinToRoot(href: string, base?: string): string {
+ return npath.posix.join(getRootPath(base), href);
}
-export function createLinkStylesheetElement(href: string, site?: string): SSRElement {
+export function createLinkStylesheetElement(href: string, base?: string): SSRElement {
return {
props: {
rel: 'stylesheet',
- href: joinToRoot(href, site),
+ href: joinToRoot(href, base),
},
children: '',
};
}
-export function createLinkStylesheetElementSet(hrefs: string[], site?: string) {
- return new Set<SSRElement>(hrefs.map((href) => createLinkStylesheetElement(href, site)));
+export function createLinkStylesheetElementSet(hrefs: string[], base?: string) {
+ return new Set<SSRElement>(hrefs.map((href) => createLinkStylesheetElement(href, base)));
}
export function createModuleScriptElement(
script: { type: 'inline' | 'external'; value: string },
- site?: string
+ base?: string
): SSRElement {
if (script.type === 'external') {
- return createModuleScriptElementWithSrc(script.value, site);
+ return createModuleScriptElementWithSrc(script.value, base);
} else {
return {
props: {
@@ -60,7 +60,7 @@ export function createModuleScriptElementWithSrcSet(
export function createModuleScriptsSet(
scripts: { type: 'inline' | 'external'; value: string }[],
- site?: string
+ base?: string
): Set<SSRElement> {
- return new Set<SSRElement>(scripts.map((script) => createModuleScriptElement(script, site)));
+ return new Set<SSRElement>(scripts.map((script) => createModuleScriptElement(script, base)));
}
diff --git a/packages/astro/test/asset-url-base.test.js b/packages/astro/test/asset-url-base.test.js
new file mode 100644
index 000000000..a6d1c0677
--- /dev/null
+++ b/packages/astro/test/asset-url-base.test.js
@@ -0,0 +1,52 @@
+import { expect } from 'chai';
+import * as cheerio from 'cheerio';
+import { loadFixture } from './test-utils.js';
+
+describe('Asset URL resolution in build', () => {
+ /** @type {import('./test-utils').Fixture} */
+ let fixture;
+
+ describe('With site and base', async () => {
+ describe('with site', () => {
+ before(async () => {
+ fixture = await loadFixture({
+ root: './fixtures/asset-url-base/',
+ site: 'http://example.com/sub/path/'
+ });
+ await fixture.build();
+ });
+
+ it('does not include the site\'s subpath', async () => {
+ const html = await fixture.readFile('/index.html');
+ const $ = cheerio.load(html);
+ const href = $('link[rel=stylesheet]').attr('href');
+ expect(href.startsWith('/sub/path/')).to.equal(false);
+ });
+ });
+
+ describe('with site and base', () => {
+ before(async () => {
+ fixture = await loadFixture({
+ root: './fixtures/asset-url-base/',
+ site: 'http://example.com/sub/path/',
+ base: '/another/base/'
+ });
+ await fixture.build();
+ });
+
+ it('does not include the site\'s subpath', async () => {
+ const html = await fixture.readFile('/index.html');
+ const $ = cheerio.load(html);
+ const href = $('link[rel=stylesheet]').attr('href');
+ expect(href.startsWith('/sub/path/')).to.equal(false);
+ });
+
+ it('does include the base subpath', async () => {
+ const html = await fixture.readFile('/index.html');
+ const $ = cheerio.load(html);
+ const href = $('link[rel=stylesheet]').attr('href');
+ expect(href.startsWith('/another/base/')).to.equal(true);
+ });
+ });
+ });
+});
diff --git a/packages/astro/test/astro-scripts.test.js b/packages/astro/test/astro-scripts.test.js
index 062b3c044..441f91b76 100644
--- a/packages/astro/test/astro-scripts.test.js
+++ b/packages/astro/test/astro-scripts.test.js
@@ -3,20 +3,18 @@ import * as cheerio from 'cheerio';
import { loadFixture } from './test-utils.js';
describe('Scripts (hoisted and not)', () => {
- let fixture;
- before(async () => {
- fixture = await loadFixture({
- root: './fixtures/astro-scripts/',
- vite: {
- build: {
- assetsInlineLimit: 0,
- },
- },
- });
- });
-
describe('Build', () => {
+ /** @type {import('./test-utils').Fixture} */
+ let fixture;
before(async () => {
+ fixture = await loadFixture({
+ root: './fixtures/astro-scripts/',
+ vite: {
+ build: {
+ assetsInlineLimit: 0,
+ },
+ },
+ });
await fixture.build();
});
@@ -45,11 +43,8 @@ describe('Scripts (hoisted and not)', () => {
// test 1: Just one entry module
expect($el).to.have.lengthOf(1);
- // test 2: attr removed
- expect($el.attr('data-astro')).to.equal(undefined);
-
- expect($el.attr('src')).to.equal(undefined);
- const inlineEntryJS = $el.text();
+ const src = $el.attr('src');
+ const inlineEntryJS = await fixture.readFile(src);
// test 3: the JS exists
expect(inlineEntryJS).to.be.ok;
@@ -69,21 +64,6 @@ describe('Scripts (hoisted and not)', () => {
expect($('script').attr('src')).to.not.equal(undefined);
});
- it('External page builds the hoisted scripts to a single bundle', async () => {
- let external = await fixture.readFile('/external/index.html');
- let $ = cheerio.load(external);
-
- // test 1: there are two scripts
- expect($('script')).to.have.lengthOf(2);
-
- let el = $('script').get(1);
- expect($(el).attr('src')).to.equal(undefined, 'This should have been inlined');
- let externalEntryJS = $(el).text();
-
- // test 2: the JS exists
- expect(externalEntryJS).to.be.ok;
- });
-
it('External page using non-hoist scripts that are modules are built standalone', async () => {
let external = await fixture.readFile('/external-no-hoist/index.html');
let $ = cheerio.load(external);
@@ -122,12 +102,48 @@ describe('Scripts (hoisted and not)', () => {
// Imported styles + tailwind
expect($('link[rel=stylesheet]')).to.have.a.lengthOf(2);
});
+
+ describe('Inlining', () => {
+ /** @type {import('./test-utils').Fixture} */
+ let fixture;
+ before(async () => {
+ fixture = await loadFixture({
+ root: './fixtures/astro-scripts/',
+ });
+ await fixture.build();
+ });
+
+ it('External page builds the hoisted scripts to a single bundle', async () => {
+ let external = await fixture.readFile('/external/index.html');
+ let $ = cheerio.load(external);
+
+ // test 1: there are two scripts
+ expect($('script')).to.have.lengthOf(2);
+
+ let el = $('script').get(1);
+ expect($(el).attr('src')).to.equal(undefined, 'This should have been inlined');
+ let externalEntryJS = $(el).text();
+
+ // test 2: the JS exists
+ expect(externalEntryJS).to.be.ok;
+ });
+ })
});
describe('Dev', () => {
+ /** @type {import('./test-utils').Fixture} */
+ let fixture;
/** @type {import('./test-utils').DevServer} */
let devServer;
before(async () => {
+ fixture = await loadFixture({
+ root: './fixtures/astro-scripts/',
+ vite: {
+ build: {
+ assetsInlineLimit: 0,
+ },
+ },
+ });
devServer = await fixture.startDevServer();
});
diff --git a/packages/astro/test/fixtures/asset-url-base/package.json b/packages/astro/test/fixtures/asset-url-base/package.json
new file mode 100644
index 000000000..187bcf931
--- /dev/null
+++ b/packages/astro/test/fixtures/asset-url-base/package.json
@@ -0,0 +1,11 @@
+{
+ "name": "@test/asset-url-base",
+ "version": "0.0.0",
+ "private": true,
+ "dependencies": {
+ "astro": "workspace:*"
+ },
+ "scripts": {
+ "dev": "astro dev"
+ }
+}
diff --git a/packages/astro/test/fixtures/asset-url-base/src/pages/index.astro b/packages/astro/test/fixtures/asset-url-base/src/pages/index.astro
new file mode 100644
index 000000000..d229ce325
--- /dev/null
+++ b/packages/astro/test/fixtures/asset-url-base/src/pages/index.astro
@@ -0,0 +1,13 @@
+<html>
+<head>
+ <title>Testing</title>
+ <style>
+ body {
+ background: blue;
+ }
+ </style>
+</head>
+<body>
+ <h1>Testing</h1>
+</body>
+</html>
diff --git a/packages/astro/test/fixtures/ssr-request/src/pages/request.astro b/packages/astro/test/fixtures/ssr-request/src/pages/request.astro
index 0b0422dd5..8027e784d 100644
--- a/packages/astro/test/fixtures/ssr-request/src/pages/request.astro
+++ b/packages/astro/test/fixtures/ssr-request/src/pages/request.astro
@@ -10,6 +10,9 @@ const origin = Astro.url.origin;
background: orangered;
}
</style>
+ <script>
+ console.log('hello world');
+ </script>
</head>
<body>
<h1 id="origin">{origin}</h1>
diff --git a/packages/astro/test/ssr-request.test.js b/packages/astro/test/ssr-request.test.js
index 8f5c242a4..42817abc5 100644
--- a/packages/astro/test/ssr-request.test.js
+++ b/packages/astro/test/ssr-request.test.js
@@ -13,6 +13,11 @@ describe('Using Astro.request in SSR', () => {
adapter: testAdapter(),
output: 'server',
base: '/subpath/',
+ vite: {
+ build: {
+ assetsInlineLimit: 0
+ }
+ }
});
await fixture.build();
});
@@ -51,6 +56,25 @@ describe('Using Astro.request in SSR', () => {
expect(css).to.not.be.an('undefined');
});
+ it('script assets have their base prefix', async () => {
+ const app = await fixture.loadTestAdapterApp();
+ let request = new Request('http://example.com/subpath/request');
+ let response = await app.render(request);
+ expect(response.status).to.equal(200);
+ const html = await response.text();
+ const $ = cheerioLoad(html);
+
+ const scriptSrc = $('script').attr('src');
+ expect(scriptSrc.startsWith('/subpath/')).to.equal(true);
+
+ request = new Request('http://example.com' + scriptSrc);
+ response = await app.render(request);
+
+ expect(response.status).to.equal(200);
+ const js = await response.text();
+ expect(js).to.not.be.an('undefined');
+ });
+
it('assets can be fetched', async () => {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/subpath/cars.json');
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
index 51794b2ce..db3b0c924 100644
--- a/pnpm-lock.yaml
+++ b/pnpm-lock.yaml
@@ -1150,6 +1150,12 @@ importers:
dependencies:
astro: link:../../..
+ packages/astro/test/fixtures/asset-url-base:
+ specifiers:
+ astro: workspace:*
+ dependencies:
+ astro: link:../../..
+
packages/astro/test/fixtures/astro pages:
specifiers:
astro: workspace:*