fix: await zipFunctions() in functions build
#7180
Conversation
📊 Benchmark resultsComparing with e7cd9e5
|
src/utils/functions/functions.ts
Outdated
| ): string | undefined => | ||
| ('functions' in options && typeof options.functions === 'string' ? options.functions : null) ?? | ||
| config.dev?.functions ?? | ||
| config.functionsDirectory ?? |
There was a problem hiding this comment.
Isn't this a behavioral change? Won't this now allow config.dev.functions and config.functionsDirectory to be an empty string? Or do we sanitize empty strings out somewhere?
There was a problem hiding this comment.
I'm not sure how you could even end up with an empty string, other than explicitly using "" in your netlify.toml or something... 🤔
I feel like it isn't worth the added complexity to this line to support ignoring hypothetical empty strings here, but in the interest of limiting behavioural changes I'll go ahead and add the conditions 🙈.
There was a problem hiding this comment.
I don't think it's worth the added complexity here or the change risk, however low that risk is (you're still adding in new behavior by checking for strings with only whitespace in them). I don't think that bundling it in with a clear and totally unrelated bug is really the way to go about this change, either.
There was a problem hiding this comment.
I thought that trim() wouldn't be controversial, but alright, I've reverted that too.
| @@ -10,7 +10,7 @@ import type BaseCommand from '../base-command.js' | |||
| export const functionsBuild = async (options: OptionValues, command: BaseCommand) => { | |||
| const { config } = command.netlify | |||
|
|
|||
| const src = options.src || config.build.functionsSource | |||
| const src = ('src' in options && typeof options.src === 'string' ? options.src : null) ?? config.build.functionsSource | |||
There was a problem hiding this comment.
Same here--this will now allow an empty string, won't it?
Summary
The types fixes in #7130 identified a number of bugs. This fixes one of those.