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
feat: print function URL on first load (closes #3618) #3646
Conversation
π Benchmark resultsComparing with 17dc0b6 Package size: 357 MBβ¬οΈ 0.52% increase vs. 17dc0b6
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.
LGTM!
My only concern is that this adds a lot of noise to the output, especially when there are dozens of functions. Rather than adding the full URL to the log line, could we experiment with using something like https://www.npmjs.com/package/terminal-link?
@netlify-team-account-1 would you mind adding a screenshot of what the output looks like with some functions, and then we ask @joeyaiello for his thoughts?
Sure thing! @joeyaiello what are your thoughts on this? |
@joeyaiello @eduardoboucas WDYT about moving forward with this and then potentially incrementing on smth like terminal-link? |
Added Unfortunately, it works neither in standard MacOS terminal, nor in the VSCode terminal, nor in Hyper (even thought that's on the list of supported terminals!). When it doesn't work, the fallback looks exactly as the original screenshot,, so it's still kind of cluttered. An alternative would be to keep the current behaviour of not printing the URL, and only |
Can we change the fallback to be just the URL in parenthesis, instead of showing "available under"? -Loaded function echo available under: http://localhost:8080/.netlify/functions/echo
+Loaded function echo (http://localhost:8080/.netlify/functions/echo) |
yes! implemented in e722ad1 |
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.
LGTM!
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.
π
This PR currently has a merge conflict. Please resolve this and then re-add the |
Summary
Fixes #3618
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)