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

Follow user's config while resolving autoImport path, related #1177 #1753

Merged
merged 2 commits into from
Aug 3, 2020

Conversation

hikerpig
Copy link
Contributor

@hikerpig hikerpig commented Mar 4, 2020

Same as #1177 , I prefer getting absolute path with the auto-import feature. So I dive into the codebase and found these related code.

My modification goes a little further, I think it is best to follow user's preferences on js/ts language features.

Pitfall

If the js/ts preferences are changed after vetur is loaded, this config seemed to fail to update ? I think it's a more common problem, but I am not an expert on this, so some help is needed.

@credred
Copy link

credred commented Mar 31, 2020

I have this problem after adding "typescript.preferences.importModuleSpecifier": "non-relative" on settings.json file.

@yoyo930021
Copy link
Member

The link is code about VSCode implementation.
https://github.com/microsoft/vscode/blob/508f43166bd8551a26d3b53b2e04008d0b9c357c/extensions/typescript-language-features/src/features/fileConfigurationManager.ts#L177

There are other places to use user preferences,
and then we need to delete the conflict Vetur config.
#1352 (comment)

@hikerpig
Copy link
Contributor Author

hikerpig commented Jun 5, 2020

@yoyo930021 Nice hints, thanks, will definitely take a look.

And about forking this project, I've already done it and released a personal version that has your PR merged in. Thumbs up for your work 💯 .

@yoyo930021
Copy link
Member

@yoyo930021 Nice hints, thanks, will definitely take a look.

And about forking this project, I've already done it and released a personal version that has your PR merged in. Thumbs up for your work 💯 .

I try to fork project and find some test problem. #1323

@hikerpig
Copy link
Contributor Author

hikerpig commented Jun 5, 2020

@yoyo930021 Nice hints, thanks, will definitely take a look.
And about forking this project, I've already done it and released a personal version that has your PR merged in. Thumbs up for your work 💯 .

I try to fork project and find some test problem. #1323

Yes, mostly in interpolation LSP tests. But I have used the personal version daily for months and it worked fine.

After the release I see it would be better to fix those test failures. Still looking into it, haven't found a clue yet (since recently I'm a little busy with new work 😢 ).

@octref
Copy link
Member

octref commented Aug 3, 2020

@yoyo930021 I opened #2109. We should gradually make the js/ts integration to follow all js/ts settings.

@hikerpig Thanks for your fix 👍

@octref octref merged commit 48088a7 into vuejs:master Aug 3, 2020
octref added a commit that referenced this pull request Aug 3, 2020
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.

4 participants