microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
163.67k stars 29.06k forks source link

Emmet JSX HTML abbreviations suggested inside `style` tag #119736

Open rzhao271 opened 3 years ago

rzhao271 commented 3 years ago

From https://github.com/styled-components/vscode-styled-components/issues/191

Jak-Ch-ll commented 3 years ago

I came across the issue of getting Emmet HTML abbreviations in JSX inside of curly braces. Since that didn't happen until fairly recently, I did the following:

Since activity spiked in the styled-component issue within this month, I assume, it is the same issue and was brought in within one of the last patches.

Here is a comparison between the versions: image 1.54.3

image 1.53.2

rzhao271 commented 3 years ago

@Jak-Ch-ll see https://github.com/microsoft/vscode/issues/118363 for that issue. It should already be fixed in Insiders, and should appear in Stable by the end of this week.

Jak-Ch-ll commented 3 years ago

@rzhao271 You are absolutly correct, just tested with Insiders. Thanks a lot for your work!

rzhao271 commented 3 years ago

Repro steps:

  1. Create a new empty JSX document with the following text:
    const styledHeader = styled.header`
    .bar {
        border-bottom: 10px solid;
        |
    }
    `;
  2. Place the cursor on | and try typing body.
  3. :bug: an HTML suggestion comes up.

The main issue is that when it comes to Emmet suggestions, the language to find abbreviations for is obtained from the document's languageId: https://github.com/microsoft/vscode/blob/main/extensions/emmet/src/defaultCompletionProvider.ts#L52

Thus, it suggests JSX (HTML) abbreviations, because the file is a JSX file.

If a user types in bgc and then manually runs the "Emmet: Expand Abbreviation" command, the CSS abbreviation is correctly expanded. This occurs because that command runs through a specific block that actually figures out what the local language in a region of code is: https://github.com/microsoft/vscode/blob/main/src/vs/workbench/contrib/emmet/browser/emmetActions.ts#L94

I think a possible fix for this change would be a VS Code API endpoint that relays which language is being used in a local region of code. This theoretical API would also improve the performance of Emmet on JSX and PHP files in general, because they both currently have the languageId issue above. The only alternatives I can currently think of would be the extension sending a command to the workbench-contrib side to fetch the language to use for suggestions. I'm not sure whether this language-identifying logic can all be moved to the Emmet side either.

@jrieken any thoughts on the possible fixes above? Emmet is already strange for having a fragment within the workbench, so I'm not sure what is the best way to proceed here.

CC @jasonwilliams

jrieken commented 3 years ago

//cc @alexdima

alexdima commented 3 years ago

The current language in a region of code is known to the UI thread (renderer process) of VS Code because it comes in via tokenization, and tokenization is executed on the renderer process. All vscode read APIs are synchronous, for example getting the current active text editor, reading its selection, reading its text, etc. If we would want to add tokens API to vscode, we would have to make that API synchronous.

But in order for the API to be synchronous, we would need to have the tokens readily available to give out to extensions. So we would either have to tokenize on the extension host and produce those tokens synchronously in the event loop of the extension host, or we would need to send those tokens eagerly from the renderer process to the extension host just in case there might be an extension that would want to read them. Neither option is good, tokenizing on the extension host would just freeze the event loop of the extension host for a long time, sending the tokens from the renderer process would cause significant CPU usage on the renderer to compute diffs and send them all the time to the extension host. To make matters worse, all of that might have been for nothing, as there might be no extensions interested in those tokens in the extension host or even if there would be an interested extension, maybe it would not need to read them all the time.

The other option would be to make the read API and diverge from the common practice of having read APIs sync. But let's analyze what the problem of that would be: it is possible that by the time the API returns, the buffer changed and the result might not reflect the state when the request was made, nor the state when the request was completed. The result might reflect an intermediate state, as the result is not synchronized in any way with the text document state. Here is a sketch of the problem:

// this lineText now captures the text of line 6 at `s1`
const lineText = document.lineAt(6); 

// by the time the request reaches the renderer thread and returns, it is possible that
// the text document was changed in the meantime. To make matters worse,
// the response to this request is not synchronized in any way with the text document
// change event, so the response might not even represent the state of the text document
// when the response completes.
// i.e. lineTokens now captures the tokens of line 6 at `s2`.
const lineTokens = await document.tokensAt(6); 

// but the document could be at `s3` at this point
const lineTextAgain = document.lineAt(6);

So IMHO we don't have any decent options. And that is why we don't have tokens API on the extension host.

aryanx455 commented 1 year ago

I was facing the same issue on m1 macbook air. Reinstalling visual studio code fixed this problem for me