VSCode 1.9.0 breaks MSSQL extension - preview HTML pane never refreshes on
kevcunnane opened this issue · 8 comments
- VSCode Version: 1.9.0
- OS Version: All
Steps to Reproduce:
A change to editor initialization in VSCode 1.9.0 is breaking the MSSQL extension – any query execution after the 1st time you open a results pane in the WebView is failing to refresh. This means that query execution appears to fail which really is our key experience. Please note this is a breaking change in behavior from previous versions of VSCode.
We raised this a couple of days ago as #19654 which was closed with instructions to change our API call. After fixing our code we discovered the problem happens even if when calling the API correctly.
- Previously, calling previewHtml would refresh the page if it already existed. This ensured our page was reloaded so the UI could show new query results.
- Now, it only refreshes if it goes out of focus and comes back into focus. If it’s the main tab on ViewColumn 2, for example, it’ll never be refreshed.
- This breaks our existing, released extension's behavior.
So this code snippet will open an editor with a WebView component the 1st time it runs, but subsequent calls will never refresh the page.
previewCommandPromise = vscode.commands.executeCommand('vscode.previewHtml', resultsUri, resultPaneColumn, paneTitle);
The impactful change causing this only went into the main branch 6 days ago in https://github.com/Microsoft/vscode/releases/tag/translation%2F20172701.01. We believe the change is due to going from side by side to editor groups control: 0763cc8.
What are the next steps to finding a solution to this? I can imagine we are not the only extension that might be hit by this change. We are looking for a workaround on our side but would appreciate if we can get help on a solution or understand if this could be hotfixed by VSCode.
@kevcunnane the best way to make progress is to provide precise reproducable steps that allow us to reproduce the issue. Then we can debug into the problem. We have also investigated into issue #19654 in the development team. but it has suffered from the same problem and did not provide reproduceable steps.
Thanks - here are the exact steps to reproduce when using our extension. We can work on getting a separate repro in a clean sample extension if needed. Please let me know if this isn't sufficient and we will work on a better repro.
Configuration to connect to SQL from the extension
You need to setup a connection profile to connect to SQL, and create a .sql file that lets you execute SQL queries. Steps are as follows:
- Have a SQL Server instance which you can connect to.
- If you do not have one, setup Docker as instructed here. Note you need Docker configured to run with 4GB of RAM, and should take note of the password you use when starting it.
- Install the MSSQL extension and reload Visual Studio Code
- Open a .sql file, including 2 statements:
select * from sys.databases
GO
select * from sys.tables
- Hit F1 and choose the
MS SQL: Connect
action. - Enter connection details:
- server name =
localhost
for docker - database name = empty (hit Return)
- username =
sa
- password =
YourStrong!Passw0rd
(whatever you entered when starting docker) - Save Password = true
- server name =
- This should create a connection profile and you will see the connection in the bottom right status bar
Repro steps - execute 2 separate queries
- Highlight the text
select * from sys.databases
and executeMS SQL: Execute Query
(Ctrl+Shift+E) to run- You will notice the results editor tab opens, which is a WebView component. It shows a result set listing databases
- Highlight the text
select * from sys.tables
and execute theMS SQL: Execute Query
Expected:
- The results editor tab is refreshed. You see a new executing event and a different result set with tables is shown
Actual
- The results editor tab is not refreshed. No change happens from the user perspective - the sys.databases results are still shown
- Note that if you close the results editor tab (or change focus from it by making a different editor active in the same ViewColumn) before calling
Execute Query
the tab is refreshed.
@kevcunnane thanks, we can work with that.
We've traced the issue on our side and have a fix that we'll be able to apply as an extension update. We will work to get this fix out ASAP so that users are unblocked.
HotFix information
Ensure that TextDocumentContentProvider.provideTextDocumentContent
always provides an updated string when being refreshed.
Root cause
We implement TextDocumentContentProvider
to represent our results. This expects you to fire an onDidChange
event whenever you want to refresh your content, which then requests the updated document contents by calling your provideTextDocumentContent
method.
- We always return the same string, which kicks off our mini HTML app
- In VSCode 1.9.0, it does a string comparison and only refreshes the page if the content is different
- A short term fix is therefore to change the content so that it detects a difference and refreshes the document
It's not clear exactly what changed on the VSCode side that caused the change in behavior. However we'll work to fix in our code via this short term hotfix and a better long term solution where our results document is kept in sync without refreshing. Thanks.
Removing the important label, since there is a fix in the SQL extension.
Thanks for the detailed steps and thanks for drilling so deep into this. The issue you are facing was an intentional change to fix #18664.
We added the TextDocumentContentProvider
-API to allow extensions to show virtual documents in editors, like sources from dlls or files from the git index. The string a content provider returns is kept in an editor model which does tokenisation, line management, etc. Not all of that is cheap to compute and therefore we added logic to not recompute things when the underlying string of a model didn't change.
Now comes the previewHtml
-case which was designed to render a document (on disk or virtual) as html. The design wasn't to allow UI extensibility but we acknowledge that it's being used like that. Now there is a conflicting goal between the original use-case (showing a document that's not on disk) and UI extensibility use-case.
Understanding how two different parties use the content provider it seems both are right. Still, the original design is the one which wants to show a document in an editor. That makes me not wanna revert the fix for #18664. By using the preview-command to build UI you are on the dark side of the force and tricks like fake-updates to a fake documents might be what you need. Us, the VS Code team, should come up with a better story for adding UI to the editor that doesn't require virtual doc, servers, preview commands etc.
Looking forward we should make sure we catch those things sooner. @kevcunnane I wonder if a few devs on your team can self host on the insiders version of VS Code. We update daily, changes and unhappy fixes should become on table a lot quicker, resulting in a lot less stress for everyone.
Last, if no one disagrees, I'd close this without code change
This makes sense to me - the API needs to work for many situations, it's just unfortunate we didn't detect it early enough. I'm on Insiders myself (and did catch pre-release, just way too close to the release date) but will encourage the whole team to use it. Also if we can get the proper fix into our code that better manages our state we'll be more insulated from future breaking changes.
Longer term, it would be nice to have a more supported way to integrate. We're always at a higher risk of breakage due to the nature of the previewHtml implementation. Thanks for the speedy investigation and responsiveness!
Ok. Closing. Happy Coding!