Skip to content

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Sep 10, 2025

🎉 Thanks for submitting a pull request! 🎉

Summary

Potential alternative to #6657 , this time following handling we already have for serverless functions for early bail if there are no functions to bundle

logFunctionsToBundle({
logs,
userFunctions,
userFunctionsSrc: relativeFunctionsSrc,
userFunctionsSrcExists: functionsSrcExists,
internalFunctions,
internalFunctionsSrc: relativeInternalFunctionsSrc,
frameworkFunctions,
generatedFunctions: getGeneratedFunctionsByGenerator(generatedFunctions),
})
if (
userFunctions.length === 0 &&
internalFunctions.length === 0 &&
frameworkFunctions.length === 0 &&
generatedFunctions.length === 0
) {
return {}
}
const { bundlers } = await zipFunctionsAndLogResults({


For us to review and ship your PR efficiently, please perform the following steps:

  • Open a bug/issue before writing your code 🧑‍💻. This ensures
    we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or
    something that`s on fire 🔥 (e.g. incident related), you can skip this step.
  • Read the contribution guidelines 📖. This ensures
    your code follows our style guide and passes our tests.
  • Update or add tests (if any source code was changed or added) 🧪
  • Update or add documentation (if features were changed or added) 📝
  • Make sure the status checks below are successful ✅

A picture of a cute animal (not mandatory, but encouraged)

Copy link
Contributor

This pull request adds or modifies JavaScript (.js, .cjs, .mjs) files.
Consider converting them to TypeScript.

return await pathExists(frameworkFunctionsSrc)
}

const logFunctions = async ({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

content of this function was just inlined in step function directly to get easy access to found edge functions to be able to bail early if there are no functions at all

Copy link
Member

@eduardoboucas eduardoboucas left a comment

Choose a reason for hiding this comment

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

LGTM.

I think there's also the case where function directories do exist but there are no registered routes. For us to bail there, I guess we'd need to do it in edge-bundler?

@pieh pieh changed the title Handle empty cases in bundling alt fix: handle empty cases in edge bundling Sep 10, 2025
@pieh
Copy link
Contributor Author

pieh commented Sep 24, 2025

I think there's also the case where function directories do exist but there are no registered routes.

Isn't what this PR does now that even if source dirs exist, but contain no functions we would not be generating edge bundles? There are added test fixtures and tests for that. If added test cases don't contain a case you thinking of here - can you describe a setup to test?

For us to bail there, I guess we'd need to do it in edge-bundler?

Bailing in edge-bundler was initial attempt in #6657, but as mentioned by Phil in #6657 (comment) because there is no early bail like that in edge-bundler now, adding one would be a breaking change (which is not a blocker, but requires more work from all consumers of the package and because we could come up with alternative for scenarios we were thinking of without breaking change - I went with that)

@pieh pieh marked this pull request as ready for review September 24, 2025 08:47
@pieh pieh requested a review from a team as a code owner September 24, 2025 08:47
@pieh pieh merged commit d78700c into main Sep 24, 2025
95 of 98 checks passed
@pieh pieh deleted the handle-empty-cases-in-bundling-alt branch September 24, 2025 09:29
This was referenced Sep 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants