diff options
author | 2022-11-11 11:34:34 -0800 | |
---|---|---|
committer | 2022-11-11 11:34:34 -0800 | |
commit | bee8c14afd475ad053b9fdf78a2d29bebdd86926 (patch) | |
tree | 0c6bcc50b58a8ec378298d8e32d890a59090b439 | |
parent | f47fb995c0dd5943deffbab082abbfb3977d062b (diff) | |
download | astro-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.md | 5 | ||||
-rw-r--r-- | packages/astro/src/core/app/index.ts | 4 | ||||
-rw-r--r-- | packages/astro/src/core/build/generate.ts | 10 | ||||
-rw-r--r-- | packages/astro/src/core/build/vite-plugin-hoisted-scripts.ts | 5 | ||||
-rw-r--r-- | packages/astro/src/core/build/vite-plugin-ssr.ts | 11 | ||||
-rw-r--r-- | packages/astro/src/core/config/schema.ts | 20 | ||||
-rw-r--r-- | packages/astro/src/core/render/ssr-element.ts | 24 | ||||
-rw-r--r-- | packages/astro/test/asset-url-base.test.js | 52 | ||||
-rw-r--r-- | packages/astro/test/astro-scripts.test.js | 80 | ||||
-rw-r--r-- | packages/astro/test/fixtures/asset-url-base/package.json | 11 | ||||
-rw-r--r-- | packages/astro/test/fixtures/asset-url-base/src/pages/index.astro | 13 | ||||
-rw-r--r-- | packages/astro/test/fixtures/ssr-request/src/pages/request.astro | 3 | ||||
-rw-r--r-- | packages/astro/test/ssr-request.test.js | 24 | ||||
-rw-r--r-- | pnpm-lock.yaml | 6 |
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:* |