-
Notifications
You must be signed in to change notification settings - Fork 32
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
Enhance custom configuration support #66
base: main
Are you sure you want to change the base?
Changes from all commits
940f268
86f658a
fb497fc
5bc0d14
6634aa7
e5ba666
b41a468
db15751
9c3fd8e
4209506
73267d4
442ea5e
072b5dc
93d6908
db12d11
06ae5bf
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 |
---|---|---|
|
@@ -8,29 +8,23 @@ import esbuild from 'esbuild'; | |
*/ | ||
|
||
const ssrFunctionRoute = '/api/__render'; | ||
|
||
/** | ||
* Validate the static web app configuration does not override the minimum config for the adapter to work correctly. | ||
* @param config {import('./types/swa').CustomStaticWebAppConfig} | ||
* */ | ||
function validateCustomConfig(config) { | ||
if (config) { | ||
if ('navigationFallback' in config) { | ||
throw new Error('customStaticWebAppConfig cannot override navigationFallback.'); | ||
} | ||
if (config.routes && config.routes.find((route) => route.route === '*')) { | ||
throw new Error(`customStaticWebAppConfig cannot override '*' route.`); | ||
} | ||
} | ||
} | ||
// These are the methods that don't have to be SSR if they were pre-rendered. | ||
const staticMethods = ['GET', 'HEAD', 'OPTIONS']; | ||
// These are the methods that must always be SSR. | ||
const ssrMethods = ['CONNECT', 'DELETE', 'PATCH', 'POST', 'PUT', 'TRACE']; | ||
// This is the phrase that will be replaced with ssrFunctionRoute in custom configurations that use it. | ||
const ssrTrigger = 'ssr'; | ||
// Default version of node to target | ||
const nodeVersion = 'node:16'; | ||
|
||
/** @type {import('.').default} */ | ||
export default function ({ | ||
debug = false, | ||
customStaticWebAppConfig = {}, | ||
esbuildOptions = {} | ||
} = {}) { | ||
return { | ||
/** @type {import('@sveltejs/kit').Adapter} */ | ||
const adapter = { | ||
name: 'adapter-azure-swa', | ||
|
||
async adapt(builder) { | ||
|
@@ -40,8 +34,6 @@ export default function ({ | |
); | ||
} | ||
|
||
const swaConfig = generateConfig(customStaticWebAppConfig, builder.config.kit.appDir); | ||
|
||
const tmp = builder.getBuildDirectory('azure-tmp'); | ||
const publish = 'build'; | ||
const staticDir = join(publish, 'static'); | ||
|
@@ -77,13 +69,25 @@ export default function ({ | |
|
||
builder.copy(join(files, 'api'), apiDir); | ||
|
||
const apiRuntime = (customStaticWebAppConfig.platform || {}).apiRuntime || nodeVersion; | ||
const apiRuntimeParts = apiRuntime.match(/^node:(\d+)$/); | ||
if (apiRuntimeParts === null) { | ||
throw new Error( | ||
`The configuration key platform.apiRuntime, if included, must be a supported Node version such as 'node:16'. It is currently '${apiRuntime}'.` | ||
); | ||
} else if (parseInt(apiRuntimeParts[1]) < 16) { | ||
throw new Error( | ||
`The minimum node version supported by SvelteKit is 16, please change configuration key platform.runTime from '${apiRuntime}' to a supported version like 'node:16' or remove it entirely.` | ||
); | ||
} | ||
|
||
/** @type {BuildOptions} */ | ||
const default_options = { | ||
entryPoints: [entry], | ||
outfile: join(apiDir, 'index.js'), | ||
bundle: true, | ||
platform: 'node', | ||
target: 'node16', | ||
target: `node${apiRuntimeParts[1]}`, | ||
sourcemap: 'linked', | ||
external: esbuildOptions.external | ||
}; | ||
|
@@ -99,59 +103,190 @@ export default function ({ | |
// If the root was not pre-rendered, add a placeholder index.html | ||
// Route all requests for the index to the SSR function | ||
writeFileSync(`${staticDir}/index.html`, ''); | ||
swaConfig.routes.push( | ||
{ | ||
route: '/index.html', | ||
rewrite: ssrFunctionRoute | ||
}, | ||
{ | ||
route: '/', | ||
rewrite: ssrFunctionRoute | ||
} | ||
); | ||
} | ||
|
||
const swaConfig = generateConfig(builder, customStaticWebAppConfig); | ||
|
||
writeFileSync(`${publish}/staticwebapp.config.json`, JSON.stringify(swaConfig)); | ||
} | ||
}; | ||
return adapter; | ||
} | ||
|
||
/** | ||
* @param {import('./types/swa').CustomStaticWebAppConfig} customStaticWebAppConfig | ||
* @param {string} appDir | ||
* @param {import('@sveltejs/kit').Builder} builder A reference to the SvelteKit builder | ||
* @param {import('./types/swa').StaticWebAppConfig} customStaticWebAppConfig Custom configuration | ||
* @returns {import('./types/swa').StaticWebAppConfig} | ||
*/ | ||
export function generateConfig(customStaticWebAppConfig, appDir) { | ||
validateCustomConfig(customStaticWebAppConfig); | ||
|
||
if (!customStaticWebAppConfig.routes) { | ||
customStaticWebAppConfig.routes = []; | ||
} | ||
|
||
export function generateConfig(builder, customStaticWebAppConfig = {}) { | ||
builder.log.minor(`Generating static web app config...`); | ||
/** @type {import('./types/swa').StaticWebAppConfig} */ | ||
const swaConfig = { | ||
...customStaticWebAppConfig, | ||
routes: [ | ||
...customStaticWebAppConfig.routes, | ||
{ | ||
route: '*', | ||
methods: ['POST', 'PUT', 'DELETE'], | ||
rewrite: ssrFunctionRoute | ||
}, | ||
{ | ||
route: `/${appDir}/immutable/*`, | ||
headers: { | ||
'cache-control': 'public, immutable, max-age=31536000' | ||
} | ||
} | ||
], | ||
navigationFallback: { | ||
rewrite: ssrFunctionRoute | ||
rewrite: ssrFunctionRoute, | ||
...customStaticWebAppConfig.navigationFallback | ||
}, | ||
platform: { | ||
apiRuntime: 'node:16' | ||
} | ||
apiRuntime: nodeVersion, | ||
...customStaticWebAppConfig.platform | ||
}, | ||
routes: [] | ||
}; | ||
|
||
if (swaConfig.navigationFallback.rewrite === ssrTrigger) { | ||
swaConfig.navigationFallback.rewrite = ssrFunctionRoute; | ||
} | ||
Comment on lines
+137
to
+139
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 don't think it makes sense to allow customization of this. This will break the adapter completely. Is there a use case I'm missing? 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. There are a few use cases when it would not break the site and would be preferable because it prevents usage charges from bots all over the world scanning your site for vulnerabilities at paths that don't exist.
And, of course, if the configuration key is not set it cannot break the adapter. It just gives more options for projects that use the adapter. 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. For 1, the user of the adapter shouldn't need to do anything for that - you configure prerendered pages in SvelteKit and Azure SWA will automatically route to those prerendered pages without hitting the SSR function. For 2, I'd want to see an example of what the use case is there. 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 want to avoid exposing an option unless there is a specific use-case to solve for. More options/features means more things to maintain and consider when making other updates to the adapter. 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 specific use-case is to prevent 404 errors from racking up Azure function usage charges on routes that are never supposed to be SSR. Yes, you can configure prerendered pages in SvelteKit, but 404 errors will be rendered by an Azure function with this adapter as it is currently built.
I agree generally, but this code only gets the adapter out of the way of features provided by Azure. It allows the custom configuration to work the way Microsoft's documentation describes, and it has no impact on people who don't use it. |
||
|
||
/** @type {Record<string,import('./types/swa').HttpMethod[]>} */ | ||
let handledRoutes = { | ||
'*': [], | ||
'/': [], | ||
'/index.html': [] | ||
}; | ||
/** @type {import('./types/swa').Route} */ | ||
let wildcardRoute = { | ||
route: '*' | ||
}; | ||
|
||
for (const route of customStaticWebAppConfig.routes || []) { | ||
if (route.route === undefined || !route.route.length) { | ||
throw new Error( | ||
'A route pattern is required for each route. https://learn.microsoft.com/en-us/azure/static-web-apps/configuration#routes' | ||
); | ||
} | ||
route.methods = route.methods || [...staticMethods, ...ssrMethods]; | ||
if (handledRoutes[route.route] && handledRoutes[route.route].some((i) => methods.includes(i))) { | ||
throw new Error( | ||
'There is a route that conflicts with another route. https://learn.microsoft.com/en-us/azure/static-web-apps/configuration#routes' | ||
); | ||
} | ||
handledRoutes[route.route] = [...(handledRoutes[route.route] || []), ...route.methods]; | ||
if (route.rewrite === ssrTrigger) { | ||
route.rewrite = ssrFunctionRoute; | ||
} | ||
if (['/*', '*'].includes(route.route)) { | ||
route.route = '*'; | ||
Comment on lines
+168
to
+169
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. Is 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 tested that by setting both routes, but SWA rejected the duplicate. |
||
wildcardRoute = route; | ||
} | ||
if (route.methods.length === staticMethods.length + ssrMethods.length) { | ||
// Either method isn't specified in this route, or all methods are. | ||
if (['/index.html', '/'].includes(route.route) && !builder.prerendered.paths.includes('/')) { | ||
// The root route must be fully SSR because it was not rendered. No need to split the route. | ||
swaConfig.routes.push({ | ||
rewrite: route.redirect ? route.rewrite : ssrFunctionRoute, | ||
...route | ||
}); | ||
} else { | ||
// This route catches all methods, but we don't want to force SSR for all methods, so will split the rule. | ||
swaConfig.routes.push({ | ||
...route, | ||
methods: staticMethods | ||
}); | ||
swaConfig.routes.push({ | ||
rewrite: route.redirect ? route.rewrite : ssrFunctionRoute, | ||
...route, | ||
methods: ssrMethods | ||
}); | ||
} | ||
} else if (route.methods.some((r) => ssrMethods.includes(r))) { | ||
const routeSSRMethods = methods.filter((m) => ssrMethods.includes(m)); | ||
if (routeSSRMethods.length === methods.length) { | ||
// This route is only for SSR methods, so we'll rewrite the single rule. | ||
swaConfig.routes.push({ | ||
rewrite: route.redirect ? route.rewrite : ssrFunctionRoute, | ||
...route | ||
}); | ||
} else { | ||
if ( | ||
['/index.html', '/'].includes(route.route) && | ||
!builder.prerendered.paths.includes('/') | ||
) { | ||
// This special route must be SSR because it was not pre-rendered. | ||
swaConfig.routes.push({ | ||
rewrite: route.redirect ? route.rewrite : ssrFunctionRoute, | ||
...route | ||
}); | ||
} else { | ||
// This route is for some methods that must be SSR, but not all. We'll split it. | ||
swaConfig.routes.push({ | ||
rewrite: route.redirect ? route.rewrite : ssrFunctionRoute, | ||
...route, | ||
methods: routeSSRMethods | ||
}); | ||
swaConfig.routes.push({ | ||
...route, | ||
methods: methods.filter((m) => staticMethods.includes(m)) | ||
}); | ||
} | ||
} | ||
} else { | ||
if (['/index.html', '/'].includes(route.route) && !builder.prerendered.paths.includes('/')) { | ||
// This special route must be SSR because it was not pre-rendered. | ||
swaConfig.routes.push({ | ||
rewrite: route.redirect ? route.rewrite : ssrFunctionRoute, | ||
...route | ||
}); | ||
} else { | ||
// None of the methods in this route must be SSR, so accept it as-is. | ||
swaConfig.routes.push({ ...route }); | ||
} | ||
} | ||
} | ||
|
||
// Make sure the wildcard is there for each SSR method. | ||
const missingWildcardMethods = ssrMethods.filter( | ||
(i) => !(wildcardRoute.methods || []).includes(i) | ||
); | ||
if (missingWildcardMethods.length > 0) { | ||
handledRoutes['*'] = missingWildcardMethods; | ||
swaConfig.routes.push({ | ||
rewrite: ssrFunctionRoute, | ||
...wildcardRoute, | ||
methods: missingWildcardMethods | ||
}); | ||
} | ||
|
||
// Make sure the fallback rewrite matches the custom config or wildcard route, if present. | ||
if ((customStaticWebAppConfig.navigationFallback || []).hasOwnProperty('rewrite')) { | ||
swaConfig.navigationFallback.rewrite = customStaticWebAppConfig.navigationFallback.rewrite; | ||
} else if (wildcardRoute.hasOwnProperty('rewrite')) { | ||
swaConfig.navigationFallback.rewrite = wildcardRoute.rewrite; | ||
} | ||
|
||
handledRoutes[`/${builder.config.kit.appDir}/immutable/*`] = [...staticMethods, ...ssrMethods]; | ||
swaConfig.routes.push({ | ||
...wildcardRoute, | ||
route: `/${builder.config.kit.appDir}/immutable/*`, | ||
headers: { | ||
'cache-control': 'public, immutable, max-age=31536000' | ||
}, | ||
methods: undefined | ||
}); | ||
if (!builder.prerendered.paths.includes('/')) { | ||
if (!staticMethods.every((i) => handledRoutes['/index.html'].includes(i))) { | ||
swaConfig.routes.push({ | ||
rewrite: wildcardRoute.redirect ? wildcardRoute.rewrite : ssrFunctionRoute, | ||
...wildcardRoute, | ||
route: '/index.html', | ||
methods: undefined | ||
}); | ||
} | ||
if (!staticMethods.every((i) => handledRoutes['/'].includes(i))) { | ||
swaConfig.routes.push({ | ||
rewrite: wildcardRoute.redirect ? wildcardRoute.rewrite : ssrFunctionRoute, | ||
...wildcardRoute, | ||
route: '/', | ||
methods: undefined | ||
}); | ||
} | ||
} | ||
|
||
if (swaConfig.navigationFallback.rewrite !== ssrFunctionRoute) { | ||
builder.log.warn( | ||
`Custom configuration has navigationFallback.rewrite set to a value other than '${ssrTrigger}'. SSR will fail unless a route specifies it.` | ||
); | ||
} | ||
|
||
return swaConfig; | ||
} |
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.
Unless there's a good reason to allow for customization of
navigationFallback.rewrite
(see other comment), these lines should be swappedThere 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.
I put it in this order specifically so that custom configuration can override it, if the key is present. I am strongly in favor of being clear about the behavior in the documentation and letting the developer decide what's best for their use-case. It doesn't affect my current use-case and this is your project, so I will commit your suggestion if you feel strongly about it, but I would really like to have the opportunity to make improvements to the documentation until you're satisfied with the results.