Skip to content
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

Merged
merged 5 commits into from
Oct 9, 2023

Conversation

Skn0tt
Copy link
Contributor

@Skn0tt Skn0tt commented Oct 9, 2023

@Skn0tt Skn0tt requested a review from a team as a code owner October 9, 2023 15:56
@Skn0tt Skn0tt self-assigned this Oct 9, 2023
@github-actions
Copy link

github-actions bot commented Oct 9, 2023

📊 Benchmark results

Comparing with c260f2d

  • Dependency count: 1,386 (no change)
  • Package size: 389 MB ⬇️ 0.00% decrease vs. c260f2d

eduardoboucas
eduardoboucas previously approved these changes Oct 9, 2023
if (
req.url.startsWith('/.netlify/') ||
req.method !== 'POST' ||
(await functionsRegistry.getFunctionForURLPath(req.url, req.method))
Copy link
Member

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.

Copy link
Contributor Author

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:

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants