feat: send functions_config during deploy #5428
Conversation
khendrikse
added
the
type: feature
📊 Benchmark resultsComparing with cfcee54 Package size: 265 MB(no change) Legend
|
khendrikse
force-pushed
the
feat-204/send-functions-config-during-deploy
branch
from
eeb0564 to
a1fd791
Compare
Skn0tt
left a comment
Skn0tt
left a comment
There was a problem hiding this comment.
looks generally good to me. left a tiny comment.
do we have tests for this behaviour? if yes, we should probably update one to assert on functions config. If no, we should make an issue to write tests.
src/utils/deploy/hash-fns.mjs
Outdated
| const fnConfig = functionZips.reduce( | ||
| (funcs, curr) => (curr.displayName ? { ...funcs, [curr.name]: { display_name: curr.displayName } } : funcs), | ||
| {}, | ||
| ) |
There was a problem hiding this comment.
this reads somewhat complicated to me. would this have the same effect?
| const fnConfig = functionZips.reduce( | |
| (funcs, curr) => (curr.displayName ? { ...funcs, [curr.name]: { display_name: curr.displayName } } : funcs), | |
| {}, | |
| ) | |
| const fnConfig = Object.fromEntries( | |
| functionZips | |
| .filter(func => !!func.displayName) | |
| .map(func => [func.name, { display_name: func.displayName}]) | |
| ) |
There was a problem hiding this comment.
It would not be exactly the same effect, as the result of the reduce would be an object, and the map would be an array. But I did add a filter like you did now, in front of the reduce, maybe that already makes it a bit easier to read?
There was a problem hiding this comment.
and the map would be an array.
that's what the Object.fromEntries would do, wouldn't it?
There was a problem hiding this comment.
Oooh sorry I totally did not see that part 😆 in that case I think I would still prefer it as a reduce, for me that is a bit more readable, as it shows how the object will look. But I do understand that it might be personal preference as well 🤔
do we have tests for this behaviour? if yes, we should probably update one to assert on functions config. If no, we should make an issue to write tests. @Skn0tt I tried to add a test, but honestly it was... not as easy as I'd like. I'll make an issue to do this so I can spend a bit more time on it 👍 |
|
|
||
| return await zipFunctions(directories, tmpDir, { basePath: rootDir, config: functionsConfig }) | ||
| return await zipFunctions(directories, tmpDir, { | ||
| featureFlags: { project_deploy_configuration_api_use_per_function_configuration_files: true }, |
There was a problem hiding this comment.
Another note for future reference: in the same way we have a single place for all edge functions feature flags, it would be good to do the same for serverless functions. This will ensure the different entry points into zip-it-and-ship-it use the same set of flags, and thus avoiding a situation where two commands have different behaviour leading to a harder time troubleshooting problems.
🎉 Thanks for submitting a pull request! 🎉
Summary
I'm sending along the functions config that contains any display names to the deploy command :)
Part of https://github.com/netlify/pod-compute/issues/156