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.

Sign up for GitHub

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

feat: allow dynamic route segments in isr.exclude array #10513

Merged
merged 5 commits into from
Mar 28, 2024
+69 −3
Merged

feat: allow dynamic route segments in isr.exclude array #10513

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
431c9af
Allow dynamic route segments in isr.exclude array
tk04 Mar 21, 2024
dd423a0
copy over eslint-disable as well
lilnasy Mar 27, 2024
bc815b9
add test
lilnasy Mar 27, 2024
d7e2915
update test: slashes dont need to be escaped
lilnasy Mar 27, 2024
4ad07a0
update changeset
lilnasy Mar 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/angry-lamps-cheer.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@astrojs/vercel": minor
---

Allow dynamic route segments in isr.exclude array
37 changes: 35 additions & 2 deletions packages/integrations/vercel/src/lib/redirects.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import nodePath from 'node:path';
import { appendForwardSlash } from '@astrojs/internal-helpers/path';
import { appendForwardSlash, removeLeadingForwardSlash } from '@astrojs/internal-helpers/path';
import type { AstroConfig, RouteData, RoutePart } from 'astro';

const pathJoin = nodePath.posix.join;
Expand All @@ -14,6 +14,33 @@ interface VercelRoute {
continue?: boolean;
}

// Copied from astro/packages/astro/src/core/routing/manifest/create.ts
// Disable eslint as we're not sure how to improve this regex yet
// eslint-disable-next-line regexp/no-super-linear-backtracking
const ROUTE_DYNAMIC_SPLIT = /\[(.+?\(.+?\)|.+?)\]/;
const ROUTE_SPREAD = /^\.{3}.+$/;
function getParts(part: string, file: string) {
Copy link
Member

@ematipico ematipico Mar 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I've seen this very function used in the Cloudflare adapter too. Maybe we should consider exposing it (not in this PR)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right, it would make sense to expose it in the core package. Maybe also the code for parsing the segments:

const segments = removeLeadingForwardSlash(content)
  .split(nodePath.posix.sep)
  .filter(Boolean)
  .map((s: string) => {
	  return getParts(s, content);
  });

I've seen it used many times in the core package and the Cloudflare adapter.

const result: RoutePart[] = [];
part.split(ROUTE_DYNAMIC_SPLIT).map((str, i) => {
if (!str) return;
const dynamic = i % 2 === 1;

const [, content] = dynamic ? /([^(]+)$/.exec(str) || [null, null] : [null, str];

if (!content || (dynamic && !/^(?:\.\.\.)?[\w$]+$/.test(content))) {
throw new Error(`Invalid route ${file} — parameter name must match /^[a-zA-Z0-9_$]+$/`);
}

result.push({
content,
dynamic,
spread: dynamic && ROUTE_SPREAD.test(content),
});
});

return result;
}

// Copied from /home/juanm04/dev/misc/astro/packages/astro/src/core/routing/manifest/create.ts
// 2022-04-26
function getMatchPattern(segments: RoutePart[][]) {
Expand Down Expand Up @@ -77,7 +104,13 @@ function getRedirectStatus(route: RouteData): number {
}

export function escapeRegex(content: string) {
return `^${getMatchPattern([[{ content, dynamic: false, spread: false }]])}$`;
const segments = removeLeadingForwardSlash(content)
.split(nodePath.posix.sep)
.filter(Boolean)
.map((s: string) => {
return getParts(s, content);
});
return `^/${getMatchPattern(segments)}$`;
}

export function getRedirects(routes: RouteData[], config: AstroConfig): VercelRoute[] {
Expand Down
2 changes: 1 addition & 1 deletion packages/integrations/vercel/test/fixtures/isr/astro.config.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export default defineConfig({
isr: {
bypassToken: "1c9e601d-9943-4e7c-9575-005556d774a8",
expiration: 120,
exclude: ["/two"]
exclude: ["/two", "/excluded/[dynamic]"]
}
})
});
8 changes: 8 additions & 0 deletions packages/integrations/vercel/test/fixtures/isr/src/pages/excluded/[dynamic].astro
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html>
<head>
<title>Dynamic</title>
</head>
<body>
<h1>Dynamic</h1>
</body>
</html>
8 changes: 8 additions & 0 deletions packages/integrations/vercel/test/isr.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,18 @@ describe('ISR', () => {
src: '^/two$',
dest: '_render',
},
{
src: '^/excluded/([^/]+?)$',
dest: '_render'
},
{
src: '^\\/_image$',
dest: '_render',
},
{
src: '^\\/excluded\\/([^/]+?)\\/?$',
dest: '/_isr?x_astro_path=$0',
},
{
src: '^\\/one\\/?$',
dest: '/_isr?x_astro_path=$0',
Expand Down
Loading