Skip to content

Commit

Permalink
fix: prevent conflict between Netlify Identity and edge function rend…
Browse files Browse the repository at this point in the history
…ering (#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 <simon.holthausen@vercel.com>
Co-authored-by: Chew Tee Ming <chew.tee.ming@nindatech.com>
Co-authored-by: Tee Ming <chewteeming01@gmail.com>
  • Loading branch information
4 people authored Jan 21, 2025
1 parent 7c81ac9 commit 0d813dc
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 62 deletions.
5 changes: 5 additions & 0 deletions .changeset/witty-teachers-film.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/adapter-netlify': patch
---

fix: avoid unnecessary Netlify edge function invocations for static files, which resolves a conflict between Netlify Edge Functions and Netlify Identity
64 changes: 41 additions & 23 deletions packages/adapter-netlify/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,19 @@ import toml from '@iarna/toml';
*/

/**
* TODO(serhalp) Replace this custom type with an import from `@netlify/edge-functions`,
* once that type is fixed to include `excludedPath` and `function`.
* @typedef {{
* functions: Array<
* | {
* function: string;
* path: string;
* excludedPath?: string | string[];
* }
* | {
* function: string;
* pattern: string;
* excludedPattern?: string | string[];
* }
* >;
* version: 1;
Expand Down Expand Up @@ -122,23 +126,6 @@ async function generate_edge_functions({ builder }) {

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

// Don't match the static directory
const pattern = '^/.*$';

// Go doesn't support lookarounds, so we can't do this
// const pattern = appDir ? `^/(?!${escapeStringRegexp(appDir)}).*$` : '^/.*$';

/** @type {HandlerManifest} */
const edge_manifest = {
functions: [
{
function: 'render',
pattern
}
],
version: 1
};

builder.log.minor('Generating Edge Function...');
const relativePath = posix.relative(tmp, builder.getServerDirectory());

Expand All @@ -153,12 +140,43 @@ async function generate_edge_functions({ builder }) {
relativePath
});

writeFileSync(
`${tmp}/manifest.js`,
`export const manifest = ${manifest};\n\nexport const prerendered = new Set(${JSON.stringify(
builder.prerendered.paths
)});\n`
);
writeFileSync(`${tmp}/manifest.js`, `export const manifest = ${manifest};\n`);

/** @type {{ assets: Set<string> }} */
const { assets } = (await import(`${tmp}/manifest.js`)).manifest;

const path = '/*';
// We only need to specify paths without the trailing slash because
// Netlify will handle the optional trailing slash for us
const excludedPath = [
// Contains static files
`/${builder.getAppPath()}/*`,
...builder.prerendered.paths,
...Array.from(assets).flatMap((asset) => {
if (asset.endsWith('/index.html')) {
const dir = asset.replace(/\/index\.html$/, '');
return [
`${builder.config.kit.paths.base}/${asset}`,
`${builder.config.kit.paths.base}/${dir}`
];
}
return `${builder.config.kit.paths.base}/${asset}`;
}),
// Should not be served by SvelteKit at all
'/.netlify/*'
];

/** @type {HandlerManifest} */
const edge_manifest = {
functions: [
{
function: 'render',
path,
excludedPath
}
],
version: 1
};

await esbuild.build({
entryPoints: [`${tmp}/entry.js`],
Expand Down
1 change: 0 additions & 1 deletion packages/adapter-netlify/internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,4 @@ declare module 'MANIFEST' {
import { SSRManifest } from '@sveltejs/kit';

export const manifest: SSRManifest;
export const prerendered: Set<string>;
}
39 changes: 1 addition & 38 deletions packages/adapter-netlify/src/edge.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { Server } from '0SERVER';
import { manifest, prerendered } from 'MANIFEST';
import { manifest } from 'MANIFEST';

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

const initialized = server.init({
// @ts-ignore
Expand All @@ -15,12 +14,6 @@ const initialized = server.init({
* @returns { Promise<Response> }
*/
export default async function handler(request, context) {
if (is_static_file(request)) {
// Static files can skip the handler
// TODO can we serve _app/immutable files with an immutable cache header?
return;
}

await initialized;
return server.respond(request, {
platform: { context },
Expand All @@ -29,33 +22,3 @@ export default async function handler(request, context) {
}
});
}

/**
* @param {Request} request
*/
function is_static_file(request) {
const url = new URL(request.url);

// Assets in the app dir
if (url.pathname.startsWith(prefix)) {
return true;
}

// prerendered pages and index.html files
const pathname = url.pathname.replace(/\/$/, '');
let file = pathname.substring(1);

try {
file = decodeURIComponent(file);
} catch {
// ignore
}

return (
manifest.assets.has(file) ||
manifest.assets.has(file + '/index.html') ||
file in manifest._.server_assets ||
file + '/index.html' in manifest._.server_assets ||
prerendered.has(pathname || '/')
);
}

0 comments on commit 0d813dc

Please sign in to comment.