-
-
Notifications
You must be signed in to change notification settings - Fork 33
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: upgrade volar #124
feat: upgrade volar #124
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just checked out the fork with this PR and linked it to a mono-repo using "prettier-plugin-organize-imports": "link:../../../OpenSource/prettier-plugin-organize-imports"
- used
pnpm install
- reloaded VS Code
- run prettier
It worked instantly, via CLI and via VSCode format 🤩
@simonhaenisch I didn't looked into the code, so feel free to make a code review before merging 🙂 |
I also didn't need to install devDependencies:
vite-plugin-dts 3.7.3
├── @vue/language-core 1.8.27
└─┬ vue-tsc 1.8.27
└── @vue/language-core 1.8.27
vue-tsc 1.8.27
└── @vue/language-core 1.8.27 |
@simonhaenisch what's the state? did you have time yet to make a review? |
bump on this one @simonhaenisch looking forward to dropping |
Sorry and thanks for the pings... kinda forgot about it, i'll try to get it done this week. |
Volar just released v2, shall we upgrade it? |
Yes please, let's get the breaking changes all done at once (: @so1ve do you have time to update your PR? |
Sure, I'll work on it |
@so1ve What's the progress? |
Sorry, but there are a few bugs that I had to work on, such as inaccurate text edit positions 😓 |
Can I help somehow? |
Checking back in on this one. @so1ve I see some code landed a week ago, though labeled WIP so maybe there is more to be done before this all gets considered? If there is help needed let me know, even if it is just testing. Just would need some steps. |
"optional": true | ||
} | ||
}, | ||
"devDependencies": { | ||
"@types/node": "20.8.10", | ||
"@volar/vue-language-plugin-pug": "1.0.9", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where did this go in the dev dependencies? didn't we have a test case that was using this plugin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If pug is not working without it, it should be changed to https://www.npmjs.com/package/@vue/language-plugin-pug cause @volar/vue-language-plugin-pug
is deprecated
It might be needed somehow so imports get not lost when using <template lang="pug">
getTypeScriptModule: () => ts, | ||
...ts.sys, | ||
configFileName: undefined, | ||
getCurrentDirectory: () => '/', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why /
?
getScriptFileNames: () => [filepath], | ||
getScriptSnapshot: (filename) => { | ||
if (filepath === filename) return ts.ScriptSnapshot.fromString(code); | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These all seem to be part of the return value of getTypeScriptLanguageServiceHost
already, why can't we pick them from there, like before?
Oh I figured it out it is caused by tabs 😅 For some reasons volar incorrectly calculates tab character's width |
@so1ve @simonhaenisch friendly bump what are the remaining steps to get this released? |
Hi there! |
Closing in favor of #129. |
resolve #98