Skip to content

Commit 0d813dc

Browse files
serhalpdummdidummeltigerchino
authored
fix: prevent conflict between Netlify Identity and edge function rendering (#12052)
Previously Netlify Edge Functions didn't support a way to configure path matching that *excludes* paths. Now that it does, we can avoid running the "render" edge function on static assets. Currently, it's [just a no-op](https://github.com/sveltejs/kit/blob/80386437030c5c79913e859e3c32fd194613e1b6/packages/adapter-netlify/src/edge.js#L18-L22), but it's still nice to avoid the invocation. Fixes #5235. --------- Co-authored-by: Simon Holthausen <[email protected]> Co-authored-by: Chew Tee Ming <[email protected]> Co-authored-by: Tee Ming <[email protected]>
1 parent 7c81ac9 commit 0d813dc

File tree

4 files changed

+47
-62
lines changed

4 files changed

+47
-62
lines changed

.changeset/witty-teachers-film.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@sveltejs/adapter-netlify': patch
3+
---
4+
5+
fix: avoid unnecessary Netlify edge function invocations for static files, which resolves a conflict between Netlify Edge Functions and Netlify Identity

packages/adapter-netlify/index.js

Lines changed: 41 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,19 @@ import toml from '@iarna/toml';
1414
*/
1515

1616
/**
17+
* TODO(serhalp) Replace this custom type with an import from `@netlify/edge-functions`,
18+
* once that type is fixed to include `excludedPath` and `function`.
1719
* @typedef {{
1820
* functions: Array<
1921
* | {
2022
* function: string;
2123
* path: string;
24+
* excludedPath?: string | string[];
2225
* }
2326
* | {
2427
* function: string;
2528
* pattern: string;
29+
* excludedPattern?: string | string[];
2630
* }
2731
* >;
2832
* version: 1;
@@ -122,23 +126,6 @@ async function generate_edge_functions({ builder }) {
122126

123127
builder.mkdirp('.netlify/edge-functions');
124128

125-
// Don't match the static directory
126-
const pattern = '^/.*$';
127-
128-
// Go doesn't support lookarounds, so we can't do this
129-
// const pattern = appDir ? `^/(?!${escapeStringRegexp(appDir)}).*$` : '^/.*$';
130-
131-
/** @type {HandlerManifest} */
132-
const edge_manifest = {
133-
functions: [
134-
{
135-
function: 'render',
136-
pattern
137-
}
138-
],
139-
version: 1
140-
};
141-
142129
builder.log.minor('Generating Edge Function...');
143130
const relativePath = posix.relative(tmp, builder.getServerDirectory());
144131

@@ -153,12 +140,43 @@ async function generate_edge_functions({ builder }) {
153140
relativePath
154141
});
155142

156-
writeFileSync(
157-
`${tmp}/manifest.js`,
158-
`export const manifest = ${manifest};\n\nexport const prerendered = new Set(${JSON.stringify(
159-
builder.prerendered.paths
160-
)});\n`
161-
);
143+
writeFileSync(`${tmp}/manifest.js`, `export const manifest = ${manifest};\n`);
144+
145+
/** @type {{ assets: Set<string> }} */
146+
const { assets } = (await import(`${tmp}/manifest.js`)).manifest;
147+
148+
const path = '/*';
149+
// We only need to specify paths without the trailing slash because
150+
// Netlify will handle the optional trailing slash for us
151+
const excludedPath = [
152+
// Contains static files
153+
`/${builder.getAppPath()}/*`,
154+
...builder.prerendered.paths,
155+
...Array.from(assets).flatMap((asset) => {
156+
if (asset.endsWith('/index.html')) {
157+
const dir = asset.replace(/\/index\.html$/, '');
158+
return [
159+
`${builder.config.kit.paths.base}/${asset}`,
160+
`${builder.config.kit.paths.base}/${dir}`
161+
];
162+
}
163+
return `${builder.config.kit.paths.base}/${asset}`;
164+
}),
165+
// Should not be served by SvelteKit at all
166+
'/.netlify/*'
167+
];
168+
169+
/** @type {HandlerManifest} */
170+
const edge_manifest = {
171+
functions: [
172+
{
173+
function: 'render',
174+
path,
175+
excludedPath
176+
}
177+
],
178+
version: 1
179+
};
162180

163181
await esbuild.build({
164182
entryPoints: [`${tmp}/entry.js`],

packages/adapter-netlify/internal.d.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,4 @@ declare module 'MANIFEST' {
66
import { SSRManifest } from '@sveltejs/kit';
77

88
export const manifest: SSRManifest;
9-
export const prerendered: Set<string>;
109
}

packages/adapter-netlify/src/edge.js

Lines changed: 1 addition & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
import { Server } from '0SERVER';
2-
import { manifest, prerendered } from 'MANIFEST';
2+
import { manifest } from 'MANIFEST';
33

44
const server = new Server(manifest);
5-
const prefix = `/${manifest.appPath}/`;
65

76
const initialized = server.init({
87
// @ts-ignore
@@ -15,12 +14,6 @@ const initialized = server.init({
1514
* @returns { Promise<Response> }
1615
*/
1716
export default async function handler(request, context) {
18-
if (is_static_file(request)) {
19-
// Static files can skip the handler
20-
// TODO can we serve _app/immutable files with an immutable cache header?
21-
return;
22-
}
23-
2417
await initialized;
2518
return server.respond(request, {
2619
platform: { context },
@@ -29,33 +22,3 @@ export default async function handler(request, context) {
2922
}
3023
});
3124
}
32-
33-
/**
34-
* @param {Request} request
35-
*/
36-
function is_static_file(request) {
37-
const url = new URL(request.url);
38-
39-
// Assets in the app dir
40-
if (url.pathname.startsWith(prefix)) {
41-
return true;
42-
}
43-
44-
// prerendered pages and index.html files
45-
const pathname = url.pathname.replace(/\/$/, '');
46-
let file = pathname.substring(1);
47-
48-
try {
49-
file = decodeURIComponent(file);
50-
} catch {
51-
// ignore
52-
}
53-
54-
return (
55-
manifest.assets.has(file) ||
56-
manifest.assets.has(file + '/index.html') ||
57-
file in manifest._.server_assets ||
58-
file + '/index.html' in manifest._.server_assets ||
59-
prerendered.has(pathname || '/')
60-
);
61-
}

0 commit comments

Comments
 (0)