Improve error message for endpoint getStaticPaths with undefined value (#6353)

Co-authored-by: bluwy <bjornlu.dev@gmail.com>
Co-authored-by: wuls <linsheng.wu@beantechs.com>
This commit is contained in:
wulinsheng123 2023-03-28 23:13:46 +08:00 committed by GitHub
parent 328c671790
commit 4bf87c64ff
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 135 additions and 1 deletions

View file

@ -0,0 +1,5 @@
---
'astro': patch
---
Throw better error when a dynamic endpoint without additional extensions is prerendered with `undefined` params.

View file

@ -530,6 +530,27 @@ See https://docs.astro.build/en/guides/server-side-rendering/ for more informati
)} are supported for optimization.`,
hint: "If you do not need optimization, using an `img` tag directly instead of the `Image` component might be what you're looking for.",
},
/**
* @docs
* @see
* - [`getStaticPaths()`](https://docs.astro.build/en/reference/api-reference/#getstaticpaths)
* - [`params`](https://docs.astro.build/en/reference/api-reference/#params)
* @description
* The endpoint is prerendered with an `undefined` param so the generated path will collide with another route.
*
* If you cannot prevent passing `undefined`, then an additional extension can be added to the endpoint file name to generate the file with a different name. For example, renaming `pages/api/[slug].ts` to `pages/api/[slug].json.ts`.
*/
PrerenderDynamicEndpointPathCollide: {
title: 'Prerendered dynamic endpoint has path collision.',
code: 3026,
message: (pathname: string) =>
`Could not render \`${pathname}\` with an \`undefined\` param as the generated path will collide during prerendering. ` +
`Prevent passing \`undefined\` as \`params\` for the endpoint's \`getStaticPaths()\` function, ` +
`or add an additional extension to the endpoint's filename.`,
hint: (filename: string) =>
`Rename \`${filename}\` to \`${filename.replace(/\.(js|ts)/, (m) => `.json` + m)}\``,
},
// No headings here, that way Vite errors are merged with Astro ones in the docs, which makes more sense to users.
// Vite Errors - 4xxx
/**

View file

@ -35,6 +35,27 @@ export async function getParamsAndProps(
const paramsMatch = route.pattern.exec(pathname);
if (paramsMatch) {
params = getParams(route.params)(paramsMatch);
// If we have an endpoint at `src/pages/api/[slug].ts` that's prerendered, and the `slug`
// is `undefined`, throw an error as we can't generate the `/api` file and `/api` directory
// at the same time. Using something like `[slug].json.ts` instead will work.
if (route.type === 'endpoint' && mod.getStaticPaths) {
const lastSegment = route.segments[route.segments.length - 1];
const paramValues = Object.values(params);
const lastParam = paramValues[paramValues.length - 1];
// Check last segment is solely `[slug]` or `[...slug]` case (dynamic). Make sure it's not
// `foo[slug].js` by checking segment length === 1. Also check here if that param is undefined.
if (lastSegment.length === 1 && lastSegment[0].dynamic && lastParam === undefined) {
throw new AstroError({
...AstroErrorData.PrerenderDynamicEndpointPathCollide,
message: AstroErrorData.PrerenderDynamicEndpointPathCollide.message(route.route),
hint: AstroErrorData.PrerenderDynamicEndpointPathCollide.hint(route.component),
location: {
file: route.component,
},
});
}
}
}
}
let routeCacheEntry = routeCache.get(route);

View file

@ -0,0 +1,52 @@
import { expect } from 'chai';
import { load as cheerioLoad } from 'cheerio';
import { loadFixture } from './test-utils.js';
describe('Dynamic endpoint collision', () => {
describe('build', () => {
let fixture;
let errorMsg;
before(async () => {
fixture = await loadFixture({
root: './fixtures/dynamic-endpoint-collision/',
});
try {
await fixture.build();
} catch (error) {
errorMsg = error;
}
});
it('throw error when dynamic endpoint has path collision', async () => {
expect(errorMsg.errorCode).to.eq(3026);
});
});
describe('dev', () => {
let fixture;
let devServer;
before(async () => {
fixture = await loadFixture({
root: './fixtures/dynamic-endpoint-collision/',
});
devServer = await fixture.startDevServer();
});
after(async () => {
await devServer.stop();
});
it('throw error when dynamic endpoint has path collision', async () => {
const html = await fixture.fetch('/api/catch').then((res) => res.text());
const $ = cheerioLoad(html);
expect($('title').text()).to.equal('PrerenderDynamicEndpointPathCollide');
});
it("don't throw error when dynamic endpoint doesn't load the colliding path", async () => {
const res = await fixture.fetch('/api/catch/one').then((r) => r.text());
expect(res).to.equal('{"slug":"one"}');
});
});
});

View file

@ -0,0 +1,7 @@
import { defineConfig } from 'astro/config';
// https://astro.build/config
export default defineConfig({
});

View file

@ -0,0 +1,8 @@
{
"name": "@test/dynamic-endpoint-collision",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
}
}

View file

@ -0,0 +1,15 @@
import type { APIRoute } from "astro";
const slugs = ["one", undefined];
export const get: APIRoute = ({ params }) => {
return {
body: JSON.stringify({
slug: params.slug || "index",
}),
};
};
export function getStaticPaths() {
return slugs.map((u) => ({ params: { slug: u } }));
}

View file

@ -5,7 +5,6 @@ export interface Props {
href: string,
}
const {href, title, body} = Astro.props;
debugger;
---
<li class="link-card">
<a href={href}>

View file

@ -1971,6 +1971,12 @@ importers:
dependencies:
astro: link:../../..
packages/astro/test/fixtures/dynamic-endpoint-collision:
specifiers:
astro: workspace:*
dependencies:
astro: link:../../..
packages/astro/test/fixtures/dynamic-route-build-file:
specifiers:
astro: workspace:*