Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(vue): update Vue grammar #5

Merged
merged 5 commits into from
Nov 30, 2023
Merged

feat(vue): update Vue grammar #5

merged 5 commits into from
Nov 30, 2023

Conversation

fvsch
Copy link
Contributor

@fvsch fvsch commented Nov 3, 2023

We've had some reports by the Nuxt team and a few users that our Vue syntax highlighting fails at times. We have not been able to replicate the issue, but for the Nuxt team report at least it seemed linked to a couple things:

  1. The vue.tmLanguage file (XML) failed to parse correctly.
  2. That file was served by our service worker; when going straight to the original server, the same file parsed correctly.

It's likely that this might be a glitch in how that file was served sometime around August and then cached by the user's service worker, and not necessarily a syntax issue in the file itself. So any update that increments the @blitz/textmate version that we use in URLs when fetching grammars would invalide the service worker cache and probably resolve this issue.

But I’m also a bit wary of using the XML PLIST format, because it seems a bit more prone to parse errors by our TextMate grammar implementation than JSON.

And the vue.tmLanguage file we're using currently is 2 years old, so I suspect it might be missing support for a few Vue template syntax features by now.

For all those reasons, I propose updating our Vue language syntax to a recent vue.tmLanguage.json sourced from https://github.com/vuejs/language-tools/blob/master/extensions/vscode/syntaxes/vue.tmLanguage.json

@fvsch fvsch requested review from SamVerschueren and d3lm November 3, 2023 12:11
@fvsch fvsch force-pushed the fvsch/update-vue-grammar branch from fccb31f to 11a0bae Compare November 3, 2023 16:20
@fvsch fvsch marked this pull request as draft November 3, 2023 17:38
@fvsch
Copy link
Contributor Author

fvsch commented Nov 3, 2023

I’m seeing an increase in requests to other grammars from this change.

Without it, setting up the 'vue' language in our setup triggers 23 requests for language configurations and grammars (which is already a lot). With this change, it looks like it triggers something like 35 requests, including things like Svelte grammars (?!?). I’ll have to investigate.

@fvsch fvsch force-pushed the fvsch/update-vue-grammar branch from 11a0bae to a2a7c9a Compare November 8, 2023 16:19
@fvsch
Copy link
Contributor Author

fvsch commented Nov 16, 2023

I’ve seen this issue happen locally as well, with the grammar file not being cached by a service worker. It looks like parsing the XML grammar can be a bit flaky.

@fvsch fvsch marked this pull request as ready for review November 30, 2023 18:15
@fvsch fvsch force-pushed the fvsch/update-vue-grammar branch from ce7c93b to 873e022 Compare November 30, 2023 18:23
@fvsch
Copy link
Contributor Author

fvsch commented Nov 30, 2023

The issue with the failing XML grammar is reported as a semi-frequent issue by VueSchools. We should address it even if we don't have a perfect solution, perf-wise.

I did a bunch of tests on why the updated Vue grammar would trigger more HTTP requests, and it boils down to its support of embedded languages, including <template lang="md"> for Markdown and <template lang="pug"> for Pug.

The extra requests I’m seeing is mostly from the reference to the text.html.markdown scope, which triggers downloading files for the markdown language. And the markdown language is currently configured in our grammar.json as supporting a huge list of embedded languages, which triggers downloading a bunch of grammars.

I also confirmed that if the project has any .md file, we're already downloading all those grammars. Since many projects have a README.md file, it looks like this is a wider problem and we shouldn't block this specific update.

Still, I took the liberty to remove a few patterns for embedded languages from Vue.tmLanguage.json:

  • lang="md"
  • lang="pug"
  • lang="stylus"

The popularity of these patterns in the wild seemed quite low, and our previous grammar didn't support most of them anyway, so we can try not matching them to limit requests in projects which don't have .md files.

@fvsch fvsch merged commit d30844c into main Nov 30, 2023
2 checks passed
@fvsch fvsch deleted the fvsch/update-vue-grammar branch November 30, 2023 18:26
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.

1 participant