From d2020c29cf285e699f92143a70ffa30a85122bb4 Mon Sep 17 00:00:00 2001 From: Nate Moore Date: Fri, 16 Jun 2023 16:58:07 -0400 Subject: [PATCH] Fix injected route edge case (#7399) * fix(#7397): update astro page entryname logic * chore: add changeset * fix: update printed log for injected routes * fix: ensure /index is included * test: add custom-404-pkg test * chore: cleanup --- .changeset/large-ants-tap.md | 5 +++ packages/astro/src/core/build/generate.ts | 7 +++- packages/astro/src/core/build/static-build.ts | 38 +++++++++---------- .../test/custom-404-injected-from-dep.test.js | 22 +++++++++++ .../astro.config.mjs | 18 +++++++++ .../custom-404-injected-from-dep/package.json | 9 +++++ .../src/pages/index.astro | 11 ++++++ .../test/fixtures/custom-404-pkg/404.astro | 13 +++++++ .../test/fixtures/custom-404-pkg/package.json | 8 ++++ pnpm-lock.yaml | 17 ++++++++- 10 files changed, 127 insertions(+), 21 deletions(-) create mode 100644 .changeset/large-ants-tap.md create mode 100644 packages/astro/test/custom-404-injected-from-dep.test.js create mode 100644 packages/astro/test/fixtures/custom-404-injected-from-dep/astro.config.mjs create mode 100644 packages/astro/test/fixtures/custom-404-injected-from-dep/package.json create mode 100644 packages/astro/test/fixtures/custom-404-injected-from-dep/src/pages/index.astro create mode 100644 packages/astro/test/fixtures/custom-404-pkg/404.astro create mode 100644 packages/astro/test/fixtures/custom-404-pkg/package.json diff --git a/.changeset/large-ants-tap.md b/.changeset/large-ants-tap.md new file mode 100644 index 000000000..f99945aee --- /dev/null +++ b/.changeset/large-ants-tap.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fix edge case where injected routes would cause builds to fail in a PNPM workspace diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index c1fd34654..a6a1ece54 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -29,6 +29,7 @@ import { type BuildInternals, } from '../../core/build/internal.js'; import { + isRelativePath, prependForwardSlash, removeLeadingForwardSlash, removeTrailingForwardSlash, @@ -252,7 +253,11 @@ async function generatePage( }; const icon = pageData.route.type === 'page' ? green('▶') : magenta('λ'); - info(opts.logging, null, `${icon} ${pageData.route.component}`); + if (isRelativePath(pageData.route.component)) { + info(opts.logging, null, `${icon} ${pageData.route.route}`); + } else { + info(opts.logging, null, `${icon} ${pageData.route.component}`); + } // Get paths for the route, calling getStaticPaths if needed. const paths = await getPathsForRoute(pageData, pageModule, opts, builtPaths); diff --git a/packages/astro/src/core/build/static-build.ts b/packages/astro/src/core/build/static-build.ts index 72bfd6988..647b31983 100644 --- a/packages/astro/src/core/build/static-build.ts +++ b/packages/astro/src/core/build/static-build.ts @@ -30,7 +30,7 @@ import { } from './plugins/plugin-pages.js'; import { RESOLVED_RENDERERS_MODULE_ID } from './plugins/plugin-renderers.js'; import { SSR_VIRTUAL_MODULE_ID } from './plugins/plugin-ssr.js'; -import type { PageBuildData, StaticBuildOptions } from './types'; +import type { AllPagesData, PageBuildData, StaticBuildOptions } from './types'; import { getTimeStat } from './util.js'; export async function viteBuild(opts: StaticBuildOptions) { @@ -143,7 +143,7 @@ async function ssrBuild( input: Set, container: AstroBuildPluginContainer ) { - const { settings, viteConfig } = opts; + const { allPages, settings, viteConfig } = opts; const ssr = isServerLikeOutput(settings.config); const out = ssr ? opts.buildConfig.server : getOutDirWithinCwd(settings.config.outDir); @@ -175,7 +175,7 @@ async function ssrBuild( ...viteConfig.build?.rollupOptions?.output, entryFileNames(chunkInfo) { if (chunkInfo.facadeModuleId?.startsWith(ASTRO_PAGE_RESOLVED_MODULE_ID)) { - return makeAstroPageEntryPointFileName(chunkInfo.facadeModuleId); + return makeAstroPageEntryPointFileName(chunkInfo.facadeModuleId, allPages); } else if ( // checks if the path of the module we have middleware, e.g. middleware.js / middleware/index.js chunkInfo.moduleIds.find((m) => m.includes('middleware')) !== undefined && @@ -418,27 +418,27 @@ async function ssrMoveAssets(opts: StaticBuildOptions) { } /** - * This function takes as input the virtual module name of an astro page and transform - * to generate an `.mjs` file: + * This function takes the virtual module name of any page entrypoint and + * transforms it to generate a final `.mjs` output file. * * Input: `@astro-page:src/pages/index@_@astro` - * * Output: `pages/index.astro.mjs` + * Input: `@astro-page:../node_modules/my-dep/injected@_@astro` + * Output: `pages/injected.mjs` * - * 1. We remove the module id prefix, `@astro-page:` - * 2. We remove `src/` - * 3. We replace square brackets with underscore, for example `[slug]` - * 4. At last, we replace the extension pattern with a simple dot - * 5. We append the `.mjs` string, so the file will always be a JS file + * 1. We clean the `facadeModuleId` by removing the `@astro-page:` prefix and `@_@` suffix + * 2. We find the matching route pattern in the manifest (or fallback to the cleaned module id) + * 3. We replace square brackets with underscore (`[slug]` => `_slug_`) and `...` with `` (`[...slug]` => `_---slug_`). + * 4. We append the `.mjs` extension, so the file will always be an ESM module * - * @param facadeModuleId + * @param facadeModuleId string + * @param pages AllPagesData */ -function makeAstroPageEntryPointFileName(facadeModuleId: string) { - return `${facadeModuleId +function makeAstroPageEntryPointFileName(facadeModuleId: string, pages: AllPagesData) { + const pageModuleId = facadeModuleId .replace(ASTRO_PAGE_RESOLVED_MODULE_ID, '') - .replace('src/', '') - .replaceAll('[', '_') - .replaceAll(']', '_') - // this must be last - .replace(ASTRO_PAGE_EXTENSION_POST_PATTERN, '.')}.mjs`; + .replace(ASTRO_PAGE_EXTENSION_POST_PATTERN, '.'); + let name = pages[pageModuleId]?.route?.route ?? pageModuleId; + if (name.endsWith('/')) name += 'index'; + return `pages${name.replaceAll('[', '_').replaceAll(']', '_').replaceAll('...', '---')}.mjs`; } diff --git a/packages/astro/test/custom-404-injected-from-dep.test.js b/packages/astro/test/custom-404-injected-from-dep.test.js new file mode 100644 index 000000000..d9a822f4e --- /dev/null +++ b/packages/astro/test/custom-404-injected-from-dep.test.js @@ -0,0 +1,22 @@ +import { expect } from 'chai'; +import { loadFixture } from './test-utils.js'; + +describe('Custom 404 with injectRoute from dependency', () => { + describe('build', () => { + /** @type {import('./test-utils.js').Fixture} */ + let fixture; + + before(async () => { + fixture = await loadFixture({ + root: './fixtures/custom-404-injected-from-dep/', + site: 'http://example.com', + }); + await fixture.build(); + }); + + it('Build succeeds', async () => { + const html = await fixture.readFile('/404.html'); + expect(html).to.contain(''); + }); + }); +}); diff --git a/packages/astro/test/fixtures/custom-404-injected-from-dep/astro.config.mjs b/packages/astro/test/fixtures/custom-404-injected-from-dep/astro.config.mjs new file mode 100644 index 000000000..840a9524c --- /dev/null +++ b/packages/astro/test/fixtures/custom-404-injected-from-dep/astro.config.mjs @@ -0,0 +1,18 @@ +import { defineConfig } from 'astro/config'; + +// https://astro.build/config +export default defineConfig({ + integrations: [ + { + name: '404-integration', + hooks: { + 'astro:config:setup': ({ injectRoute }) => { + injectRoute({ + pattern: '404', + entryPoint: '@test/custom-404-pkg/404.astro', + }); + }, + }, + }, + ], +}); diff --git a/packages/astro/test/fixtures/custom-404-injected-from-dep/package.json b/packages/astro/test/fixtures/custom-404-injected-from-dep/package.json new file mode 100644 index 000000000..6af832602 --- /dev/null +++ b/packages/astro/test/fixtures/custom-404-injected-from-dep/package.json @@ -0,0 +1,9 @@ +{ + "name": "@test/custom-404-injected-from-dep", + "version": "0.0.0", + "private": true, + "dependencies": { + "astro": "workspace:*", + "@test/custom-404-pkg": "workspace:*" + } +} diff --git a/packages/astro/test/fixtures/custom-404-injected-from-dep/src/pages/index.astro b/packages/astro/test/fixtures/custom-404-injected-from-dep/src/pages/index.astro new file mode 100644 index 000000000..cf5ef9b58 --- /dev/null +++ b/packages/astro/test/fixtures/custom-404-injected-from-dep/src/pages/index.astro @@ -0,0 +1,11 @@ +--- +--- + + + + Custom 404 + + +

Home

+ + diff --git a/packages/astro/test/fixtures/custom-404-pkg/404.astro b/packages/astro/test/fixtures/custom-404-pkg/404.astro new file mode 100644 index 000000000..63d560b0f --- /dev/null +++ b/packages/astro/test/fixtures/custom-404-pkg/404.astro @@ -0,0 +1,13 @@ +--- +const canonicalURL = new URL(Astro.url.pathname, Astro.site); +--- + + + + Not Found - Custom 404 + + +

Page not found

+

{canonicalURL.pathname}

+ + diff --git a/packages/astro/test/fixtures/custom-404-pkg/package.json b/packages/astro/test/fixtures/custom-404-pkg/package.json new file mode 100644 index 000000000..f32073534 --- /dev/null +++ b/packages/astro/test/fixtures/custom-404-pkg/package.json @@ -0,0 +1,8 @@ +{ + "name": "@test/custom-404-pkg", + "version": "0.0.0", + "private": true, + "dependencies": { + "astro": "workspace:*" + } +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 548b8b4d9..29a708e95 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1,4 +1,4 @@ -lockfileVersion: '6.1' +lockfileVersion: '6.0' settings: autoInstallPeers: false @@ -2499,12 +2499,27 @@ importers: specifier: workspace:* version: link:../../.. + packages/astro/test/fixtures/custom-404-injected-from-dep: + dependencies: + '@test/custom-404-pkg': + specifier: workspace:* + version: link:../custom-404-pkg + astro: + specifier: workspace:* + version: link:../../.. + packages/astro/test/fixtures/custom-404-md: dependencies: astro: specifier: workspace:* version: link:../../.. + packages/astro/test/fixtures/custom-404-pkg: + dependencies: + astro: + specifier: workspace:* + version: link:../../.. + packages/astro/test/fixtures/custom-assets-name: dependencies: '@astrojs/node':