remote viewer: rebuild remote preview on every editor keystroke#11878
remote viewer: rebuild remote preview on every editor keystroke#11878ogoffart wants to merge 2 commits into
Conversation
Previously the remote viewer only recompiled when the LSP sent ShowPreview. SetContents from intermediate keystrokes was cached but never triggered a rebuild. Emit a new ContentsChanged message from the cache update, and have the viewer re-run build_and_show when the changed file is a dependency of the currently shown component (mirroring tools/lsp/preview.rs:set_contents).
| .set_property("message", SharedString::from("Component not found").into()) | ||
| { | ||
| tracing::error!("Failed setting property: {err}"); | ||
| if let Some(BuildOutcome { new_instance, watch_paths }) = build_and_show( |
There was a problem hiding this comment.
So this will immediately recompile, right?
We should add a debounce delay here, same as for the native preview, so that if I e.g. check out a different branch we don't end up queuing a bunch of recompiles.
|
|
||
| connection.send(message).ok(); | ||
|
|
||
| return Some(BuildOutcome { new_instance: None, watch_paths: None }); |
There was a problem hiding this comment.
I can see this causing bugs, as we won't update the dependencies list on error.
So if I try to import a file which does not yet exist, we hit this path.
Then when the new file is added via SetContents, it will not be in the dependencies list and won't trigger a recompile.
There was a problem hiding this comment.
Even on a build with errors, the type loader should still return the correct list of files to watch. So we should just use the watch paths here I think 🤔
|
|
||
| /// Compile `preview_component` (using `file_cache` for the in-memory editor buffer if available) | ||
| /// and replace the visible instance with it. Returns `None` only when the host window cannot be | ||
| /// reused, which is a fatal condition for the caller. |
There was a problem hiding this comment.
This function mentions that returning None is a fatal condition to the caller, but the call sites don't treat this as a fatal error...
| ConnectionMessage::ContentsChanged { url, file_cache } => { | ||
| let Some(preview_component) = current_preview.clone() else { continue }; | ||
| let Ok(path) = url.to_file_path() else { continue }; | ||
| if !dependencies.contains(&path) { |
There was a problem hiding this comment.
In general, I think the dependency tracking should not live here. It should live in the live preview crate.
The connection message basically should just say that there are actual changes to the preview and that it needs to be reloaded in the UI.
2 participants
Previously the remote viewer only recompiled when the LSP sent ShowPreview. SetContents from intermediate keystrokes was cached but never triggered a rebuild. Emit a new ContentsChanged message from the cache update, and have the viewer re-run build_and_show when the changed file is a dependency of the currently shown component (mirroring tools/lsp/preview.rs:set_contents).