-
Notifications
You must be signed in to change notification settings - Fork 394
fix: fix framework server loading spinner ("Waiting for framework port") #7242
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
Conversation
@@ -68,7 +68,7 @@ export const startFrameworkServer = async function ({ | |||
throw new Error(`Timed out waiting for port '${settings.frameworkPort}' to be open`) | |||
} | |||
} | |||
stopSpinner({ error: false, spinner }) | |||
spinner.success() |
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.
We want to draw the ✔️, not erase it
5a2e430
to
7a7bbde
Compare
spinner.clear() | ||
} | ||
// Clear the spinner, write the framework command line, then resume spinning | ||
spinner?.clear() | ||
writeStream.write(chunk, () => { | ||
spinner?.spin() |
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.
I believe this still isn't quite right — spin() renders a single frame, but here we want to keep looping even when we aren't receiving chunks from the framework server.
But this fix is worth getting in now and I'll follow up when I get the full fix working.
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.
(In that follow-up I'll also look into how we can add test coverage for this)
This is a regression in the loading spinner shown before "Waiting for framework port", introduced in 19.0.3.
See: 1. #7026 2. #7131 3. #7124 Oops, I was really overthinking this. All this needs to do is, every time a chunk comes in from the underlying framework dev server, clear the spinner line, write the chunk, then resume the spinner. This wasn't set up right at all, which was obfuscated by the erroneous use of `isSpinning` (see previous commit).
Summary
This is a regression in the loading spinner shown before "Waiting for framework port", introduced in 19.0.3.
See:
ora
andlog-symbols
with tiny dependencynanospinner
#7026Oops, I was really overthinking this. All this needs to do is, every time a chunk comes in from the underlying framework dev server, clear the spinner line, write the chunk, then resume the spinner.
This wasn't set up right at all, which was obfuscated by the erroneous use of
isSpinning
.Fixes #7124.
Thank you @Florian-Mt for spotting the (embarrassing!) fix.
Before
After