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
This commit is contained in:
Nate Moore 2023-06-16 16:58:07 -04:00 committed by GitHub
parent 556fd694a6
commit d2020c29cf
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 127 additions and 21 deletions

View file

@ -0,0 +1,5 @@
---
'astro': patch
---
Fix edge case where injected routes would cause builds to fail in a PNPM workspace

View file

@ -29,6 +29,7 @@ import {
type BuildInternals, type BuildInternals,
} from '../../core/build/internal.js'; } from '../../core/build/internal.js';
import { import {
isRelativePath,
prependForwardSlash, prependForwardSlash,
removeLeadingForwardSlash, removeLeadingForwardSlash,
removeTrailingForwardSlash, removeTrailingForwardSlash,
@ -252,7 +253,11 @@ async function generatePage(
}; };
const icon = pageData.route.type === 'page' ? green('▶') : magenta('λ'); 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. // Get paths for the route, calling getStaticPaths if needed.
const paths = await getPathsForRoute(pageData, pageModule, opts, builtPaths); const paths = await getPathsForRoute(pageData, pageModule, opts, builtPaths);

View file

@ -30,7 +30,7 @@ import {
} from './plugins/plugin-pages.js'; } from './plugins/plugin-pages.js';
import { RESOLVED_RENDERERS_MODULE_ID } from './plugins/plugin-renderers.js'; import { RESOLVED_RENDERERS_MODULE_ID } from './plugins/plugin-renderers.js';
import { SSR_VIRTUAL_MODULE_ID } from './plugins/plugin-ssr.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'; import { getTimeStat } from './util.js';
export async function viteBuild(opts: StaticBuildOptions) { export async function viteBuild(opts: StaticBuildOptions) {
@ -143,7 +143,7 @@ async function ssrBuild(
input: Set<string>, input: Set<string>,
container: AstroBuildPluginContainer container: AstroBuildPluginContainer
) { ) {
const { settings, viteConfig } = opts; const { allPages, settings, viteConfig } = opts;
const ssr = isServerLikeOutput(settings.config); const ssr = isServerLikeOutput(settings.config);
const out = ssr ? opts.buildConfig.server : getOutDirWithinCwd(settings.config.outDir); const out = ssr ? opts.buildConfig.server : getOutDirWithinCwd(settings.config.outDir);
@ -175,7 +175,7 @@ async function ssrBuild(
...viteConfig.build?.rollupOptions?.output, ...viteConfig.build?.rollupOptions?.output,
entryFileNames(chunkInfo) { entryFileNames(chunkInfo) {
if (chunkInfo.facadeModuleId?.startsWith(ASTRO_PAGE_RESOLVED_MODULE_ID)) { if (chunkInfo.facadeModuleId?.startsWith(ASTRO_PAGE_RESOLVED_MODULE_ID)) {
return makeAstroPageEntryPointFileName(chunkInfo.facadeModuleId); return makeAstroPageEntryPointFileName(chunkInfo.facadeModuleId, allPages);
} else if ( } else if (
// checks if the path of the module we have middleware, e.g. middleware.js / middleware/index.js // 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 && 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 * This function takes the virtual module name of any page entrypoint and
* to generate an `.mjs` file: * transforms it to generate a final `.mjs` output file.
* *
* Input: `@astro-page:src/pages/index@_@astro` * Input: `@astro-page:src/pages/index@_@astro`
*
* Output: `pages/index.astro.mjs` * 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:` * 1. We clean the `facadeModuleId` by removing the `@astro-page:` prefix and `@_@` suffix
* 2. We remove `src/` * 2. We find the matching route pattern in the manifest (or fallback to the cleaned module id)
* 3. We replace square brackets with underscore, for example `[slug]` * 3. We replace square brackets with underscore (`[slug]` => `_slug_`) and `...` with `` (`[...slug]` => `_---slug_`).
* 4. At last, we replace the extension pattern with a simple dot * 4. We append the `.mjs` extension, so the file will always be an ESM module
* 5. We append the `.mjs` string, so the file will always be a JS file
* *
* @param facadeModuleId * @param facadeModuleId string
* @param pages AllPagesData
*/ */
function makeAstroPageEntryPointFileName(facadeModuleId: string) { function makeAstroPageEntryPointFileName(facadeModuleId: string, pages: AllPagesData) {
return `${facadeModuleId const pageModuleId = facadeModuleId
.replace(ASTRO_PAGE_RESOLVED_MODULE_ID, '') .replace(ASTRO_PAGE_RESOLVED_MODULE_ID, '')
.replace('src/', '') .replace(ASTRO_PAGE_EXTENSION_POST_PATTERN, '.');
.replaceAll('[', '_') let name = pages[pageModuleId]?.route?.route ?? pageModuleId;
.replaceAll(']', '_') if (name.endsWith('/')) name += 'index';
// this must be last return `pages${name.replaceAll('[', '_').replaceAll(']', '_').replaceAll('...', '---')}.mjs`;
.replace(ASTRO_PAGE_EXTENSION_POST_PATTERN, '.')}.mjs`;
} }

View file

@ -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('<!DOCTYPE html>');
});
});
});

View file

@ -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',
});
},
},
},
],
});

View file

@ -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:*"
}
}

View file

@ -0,0 +1,11 @@
---
---
<html lang="en">
<head>
<title>Custom 404</title>
</head>
<body>
<h1>Home</h1>
</body>
</html>

View file

@ -0,0 +1,13 @@
---
const canonicalURL = new URL(Astro.url.pathname, Astro.site);
---
<html lang="en">
<head>
<title>Not Found - Custom 404</title>
</head>
<body>
<h1>Page not found</h1>
<p>{canonicalURL.pathname}</p>
</body>
</html>

View file

@ -0,0 +1,8 @@
{
"name": "@test/custom-404-pkg",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
}
}

View file

@ -1,4 +1,4 @@
lockfileVersion: '6.1' lockfileVersion: '6.0'
settings: settings:
autoInstallPeers: false autoInstallPeers: false
@ -2499,12 +2499,27 @@ importers:
specifier: workspace:* specifier: workspace:*
version: link:../../.. 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: packages/astro/test/fixtures/custom-404-md:
dependencies: dependencies:
astro: astro:
specifier: workspace:* specifier: workspace:*
version: link:../../.. version: link:../../..
packages/astro/test/fixtures/custom-404-pkg:
dependencies:
astro:
specifier: workspace:*
version: link:../../..
packages/astro/test/fixtures/custom-assets-name: packages/astro/test/fixtures/custom-assets-name:
dependencies: dependencies:
'@astrojs/node': '@astrojs/node':