-
Notifications
You must be signed in to change notification settings - Fork 367
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: don't run form submission logic on paths matching function #6048
Conversation
if ( | ||
req.url.startsWith('/.netlify/') || | ||
req.method !== 'POST' || | ||
(await functionsRegistry.getFunctionForURLPath(req.url, req.method)) |
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.
Is it possible that functionsRegistry
isn't defined here? It's a bit hard to tell, but we do have a few checks for it elsewhere in the codebase, which makes me think it may not be there at times.
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.
From what i'm seeing, this is the only codepath where this is ever going to be called:
cli/src/lib/functions/server.mjs
Lines 276 to 290 in 62e0ef5
const functionsRegistry = new FunctionsRegistry({ | |
capabilities, | |
config, | |
debug, | |
isConnected: Boolean(siteUrl), | |
// functions always need to be inside the packagePath if set inside a monorepo | |
projectRoot: command.workingDir, | |
settings, | |
timeouts, | |
}) | |
await functionsRegistry.scan(functionsDirectories) | |
const server = getFunctionsServer(Object.assign(options, { functionsRegistry })) |
So functionsRegistry
is always defined.
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.
👍🏻 🚀
Part of https://github.com/netlify/pod-dev-foundations/issues/578. See title.