-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
fix: prevent conflict between Netlify Identity and edge function rendering #12052
Changes from 7 commits
aed3284
5c3e74a
902ea7a
beacaae
a63c563
2d81ef4
75fce73
09e42c6
42bab13
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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`. | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at https://www.npmjs.com/package/@netlify/edge-functions?activeTab=code the |
||||||||||
* @typedef {{ | ||||||||||
* functions: Array< | ||||||||||
* | { | ||||||||||
* function: string; | ||||||||||
* path: string; | ||||||||||
* excludedPath?: string | string[]; | ||||||||||
* } | ||||||||||
* | { | ||||||||||
* function: string; | ||||||||||
* pattern: string; | ||||||||||
* excludedPattern?: string | string[]; | ||||||||||
* } | ||||||||||
* >; | ||||||||||
* version: 1; | ||||||||||
|
@@ -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()); | ||||||||||
|
||||||||||
|
@@ -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 = '/*'; | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wonder if we should add the base path to this: const path = `${builder.config.kit.paths.base}/*`; |
||||||||||
// 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) => { | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the previous logic also checked within There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could add kit/packages/kit/src/exports/vite/index.js Lines 848 to 851 in f451f6c
|
||||||||||
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`], | ||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI I'm working on this as well. I should have a follow-up PR soon.