feat: match edge functions by header#7439
Conversation
📊 Benchmark resultsComparing with 69134d2
|
serhalp
left a comment
There was a problem hiding this comment.
LGTM. just a couple suggestions.
src/lib/edge-functions/registry.ts
Outdated
| return | ||
| } | ||
|
|
||
| if (route.headers && headers) { |
There was a problem hiding this comment.
🤔 what happens if route.headers but !headers? we might intend to match specific headers but we wouldn't run that logic? is !headers every expected? could we handle this more gracefully and explicitly?
There was a problem hiding this comment.
I don't think we should ever hit !headers, though. Removed in 3cc29d0.
| return pattern.test(headerValueString) | ||
| } | ||
|
|
||
| return false |
There was a problem hiding this comment.
am I reading correctly that this is an invalid user configuration? since we're in dev, it might be helpful DX to print a user warning in this case.
There was a problem hiding this comment.
Not really. This is an internal structure that has been generated from parsing the edge function files and the TOML config. If there's an error in syntax, it will be thrown by edge-bundler.
From my tests, we might need some additional logic to surface those errors better (unrelated to this PR), which I'll do in a follow-up.
There was a problem hiding this comment.
Actually, I've done it in 7d6f789 because it's a small fix (and you now you're obliged to review!).
Summary
Adds support for matching edge functions by header.