-
Notifications
You must be signed in to change notification settings - Fork 63
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 Server polling and discovery #1274
Conversation
This allows better matching to know when to avoid polling
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
@@ -71,7 +73,7 @@ func Autodiscover(ctx context.Context) map[string]struct{} { | |||
for _, path := range Paths { | |||
// These requests _should_ be fast as we know a port is open, | |||
// so we do these sequentially. | |||
url := fmt.Sprintf("http://127.0.0.1:%d%s", port, path) | |||
url := util.NormalizeAppURL(fmt.Sprintf("http://127.0.0.1:%d%s", port, path), false) |
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 fixes an issue where, even if polling was disabled, an app would be discovered at this URL and pinged, as this raw URL was different to the normalized URL within the existing app.
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.
These are great improvements.
Motivation
Polling and discovery in the Dev Server can be really verbose. Many frameworks, by default, will log all incoming HTTP requests, resulting in a huge spam of logs while the Dev Server is running. This makes debugging harder and it just looks messy.
In addition, the
--no-discovery
and--no-poll
flags can unintentionally remove features, meaning users cannot achieve a balance of automatically adding the apps they want without spamming.Description
This PR aims to clear up the confusion of these flags the functionality it controls.
In this area, there are a few scenarios to cover. This is the behaviour we're aiming for, with⚠️ marks representing that this is not the current functionality and will be fixed in this PR.
--no-poll
--no-discovery
--no-poll --no-discovery
--sdk-url
--no-poll --sdk-url
--no-discovery --sdk-url
--no-poll --no-discovery --sdk-url
From this, we see an incompatibility with
--no-poll
and other options. To fix this, if--no-poll
is specified:With the latter, an erroring app means that either it did exist or a user manually added it via the flag or UI. If this is the case, should we poll until we find it?
UI
The UI also always shows that we are "Auto-detecting apps..." and doesn't actually check whether this feature is enabled or not.
Type of change (choose one)
Checklist
Check our Pull Request Guidelines