-
Notifications
You must be signed in to change notification settings - Fork 368
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: determine duplicate function precedence based on extension #5617
fix: determine duplicate function precedence based on extension #5617
Conversation
📊 Benchmark resultsComparing with 219dd3c Package size: 313 MB⬇️ 0.00% decrease vs. 219dd3c
Legend
|
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.
Can we add a test?
I will! |
3684235
to
c5a759a
Compare
@@ -212,7 +212,8 @@ export class FunctionsRegistry { | |||
await Promise.all(deletedFunctions.map((func) => this.unregisterFunction(func.name))) | |||
|
|||
await Promise.all( | |||
functions.map(async ({ displayName, mainFile, name, runtime: runtimeName }) => { | |||
// We reverse the array so we get the right functions precedence | |||
functions.reverse().map(async ({ displayName, mainFile, name, runtime: runtimeName }) => { |
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.
how does this solve the problem? Can you describe how this works?
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.
Sure! I added an extra comment now :) zip-it-and-ship-it returns an array with the last functions preceding the previous. But we need to reverse that in the cli
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.
Nice, so much simpler :)
@@ -212,7 +212,8 @@ export class FunctionsRegistry { | |||
await Promise.all(deletedFunctions.map((func) => this.unregisterFunction(func.name))) | |||
|
|||
await Promise.all( | |||
functions.map(async ({ displayName, mainFile, name, runtime: runtimeName }) => { | |||
// We reverse the array so we get the right functions precedence | |||
functions.reverse().map(async ({ displayName, mainFile, name, runtime: runtimeName }) => { |
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.
Nice, so much simpler :)
This broke some tests I think. |
I'm really confused as to how I was able to merge this PR :/ |
🎉 Thanks for submitting a pull request! 🎉
Summary
Fixes https://github.com/netlify/pod-compute/issues/462
Right now, the cli doesn't behave the same as production when it comes to duplicate function precedence. This should fix it. For more detail check the issue1
For us to review and ship your PR efficiently, please perform the following steps:
passes our tests.
A picture of a cute animal (not mandatory, but encouraged)