fix: Remove a useless if to setShowSpinner(true)
#271
Conversation
The code says: if the spinner must be shown, then let's set the spinner to be shown. This is redundant. This patch removes the `if (showSpinner)` to keep only the `setShowSpinner(true)`, as it will have the same behaviour. Even if the intend was to call `setShowSpinner` when `showSpinner` was `false`, it wasn't working before but will work now.
| @@ -86,7 +86,7 @@ export async function transpileComponent (component, opts = {}) { | |||
| let spinner; | |||
| const showSpinner = getShowSpinner(); | |||
| if (opts.optimize) { | |||
| if (showSpinner) setShowSpinner(true); | |||
| setShowSpinner(true); | |||
There was a problem hiding this comment.
This isn't actually redundant, because setShowSpinner internally sets the spinner show value to false. It's weird logic I know, but it does its job I think (which is to determine if the state needs to be "switched", since the spinner itself is imperatively created). optimizeComponent will then run its own spinner lifecycle, showing and hiding the spinner, after which the showSpinner value will again bubble up to the transpile operation itself.
We only want opimtimizeComponent to show the spinner if it actually already was originally being show, determined by the if (showSpinner) check.
It's convoluted sure, but the logic works!
There was a problem hiding this comment.
That is, if showSpinner is false I'm not sure we want to show the spinner? Specifically we need to respect opts.quiet.
|
Thanks for the triage. |
The code says: if the spinner must be shown, then let's set the spinner to be shown. This is redundant.
This patch removes the
if (showSpinner)to keep only thesetShowSpinner(true), as it will have the same behaviour. Even if the intend was to callsetShowSpinnerwhenshowSpinnerwasfalse, it wasn't working before but will work now.