From c9bb1147cbfae20e3ecdf29ef2866a183b3b18e3 Mon Sep 17 00:00:00 2001 From: "Fred K. Schott" Date: Fri, 28 Jan 2022 17:29:53 -0800 Subject: [PATCH] [MINOR] standardize trailing subpath, fix fetchContent url issue (#2471) * standardize trailing subpath, and fix fetchcontent issue * debug windows ci * improve ci test * fix windows test issue? * fix only usage * end debugging --- .changeset/old-parents-obey.md | 7 +++++++ .changeset/shaggy-shoes-leave.md | 7 +++++++ packages/astro/src/core/config.ts | 5 ++++- packages/astro/src/runtime/server/index.ts | 11 ++++++----- packages/astro/test/astro-global.test.js | 6 ++++++ packages/astro/test/dev-routing.test.js | 4 ++-- .../astro-global/src/pages/posts/[page].astro | 2 +- 7 files changed, 33 insertions(+), 9 deletions(-) create mode 100644 .changeset/old-parents-obey.md create mode 100644 .changeset/shaggy-shoes-leave.md diff --git a/.changeset/old-parents-obey.md b/.changeset/old-parents-obey.md new file mode 100644 index 0000000000..0e54404da8 --- /dev/null +++ b/.changeset/old-parents-obey.md @@ -0,0 +1,7 @@ +--- +'astro': patch +--- + +Respect subpath URL paths in the fetchContent url property. + +This fixes an issue where fetchContent() URL property did not include the buildOptions.site path in it. diff --git a/.changeset/shaggy-shoes-leave.md b/.changeset/shaggy-shoes-leave.md new file mode 100644 index 0000000000..a92abed9f4 --- /dev/null +++ b/.changeset/shaggy-shoes-leave.md @@ -0,0 +1,7 @@ +--- +'astro': minor +--- + +Standardize trailing subpath behavior in config. + +Most users are not aware of the subtle differences between `/foo` and `/foo/`. Internally, we have to handle both which means that we are constantly worrying about the format of the URL, needing to add/remove trailing slashes when we go to work with this property, etc. This change transforms all `site` values to use a trailing slash internally, which should help reduce bugs for both users and maintainers. \ No newline at end of file diff --git a/packages/astro/src/core/config.ts b/packages/astro/src/core/config.ts index 4c0b754490..1866778029 100644 --- a/packages/astro/src/core/config.ts +++ b/packages/astro/src/core/config.ts @@ -53,7 +53,10 @@ export const AstroConfigSchema = z.object({ .default({}), buildOptions: z .object({ - site: z.string().optional(), + site: z + .string() + .optional() + .transform((val) => (val ? addTrailingSlash(val) : val)), sitemap: z.boolean().optional().default(true), pageUrlFormat: z .union([z.literal('file'), z.literal('directory')]) diff --git a/packages/astro/src/runtime/server/index.ts b/packages/astro/src/runtime/server/index.ts index fd401fbc81..894f9867d8 100644 --- a/packages/astro/src/runtime/server/index.ts +++ b/packages/astro/src/runtime/server/index.ts @@ -262,7 +262,7 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr } /** Create the Astro.fetchContent() runtime function. */ -function createFetchContentFn(url: URL) { +function createFetchContentFn(url: URL, site: URL) { const fetchContent = (importMetaGlobResult: Record) => { let allEntries = [...Object.entries(importMetaGlobResult)]; if (allEntries.length === 0) { @@ -280,7 +280,7 @@ function createFetchContentFn(url: URL) { Content: mod.default, content: mod.metadata, file: new URL(spec, url), - url: urlSpec.includes('/pages/') ? urlSpec.replace(/^.*\/pages\//, '/').replace(/(\/index)?\.md$/, '') : undefined, + url: urlSpec.includes('/pages/') ? urlSpec.replace(/^.*\/pages\//, site.pathname).replace(/(\/index)?\.md$/, '') : undefined, }; }) .filter(Boolean); @@ -293,12 +293,13 @@ function createFetchContentFn(url: URL) { // This is used to create the top-level Astro global; the one that you can use // Inside of getStaticPaths. -export function createAstro(filePathname: string, site: string, projectRootStr: string): AstroGlobalPartial { +export function createAstro(filePathname: string, _site: string, projectRootStr: string): AstroGlobalPartial { + const site = new URL(_site); const url = new URL(filePathname, site); const projectRoot = new URL(projectRootStr); - const fetchContent = createFetchContentFn(url); + const fetchContent = createFetchContentFn(url, site); return { - site: new URL(site), + site, fetchContent, // INVESTIGATE is there a use-case for multi args? resolve(...segments: string[]) { diff --git a/packages/astro/test/astro-global.test.js b/packages/astro/test/astro-global.test.js index 168d66bf96..e479eb4e6d 100644 --- a/packages/astro/test/astro-global.test.js +++ b/packages/astro/test/astro-global.test.js @@ -54,4 +54,10 @@ describe('Astro.*', () => { expect($('img').attr('src')).to.include('assets/penguin.ccd44411.png'); // Main src/images expect($('#inner-child img').attr('src')).to.include('assets/penguin.b9ab122a.png'); }); + + it('Astro.fetchContent() returns the correct "url" property, including buildOptions.site subpath', async () => { + const html = await fixture.readFile('/posts/1/index.html'); + const $ = cheerio.load(html); + expect($('.post-url').attr('href')).to.equal('/blog/post/post-2'); + }); }); diff --git a/packages/astro/test/dev-routing.test.js b/packages/astro/test/dev-routing.test.js index 1f56479359..1984458181 100644 --- a/packages/astro/test/dev-routing.test.js +++ b/packages/astro/test/dev-routing.test.js @@ -151,9 +151,9 @@ describe('Development Routing', () => { expect(response.status).to.equal(200); }); - it('200 when loading subpath root without trailing slash', async () => { + it('404 when loading subpath root without trailing slash', async () => { const response = await fixture.fetch('/blog'); - expect(response.status).to.equal(200); + expect(response.status).to.equal(404); }); it('200 when loading another page with subpath used', async () => { diff --git a/packages/astro/test/fixtures/astro-global/src/pages/posts/[page].astro b/packages/astro/test/fixtures/astro-global/src/pages/posts/[page].astro index e8df864eea..e684161e65 100644 --- a/packages/astro/test/fixtures/astro-global/src/pages/posts/[page].astro +++ b/packages/astro/test/fixtures/astro-global/src/pages/posts/[page].astro @@ -16,7 +16,7 @@ const { params, canonicalURL} = Astro.request; {page.data.map((data) => (

{data.title}

- Read + Read
))}