-
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(dev): reloading edge functions for child dependencies #6276
Conversation
📊 Benchmark resultsComparing with a4b7449
|
The rough problem seems to be that we were only checking one level-deep dependencies. I haven't fully grasped the change you're proposing here, but traversing the full module graph seems to be the right solution. As for adding a test, I'd add a case similar to cli/tests/integration/commands/dev/edge-functions.test.ts Lines 135 to 152 in 3379f40
Reloading function .
|
I've added a test, refactored the code to make it a bit more concise, and added a couple other small refactorings. @JGAntunes would love your review! |
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't approve because I kicked of the PR 😅 but LGTM
🎉 Thanks for submitting a pull request! 🎉
Summary
Fixes COM-209 - https://linear.app/netlify/issue/COM-209/reloading-issues-with-edge-function-files-when-using-cli-netlify-dev. The way we currently process the dependency graph of edge function is limited to the direct dependencies of it. This PR changes it to perform a depth first search and build a full depedency list for a given function.
Keeping it as draft as I would love to add a test for it but unsure what would be the best way to go about it? 🤔 Any ideas? Maybe @Skn0tt?
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)