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

Fix interference with coc-tsserver #277

Closed
wants to merge 2 commits into from
Closed

Conversation

knpwrs
Copy link

@knpwrs knpwrs commented Jan 20, 2021

Summary

coc-tsserver has the following notice in its README:

Note: for React to work as expected, you need your JSX filetype to be javascript.jsx or javascriptreact and your TSX filetype to be typescript.jsx or typescript.tsx or typescriptreact.

With the setting that vim-prettier uses (typescript), tsx files become an error-riddled mess with coc-tsserver.

I've found that vim-prettier works better than coc-prettier, so I'd prefer to keep vim-prettier installed rather than use coc-prettier.

Test Plan

  1. Confirm prettier still works as expected (appears to work for me)

@mitermayer
Copy link
Member

Hi @knpwrs,

Thank you for this PR! On the PR https://github.com/prettier/vim-prettier/pull/272/files @johanventer has updated to remove the augroup in favour of the a single prettier group.

Could you please rebase and follow similar principles? After that this should be good to be merged.

How would this conflict with #279 ? @yzia2000 would this be a problem ?

@yzia2000
Copy link
Contributor

yzia2000 commented Feb 1, 2021

Hi @knpwrs,

Thank you for this PR! On the PR https://github.com/prettier/vim-prettier/pull/272/files @johanventer has updated to remove the augroup in favour of the a single prettier group.

Could you please rebase and follow similar principles? After that this should be good to be merged.

How would this conflict with #279 ? @yzia2000 would this be a problem ?

I used typescriptreact instead of typescript.tsx because the additive support for react files. This was inspired by this comment: vim/vim#4830 (comment). This also happens to be the way default runtime .vim files change ft.
However, I suspect that using typescriptreact is causing #280 although I am not entirely sure.
Give me a bit to investigate on which filetype to revert to by default, typescript.tsx or typescriptreact.

UPDATE

I think #280 is unrelated to the typescriptreact change. I would support using javascriptreact instead of javascript.jsx and typescriptreact instead of typescript.tsx

Copy link
Contributor

@yzia2000 yzia2000 left a comment

Choose a reason for hiding this comment

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

Please look into typescriptreact and javascriptreact.

@@ -1,3 +1,4 @@
augroup PrettierFileDetect
autocmd BufNewFile,BufReadPost *.js,*jsx setfiletype javascript
autocmd BufNewFile,BufReadPost *.js setfiletype javascript
autocmd BufNewFile,BufReadPost *.jsx setfiletype javascript.jsx
Copy link
Contributor

Choose a reason for hiding this comment

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

Any specific reason for choosing javascript.jsx. If not can you change it to javascriptreact

Copy link
Author

Choose a reason for hiding this comment

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

Only that the jsx / tsx types are more semantically correct. People can use jsx / tsx for more than just React.

@@ -1,3 +1,4 @@
augroup PrettierFileDetect
autocmd BufNewFile,BufReadPost *.ts,*.tsx setfiletype typescript
autocmd BufNewFile,BufReadPost *.ts setfiletype typescript
autocmd BufNewFile,BufReadPost *.tsx setfiletype typescript.tsx
Copy link
Contributor

Choose a reason for hiding this comment

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

Any specific reason for choosing typescript.tsx. If not can you change it to typescriptreact

@knpwrs
Copy link
Author

knpwrs commented Feb 1, 2021

It seems like #279 fixes the issue regardless. I'll close this PR for now. Thank you, all!

@knpwrs knpwrs closed this Feb 1, 2021
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.

3 participants