From 2f05f5d3071f01bf011212b5a91a5ac0c84fcff1 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Tue, 30 Aug 2022 11:45:16 -0400 Subject: [PATCH] Include trailingSlash in astro:build:done hook (#4553) --- .changeset/moody-cars-return.md | 7 ++ packages/astro/src/core/build/generate.ts | 20 ++++- packages/integrations/sitemap/package.json | 3 +- .../fixtures/trailing-slash/astro.config.mjs | 7 ++ .../test/fixtures/trailing-slash/package.json | 9 +++ .../trailing-slash/src/pages/one.astro | 8 ++ .../trailing-slash/src/pages/two.astro | 8 ++ .../integrations/sitemap/test/test-utils.js | 30 +++++++ .../sitemap/test/trailing-slash.test.js | 79 +++++++++++++++++++ pnpm-lock.yaml | 24 +++++- 10 files changed, 192 insertions(+), 3 deletions(-) create mode 100644 .changeset/moody-cars-return.md create mode 100644 packages/integrations/sitemap/test/fixtures/trailing-slash/astro.config.mjs create mode 100644 packages/integrations/sitemap/test/fixtures/trailing-slash/package.json create mode 100644 packages/integrations/sitemap/test/fixtures/trailing-slash/src/pages/one.astro create mode 100644 packages/integrations/sitemap/test/fixtures/trailing-slash/src/pages/two.astro create mode 100644 packages/integrations/sitemap/test/test-utils.js create mode 100644 packages/integrations/sitemap/test/trailing-slash.test.js diff --git a/.changeset/moody-cars-return.md b/.changeset/moody-cars-return.md new file mode 100644 index 000000000..462123c98 --- /dev/null +++ b/.changeset/moody-cars-return.md @@ -0,0 +1,7 @@ +--- +'astro': patch +--- + +Include trailingSlash in astro:build:done hook + +This change ensures that the `pages` provided in the `astro:build:done` hook conform to the `trailingSlash` and `build.format` configs. diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index d59b650b4..2f70b9bf5 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -240,8 +240,26 @@ interface GeneratePathOptions { renderers: SSRLoadedRenderer[]; } +function shouldAppendForwardSlash(trailingSlash: AstroConfig['trailingSlash'], buildFormat: AstroConfig['build']['format']): boolean { + switch(trailingSlash) { + case 'always': return true; + case 'never': return false; + case 'ignore': { + switch(buildFormat) { + case 'directory': return true; + case 'file': return false; + } + } + } +} + function addPageName(pathname: string, opts: StaticBuildOptions): void { - opts.pageNames.push(pathname.replace(/^\//, '')); + const trailingSlash = opts.astroConfig.trailingSlash; + const buildFormat = opts.astroConfig.build.format; + const pageName = shouldAppendForwardSlash(trailingSlash, buildFormat) ? + pathname.replace(/\/?$/, '/').replace(/^\//, '') : + pathname.replace(/^\//, '') + opts.pageNames.push(pageName); } function getUrlForPath( diff --git a/packages/integrations/sitemap/package.json b/packages/integrations/sitemap/package.json index 33f650b56..fa66a1189 100644 --- a/packages/integrations/sitemap/package.json +++ b/packages/integrations/sitemap/package.json @@ -38,6 +38,7 @@ }, "devDependencies": { "astro": "workspace:*", - "astro-scripts": "workspace:*" + "astro-scripts": "workspace:*", + "xml2js": "0.4.23" } } diff --git a/packages/integrations/sitemap/test/fixtures/trailing-slash/astro.config.mjs b/packages/integrations/sitemap/test/fixtures/trailing-slash/astro.config.mjs new file mode 100644 index 000000000..7d02e26ca --- /dev/null +++ b/packages/integrations/sitemap/test/fixtures/trailing-slash/astro.config.mjs @@ -0,0 +1,7 @@ +import { defineConfig } from 'astro/config'; +import sitemap from '@astrojs/sitemap'; + +export default defineConfig({ + integrations: [sitemap()], + site: 'http://example.com' +}) diff --git a/packages/integrations/sitemap/test/fixtures/trailing-slash/package.json b/packages/integrations/sitemap/test/fixtures/trailing-slash/package.json new file mode 100644 index 000000000..980e02e73 --- /dev/null +++ b/packages/integrations/sitemap/test/fixtures/trailing-slash/package.json @@ -0,0 +1,9 @@ +{ + "name": "@test/sitemap-trailing-slash", + "version": "0.0.0", + "private": true, + "dependencies": { + "astro": "workspace:*", + "@astrojs/sitemap": "workspace:*" + } +} diff --git a/packages/integrations/sitemap/test/fixtures/trailing-slash/src/pages/one.astro b/packages/integrations/sitemap/test/fixtures/trailing-slash/src/pages/one.astro new file mode 100644 index 000000000..0c7fb90a7 --- /dev/null +++ b/packages/integrations/sitemap/test/fixtures/trailing-slash/src/pages/one.astro @@ -0,0 +1,8 @@ + + + One + + +

One

+ + diff --git a/packages/integrations/sitemap/test/fixtures/trailing-slash/src/pages/two.astro b/packages/integrations/sitemap/test/fixtures/trailing-slash/src/pages/two.astro new file mode 100644 index 000000000..e7ba9910e --- /dev/null +++ b/packages/integrations/sitemap/test/fixtures/trailing-slash/src/pages/two.astro @@ -0,0 +1,8 @@ + + + Two + + +

Two

+ + diff --git a/packages/integrations/sitemap/test/test-utils.js b/packages/integrations/sitemap/test/test-utils.js new file mode 100644 index 000000000..22c1d1919 --- /dev/null +++ b/packages/integrations/sitemap/test/test-utils.js @@ -0,0 +1,30 @@ +import { loadFixture as baseLoadFixture } from '../../../astro/test/test-utils.js'; +import * as xml2js from 'xml2js'; + +/** + * @typedef {import('../../../astro/test/test-utils').Fixture} Fixture + */ + +export function loadFixture(inlineConfig) { + if (!inlineConfig || !inlineConfig.root) + throw new Error("Must provide { root: './fixtures/...' }"); + + // resolve the relative root (i.e. "./fixtures/tailwindcss") to a full filepath + // without this, the main `loadFixture` helper will resolve relative to `packages/astro/test` + return baseLoadFixture({ + ...inlineConfig, + root: new URL(inlineConfig.root, import.meta.url).toString(), + }); +} + +export function readXML(fileOrPromise) { + const parseString = xml2js.parseString; + return Promise.resolve(fileOrPromise).then(xml => { + return new Promise((resolve, reject) => { + parseString(xml, function (err, result) { + if(err) return reject(err); + resolve(result); + }); + }) + }); +} diff --git a/packages/integrations/sitemap/test/trailing-slash.test.js b/packages/integrations/sitemap/test/trailing-slash.test.js new file mode 100644 index 000000000..6b6aa3a35 --- /dev/null +++ b/packages/integrations/sitemap/test/trailing-slash.test.js @@ -0,0 +1,79 @@ +import { loadFixture, readXML } from './test-utils.js'; +import { expect } from 'chai'; + +describe('Trailing slash', () => { + /** @type {import('./test-utils').Fixture} */ + let fixture; + + describe('trailingSlash: ignore', () => { + describe('build.format: directory', () => { + before(async () => { + fixture = await loadFixture({ + root: './fixtures/trailing-slash/', + trailingSlash: 'ignore', + build: { + format: 'directory' + } + }); + await fixture.build(); + }); + + it('URLs end with trailing slash', async () => { + const data = await readXML(fixture.readFile('/sitemap-0.xml')); + const urls = data.urlset.url; + expect(urls[0].loc[0]).to.equal('http://example.com/one/'); + }); + }); + + describe('build.format: file', () => { + before(async () => { + fixture = await loadFixture({ + root: './fixtures/trailing-slash/', + trailingSlash: 'ignore', + build: { + format: 'file' + } + }); + await fixture.build(); + }); + + it('URLs do not end with trailing slash', async () => { + const data = await readXML(fixture.readFile('/sitemap-0.xml')); + const urls = data.urlset.url; + expect(urls[0].loc[0]).to.equal('http://example.com/one'); + }); + }); + }); + + describe('trailingSlash: never', () => { + before(async () => { + fixture = await loadFixture({ + root: './fixtures/trailing-slash/', + trailingSlash: 'never', + }); + await fixture.build(); + }); + + it('URLs do no end with trailing slash', async () => { + const data = await readXML(fixture.readFile('/sitemap-0.xml')); + const urls = data.urlset.url; + expect(urls[0].loc[0]).to.equal('http://example.com/one'); + }); + }); + + describe('trailingSlash: always', () => { + before(async () => { + fixture = await loadFixture({ + root: './fixtures/trailing-slash/', + trailingSlash: 'always', + }); + await fixture.build(); + }); + + it('URLs end with trailing slash', async () => { + const data = await readXML(fixture.readFile('/sitemap-0.xml')); + const urls = data.urlset.url; + expect(urls[0].loc[0]).to.equal('http://example.com/one/'); + }); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9621c4343..0e400764f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2552,6 +2552,7 @@ importers: astro: workspace:* astro-scripts: workspace:* sitemap: ^7.1.1 + xml2js: 0.4.23 zod: ^3.17.3 dependencies: sitemap: 7.1.1 @@ -2559,6 +2560,15 @@ importers: devDependencies: astro: link:../../astro astro-scripts: link:../../../scripts + xml2js: 0.4.23 + + packages/integrations/sitemap/test/fixtures/trailing-slash: + specifiers: + '@astrojs/sitemap': workspace:* + astro: workspace:* + dependencies: + '@astrojs/sitemap': link:../../.. + astro: link:../../../../../astro packages/integrations/solid: specifiers: @@ -15540,7 +15550,6 @@ packages: /sax/1.2.4: resolution: {integrity: sha512-NqVDv9TpANUjFm0N8uM5GxL36UgKi9/atZw+x7YFnQ8ckwFGKrl4xX4yWtrey3UJm5nP1kUbnYgLopqWNSRhWw==} - dev: false /scheduler/0.23.0: resolution: {integrity: sha512-CtuThmgHNg7zIZWAXi3AsyIzA3n4xx7aNyjwC2VJldO2LMVDhFK+63xGqq6CsJH4rTAt6/M+N4GhZiDYPx9eUw==} @@ -17443,6 +17452,19 @@ packages: optional: true dev: true + /xml2js/0.4.23: + resolution: {integrity: sha512-ySPiMjM0+pLDftHgXY4By0uswI3SPKLDw/i3UXbnO8M/p28zqexCUoPmQFrYD+/1BzhGJSs2i1ERWKJAtiLrug==} + engines: {node: '>=4.0.0'} + dependencies: + sax: 1.2.4 + xmlbuilder: 11.0.1 + dev: true + + /xmlbuilder/11.0.1: + resolution: {integrity: sha512-fDlsI/kFEx7gLvbecc0/ohLG50fugQp8ryHzMTuW9vSa1GJ0XYWKnhsUx7oie3G98+r56aTQIUB4kht42R3JvA==} + engines: {node: '>=4.0'} + dev: true + /xregexp/2.0.0: resolution: {integrity: sha512-xl/50/Cf32VsGq/1R8jJE5ajH1yMCQkpmoS10QbFZWl2Oor4H0Me64Pu2yxvsRWK3m6soJbmGfzSR7BYmDcWAA==} dev: true