-
Notifications
You must be signed in to change notification settings - Fork 380
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
feat: fetch all edge function routes #5219
feat: fetch all edge function routes #5219
Conversation
📊 Benchmark resultsComparing with 93aface Package size: 247 MB⬇️ 0.00% decrease vs. 93aface
Legend
|
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.
Left a minor comment. Also, we should capture in an issue the passthrough improvements. Could you do that?
src/lib/edge-functions/registry.cjs
Outdated
...route, | ||
pattern: new RegExp(route.pattern), | ||
})) | ||
const postCacheRoutes = manifest.post_cache_routes.map((route) => ({ |
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.
I think you could merge the two routes arrays and have a single map?
const routes = [...manifest.routes, ...manifest.post_cache_routes].map((route => ({
...route,
pattern: new RegExp(route.pattern),
}))
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.
Better! Thanks, done!
🎉 Thanks for submitting a pull request! 🎉
Summary
https://github.com/netlify/pod-compute/issues/257
For us to review and ship your PR efficiently, please perform the following steps:
passes our tests.
A picture of a cute animal (not mandatory, but encouraged)