Skip to content

Comments

fix: show LocalStack setup on only when setup is required#58

Merged
tiurin merged 8 commits intomainfrom
skip-setup-checks-when-file-watchers-not-ready
Sep 12, 2025
Merged

fix: show LocalStack setup on only when setup is required#58
tiurin merged 8 commits intomainfrom
skip-setup-checks-when-file-watchers-not-ready

Conversation

@tiurin
Copy link
Collaborator

@tiurin tiurin commented Sep 12, 2025

We've had a couple of race conditions and missing status check dependencies. This resulted in LocalStack Setup notification being shown when the setup is not required - this was happening on the most of VS Code starts.

This PR fixes it with the following:

  • Add explicit status initialization on every status tracker creation.
  • Show Setup Wizard pop-up once setup is required. This ensures that pop-up appears if setup is required at the start. Instead of synchronizing statuses at the start and ensuring Setup Wizard is only popping up once, we decided to have pop-up appearing every time the setup is broken. This shouldn't happen normally, but when it happens then setup wizard pop-up in addition to painting setup bar red can actually be helpful.
  • License status tracker now watches both license and auth token file (not very clean solution, in the next iteration we should add behavioral dependency between license tracker and auth tracker).
  • Refresh status bar is now happening after the last setup wizard step. Before it was in the middle.

@anisaoshafi anisaoshafi changed the title Skip setup checks when file watchers not ready fix: skip setup checks when file watchers not ready Sep 12, 2025
@tiurin tiurin changed the title fix: skip setup checks when file watchers not ready fix: setup LocalStack is shown on extension start when not necessary Sep 12, 2025
@tiurin tiurin marked this pull request as ready for review September 12, 2025 11:41
@tiurin tiurin changed the title fix: setup LocalStack is shown on extension start when not necessary fix: show LocalStack setup on extension start only when setup is required Sep 12, 2025
});

// Update the status immediately on file tracker initialization
void updateStatus();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI I checked the docs and chokidar docs should call one of their callbacks upon initialization:

ignoreInitial (default: false). If set to false then add/addDir events are also emitted for matching paths while instantiating the watching as chokidar discovers these file paths (before the ready event).

So I think this shouldn't be required, although it doesn't hurt either.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I retract my words.

Chokidar will only emit add if the file was already present, but won't emit unlink. We are interested in that, as well, so we have to run the check.

This ensures that pop-up appears if setup is required at the start. Instead of synchronizing changes at the start we decided to have pop-up appearing every time the setup is broken. This shouldn't happen normally, but when it happens then setup wizard pop-up in addition to painting setup bar red can actually be helpful.
Copy link
Collaborator

@skyrpex skyrpex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to restore the early return in case some of the values were undefined. Thanks @anisaoshafi for catching the bug.

Image

@tiurin tiurin changed the title fix: show LocalStack setup on extension start only when setup is required fix: show LocalStack setup on only when setup is required Sep 12, 2025
@tiurin tiurin requested a review from skyrpex September 12, 2025 12:49
@tiurin tiurin enabled auto-merge (squash) September 12, 2025 12:52
@tiurin tiurin merged commit 866146a into main Sep 12, 2025
3 checks passed
@tiurin tiurin deleted the skip-setup-checks-when-file-watchers-not-ready branch September 12, 2025 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants