Skip to content

Address Log Viewer first render issues - #6

Merged
Tim Mulholland (tlmii) merged 1 commit into
microsoft:mainfrom
tlmii:dev/log-prerender-queue
Oct 2, 2023
Merged

Address Log Viewer first render issues#6
Tim Mulholland (tlmii) merged 1 commit into
microsoft:mainfrom
tlmii:dev/log-prerender-queue

Conversation

@tlmii

Copy link
Copy Markdown
Member

Even though we were delaying the calls to LogViewer.AddLogEntriesAsync until the LogViewer component was available in its parent, that did not guarantee that the LogViewer component had fully rendered itself and the JS functionality would be available. This caused the first load of the ProjectLogs and ContainerLogs pages to often be blank.

To address this, we create a queue of logs that are held just for pre-render and are then written out to the DOM once OnAfterRenderAsync is called. Subsequent calls to AddLogEntriesAsync will just write directly to the DOM as expected.

@tlmii
Tim Mulholland (tlmii) merged commit 7358457 into microsoft:main Oct 2, 2023
@tlmii
Tim Mulholland (tlmii) deleted the dev/log-prerender-queue branch October 2, 2023 19:08
@github-actions github-actions Bot locked and limited conversation to collaborators May 1, 2024
Adam Ratzman (adamint) added a commit to ellahathaway/aspire that referenced this pull request May 30, 2026
Fix microsoft#1: Rename command IDs in package.json menus and walkthrough so
they target the new aspire-vscode.runAppHostCommand and
aspire-vscode.debugAppHostCommand registrations introduced in this PR.
Without this the editor title bar, explorer context menu, and Get
Started walkthrough Run/Debug buttons silently no-op.

Fix microsoft#2: Wrap vscode.debug.startDebugging in try/catch in
AppHostLaunchService.launch so a 'false' return value (debug adapter
rejected) or thrown error clears the launching state. Otherwise the
tree item is stuck showing the 'Starting...' spinner forever and the
user cannot retry.

Fix microsoft#3: Make AspireAppHostTreeProvider.runAppHost async and await
launch so launch failures surface via showErrorMessage instead of
being dropped as unhandled promise rejections.

Fix microsoft#4: In workspace mode with multiple candidate AppHost paths, match
running AppHosts to candidates by directory equivalence
(isMatchingAppHostPath) rather than exact path. This is the same
matching used elsewhere in AppHostDataRepository when correlating
'aspire ps' output to candidate paths, so canonicalization
differences (case, separators, trailing slashes) no longer cause a
running AppHost to display as idle.

Fix microsoft#5: Introduce aspire.noRunningAppHosts context key so the Open
Dashboard palette command is only enabled when at least one AppHost
is actually running. Previously the palette appeared when only idle
candidates were known and then silently no-oped.

Fix microsoft#6: Widen the workspaceResources contextValue regex in
package.json so the read-only 'Open AppHost Source' and 'Copy AppHost
Path' actions appear on bare 'workspaceResources' items, not only on
'workspaceResources:hasAppHost'. The destructive 'Stop AppHost' menu
remains gated on :hasAppHost.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adam Ratzman (adamint) added a commit that referenced this pull request May 30, 2026
* Show idle AppHosts in Aspire pane with Run/Debug context menu

* Rename view ID from runningAppHosts to appHosts

* Rename context key from noRunningAppHosts to noAppHosts

* Keep panel visible when stopped AppHost has workspace candidates

When an AppHost stops, the noAppHosts context key now considers
workspace candidates. This ensures the panel shows idle AppHosts
instead of the empty welcome view after a running AppHost is stopped.

* Fix noAppHosts assertions: workspace candidates keep panel visible

The _updateWorkspaceContext change (0e312f9) added !hasWorkspaceCandidates
to the noAppHosts condition, meaning the panel stays visible when idle
AppHosts are discovered. Two tests asserted noAppHosts=true after describe
exit, but the legacy format candidate is treated as buildable (toAppHostCandidate
defaults null status to 'buildable'), so workspace candidates persist and
noAppHosts is correctly false.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Make workspace AppHosts expandable with launch actions

* Address review feedback for PR #17506

Fix #1: Rename command IDs in package.json menus and walkthrough so
they target the new aspire-vscode.runAppHostCommand and
aspire-vscode.debugAppHostCommand registrations introduced in this PR.
Without this the editor title bar, explorer context menu, and Get
Started walkthrough Run/Debug buttons silently no-op.

Fix #2: Wrap vscode.debug.startDebugging in try/catch in
AppHostLaunchService.launch so a 'false' return value (debug adapter
rejected) or thrown error clears the launching state. Otherwise the
tree item is stuck showing the 'Starting...' spinner forever and the
user cannot retry.

Fix #3: Make AspireAppHostTreeProvider.runAppHost async and await
launch so launch failures surface via showErrorMessage instead of
being dropped as unhandled promise rejections.

Fix #4: In workspace mode with multiple candidate AppHost paths, match
running AppHosts to candidates by directory equivalence
(isMatchingAppHostPath) rather than exact path. This is the same
matching used elsewhere in AppHostDataRepository when correlating
'aspire ps' output to candidate paths, so canonicalization
differences (case, separators, trailing slashes) no longer cause a
running AppHost to display as idle.

Fix #5: Introduce aspire.noRunningAppHosts context key so the Open
Dashboard palette command is only enabled when at least one AppHost
is actually running. Previously the palette appeared when only idle
candidates were known and then silently no-oped.

Fix #6: Widen the workspaceResources contextValue regex in
package.json so the read-only 'Open AppHost Source' and 'Copy AppHost
Path' actions appear on bare 'workspaceResources' items, not only on
'workspaceResources:hasAppHost'. The destructive 'Stop AppHost' menu
remains gated on :hasAppHost.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Adam Ratzman <adam@adamratzman.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants