Skip to content

Sync Volar 1.9#7

Merged
mizdra merged 4 commits into
mizdra:mainfrom
johnsoncodehk:main
Jul 27, 2023
Merged

Sync Volar 1.9#7
mizdra merged 4 commits into
mizdra:mainfrom
johnsoncodehk:main

Conversation

@johnsoncodehk

Copy link
Copy Markdown
Contributor

Hi, I tried to make the required changes for the TS Plugin integration updated to Volar 1.9, can you see if it matches your original needs? Thanks.

@mizdra

mizdra commented Jul 23, 2023

Copy link
Copy Markdown
Owner

Thank you! I was going to update Volar.js too, but didn't have time to do it. So sending me the PR helps me a lot.

I will check the PR now.

Comment thread src/index.ts
}
return target[property];
},
});

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am glad to remove many of the codes :)

@mizdra

mizdra commented Jul 23, 2023

Copy link
Copy Markdown
Owner

I found a problem with newly added assets not being suggested due to forgetting to implement getScriptFileNames. I fixed the implementation.: db48182

Other than that, there seems to be no problem. It matches my needs well enough. Thanks! @johnsoncodehk

@mizdra

mizdra commented Jul 23, 2023

Copy link
Copy Markdown
Owner

I will merge this PR if it is ok with you.

@johnsoncodehk

Copy link
Copy Markdown
Contributor Author

Hi, what problems can it cause if we don't implement getScriptFileNames? Can you explain the steps to reproduce the problem?
I'm asking this question because getExternalFiles is already implemented, so getScriptFileNames is theoretically not needed.

@mizdra

mizdra commented Jul 25, 2023

Copy link
Copy Markdown
Owner

If the patch db48182 is not included, the added assets will not be suggested.

How to reproduce:

$ git checkout 6c19f97
$ npm run build

$ # Open example project with VS Code
$ cd examples/nextjs
$ code .
2023-07-26.0.23.26.mov

If the patch is included, the added assets are also suggested.

2023-07-26.0.32.19.mov

@johnsoncodehk

johnsoncodehk commented Jul 25, 2023

Copy link
Copy Markdown
Contributor Author

Thanks! I'll investigate how to fix it from the framework, before that I think we can merge this PR, as the change is probably just an implementation improvement rather than a bug fix.

@johnsoncodehk
johnsoncodehk marked this pull request as ready for review July 25, 2023 15:42
@mizdra

mizdra commented Jul 27, 2023

Copy link
Copy Markdown
Owner

It sounds good that Volar.js would take care of it 👍.

I'll merge this PR for now. Thanks for the contribution!

@mizdra
mizdra merged commit 5b87351 into mizdra:main Jul 27, 2023
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.

2 participants