Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates the dev server to handle multiple routes matching the same URL #4087

Merged
merged 10 commits into from
Jul 29, 2022
Next Next commit
updates the dev server to handle multiple routes matching the same URL
  • Loading branch information
Tony Sullivan committed Jul 28, 2022
commit 0c1bb3fe4b85c4e47d1ec0a0052cecae0dc9a32b
2 changes: 1 addition & 1 deletion packages/astro/src/core/routing/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export { createRouteManifest } from './manifest/create.js';
export { deserializeRouteData, serializeRouteData } from './manifest/serialization.js';
export { matchRoute } from './match.js';
export { matchRoute, matchAllRoutes } from './match.js';
export { getParams } from './params.js';
export { validateGetStaticPathsModule, validateGetStaticPathsResult } from './validation.js';
5 changes: 5 additions & 0 deletions packages/astro/src/core/routing/match.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,8 @@ import type { ManifestData, RouteData } from '../../@types/astro';
export function matchRoute(pathname: string, manifest: ManifestData): RouteData | undefined {
return manifest.routes.find((route) => route.pattern.test(pathname));
}

/** Finds all matching routes from pathname */
export function matchAllRoutes(pathname: string, manifest: ManifestData): RouteData[] {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

used by the dev server to find all potential routes for the requested URL

return manifest.routes.filter((route) => route.pattern.test(pathname));
}
110 changes: 62 additions & 48 deletions packages/astro/src/vite-plugin-astro-server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { getParamsAndProps, GetParamsAndPropsError } from '../core/render/core.j
import { preload, ssr } from '../core/render/dev/index.js';
import { RouteCache } from '../core/render/route-cache.js';
import { createRequest } from '../core/request.js';
import { createRouteManifest, matchRoute } from '../core/routing/index.js';
import { createRouteManifest, matchAllRoutes, matchRoute } from '../core/routing/index.js';
import { createSafeError, resolvePages } from '../core/util.js';
import notFoundTemplate, { subpathNotUsedTemplate } from '../template/4xx.js';

Expand Down Expand Up @@ -248,26 +248,72 @@ async function handleRequest(
clientAddress: buildingToSSR ? req.socket.remoteAddress : undefined,
});

async function matchRoute() {
const matches = matchAllRoutes(pathname, manifest);

for await (const maybeRoute of matches) {
const filePath = new URL(`./${maybeRoute.component}`, config.root);
const preloadedComponent = await preload({ astroConfig: config, filePath, viteServer });
const [, mod] = preloadedComponent;
// attempt to get static paths
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this result in getStaticPaths being called more than once?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's still only called once. The getStaticPaths() result is always cached even if the params don't match

Just noticed we have a test to make sure it's only called once but that test only covers build. I just pushed up a test that checks that for dev as well 👍

// if this fails, we have a bad URL match!
const paramsAndPropsRes = await getParamsAndProps({
mod,
route: maybeRoute,
routeCache,
pathname: pathname,
logging,
ssr: config.output === 'server',
});

if (paramsAndPropsRes !== GetParamsAndPropsError.NoMatchingStaticPath) {
return {
route: maybeRoute,
filePath,
preloadedComponent,
mod,
}
}
}

if (matches.length) {
warn(
logging,
'getStaticPaths',
`Route pattern matched, but no matching static path found. (${pathname})`
);
}

log404(logging, pathname);
const custom404 = getCustom404Route(config, manifest);

if (custom404) {
const filePath = new URL(`./${custom404.component}`, config.root);
const preloadedComponent = await preload({ astroConfig: config, filePath, viteServer });
const [, mod] = preloadedComponent;

return {
route: custom404,
filePath,
preloadedComponent,
mod
};
}

return undefined;
}

let filePath: URL | undefined;
try {
// Attempt to match the URL to a valid page route.
// If that fails, switch the response to a 404 response.
let route = matchRoute(pathname, manifest);
const statusCode = route ? 200 : 404;
const matchedRoute = await matchRoute();

if (!route) {
log404(logging, pathname);
const custom404 = getCustom404Route(config, manifest);
if (custom404) {
route = custom404;
} else {
return handle404Response(origin, config, req, res);
}
if (!matchedRoute) {
return handle404Response(origin, config, req, res);
}

filePath = new URL(`./${route.component}`, config.root);
const preloadedComponent = await preload({ astroConfig: config, filePath, viteServer });
const [, mod] = preloadedComponent;
const { route, preloadedComponent, mod } = matchedRoute;
filePath = matchedRoute.filePath;

// attempt to get static paths
// if this fails, we have a bad URL match!
const paramsAndPropsRes = await getParamsAndProps({
Expand All @@ -278,38 +324,6 @@ async function handleRequest(
logging,
ssr: config.output === 'server',
});
if (paramsAndPropsRes === GetParamsAndPropsError.NoMatchingStaticPath) {
warn(
logging,
'getStaticPaths',
`Route pattern matched, but no matching static path found. (${pathname})`
);
log404(logging, pathname);
const routeCustom404 = getCustom404Route(config, manifest);
if (routeCustom404) {
const filePathCustom404 = new URL(`./${routeCustom404.component}`, config.root);
const preloadedCompCustom404 = await preload({
astroConfig: config,
filePath: filePathCustom404,
viteServer,
});
const result = await ssr(preloadedCompCustom404, {
astroConfig: config,
filePath: filePathCustom404,
logging,
mode: 'development',
origin,
pathname: pathname,
request,
route: routeCustom404,
routeCache,
viteServer,
});
return await writeSSRResult(result, res);
} else {
return handle404Response(origin, config, req, res);
}
}

const options: SSROptions = {
astroConfig: config,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
export async function getStaticPaths() {
return [
{
params: { page: "page-1" }
},
{
params: { page: "page-2" }
}
]
}
---

<html lang="en">

<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width" />
<title>Routing</title>
</head>

<body>
<h1>[page].astro</h1>
<p>{Astro.params.page}</p>
</body>

</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
export async function getStaticPaths() {
return [
{
params: { slug: "slug-1" },
},
{
params: { slug: "slug-2" },
}
]
}
---

<html lang="en">

<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width" />
<title>Routing</title>
</head>

<body>
<h1>[slug].astro</h1>
<p>{Astro.params.slug}</p>
</body>

</html>
64 changes: 64 additions & 0 deletions packages/astro/test/routing-priority.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,38 @@ describe('Routing priority', () => {
expect($('h1').text()).to.equal('index.astro');
});

it('matches /slug-1 to [slug].astro', async () => {
const html = await fixture.readFile('/slug-1/index.html');
const $ = cheerioLoad(html);

expect($('h1').text()).to.equal('[slug].astro');
expect($('p').text()).to.equal('slug-1');
});

it('matches /slug-2 to [slug].astro', async () => {
const html = await fixture.readFile('/slug-2/index.html');
const $ = cheerioLoad(html);

expect($('h1').text()).to.equal('[slug].astro');
expect($('p').text()).to.equal('slug-2');
});

it('matches /page-1 to [page].astro', async () => {
const html = await fixture.readFile('/page-1/index.html');
const $ = cheerioLoad(html);

expect($('h1').text()).to.equal('[page].astro');
expect($('p').text()).to.equal('page-1');
});

it('matches /page-2 to [page].astro', async () => {
const html = await fixture.readFile('/page-2/index.html');
const $ = cheerioLoad(html);

expect($('h1').text()).to.equal('[page].astro');
expect($('p').text()).to.equal('page-2');
});

it('matches /posts/post-1 to posts/[pid].astro', async () => {
const html = await fixture.readFile('/posts/post-1/index.html');
const $ = cheerioLoad(html);
Expand Down Expand Up @@ -149,6 +181,38 @@ describe('Routing priority', () => {
expect($('h1').text()).to.equal('index.astro');
});

it('matches /slug-1 to /[slug].astro', async () => {
const html = await fixture.fetch('/slug-1').then((res) => res.text());
const $ = cheerioLoad(html);

expect($('h1').text()).to.equal('[slug].astro');
expect($('p').text()).to.equal('slug-1');
});

it('matches /slug-2 to /[slug].astro', async () => {
const html = await fixture.fetch('/slug-2').then((res) => res.text());
const $ = cheerioLoad(html);

expect($('h1').text()).to.equal('[slug].astro');
expect($('p').text()).to.equal('slug-2');
});

it('matches /page-1 to /[page].astro', async () => {
const html = await fixture.fetch('/page-1').then((res) => res.text());
const $ = cheerioLoad(html);

expect($('h1').text()).to.equal('[page].astro');
expect($('p').text()).to.equal('page-1');
});

it('matches /page-2 to /[page].astro', async () => {
const html = await fixture.fetch('/page-2').then((res) => res.text());
const $ = cheerioLoad(html);

expect($('h1').text()).to.equal('[page].astro');
expect($('p').text()).to.equal('page-2');
});

it('matches /posts/post-1 to /posts/[pid].astro', async () => {
const html = await fixture.fetch('/posts/post-1').then((res) => res.text());
const $ = cheerioLoad(html);
Expand Down