Incorrect syntax highlighting with yats.vim

[sheerun]

.vimrc used:

Plug 'HerringtonDarkholme/yats.vim'
Plug 'maxmellon/vim-jsx-pretty'

When I have file with following contents:

const head = <T>(arr: T[]): T => arr[0]

And then press enter, next line is wrongly indented. Could you please fix this?

Reference: sheerun/vim-polyglot#428

[yuezk]

Thanks for your feedback, will take a look in my spare time.

[yuezk]

@sheerun I can reproduce this issue without vim-jsx-pretty. So this should be an issue of yats.vim.

[yuezk]

@sheerun Closing this issue since it's not an issue of this plugin.

[sQVe]

@yuezk, @sheerun: The indenting is an issue with yats.vim. The issue (sheerun/vim-polyglot#428) referred to highlighting. As soon as I add vim-jsx-pretty the following breaks typescript (both yats.vim and typescript-vim) highlighting for all code below it:

const head = <T>(arr: T[]): T => arr[0]

I suggest reopening and renaming this issue or alternatively creating a new issue that tracks the problematic highlighting.

[sheerun]

@sQVe If that's issue with yats.vim maybe you'll open an issue there?

[yuezk]

@sQVe OK, I see. I'll reopen this issue.

[davidroeca]

I think the highlighting of generic functions is an issue here around how a JSX group is defined. I've encountered the same issue in https://github.com/leafgarland/typescript-vim and also in https://github.com/pangloss/vim-javascript with flow.

[davidroeca]

Created a simple repo that reproduces this issue both for flow and typescript https://github.com/davidroeca/vim-jsx-pretty-issue-86-repro

[davidroeca]

My best guess at a fix would be an edit in the following lines:

vim-jsx-pretty/after/syntax/jsx_pretty.vim

Lines 38 to 41 in c3dc920

	syntax region jsxRegion
	\ start=+\(\(\_[([,?:=+\-*/<>{}]\|&&\|||\|=>\|\<return\|\<default\|\<await\|\<yield\)\_s*\)\@<=<\_s*\(>\|\z(\(script\)\@!\<[_\$A-Za-z][-:_\.\$0-9A-Za-z]*\>\)\(\_s*\([-+*)\]}&|?]\|/\([/*]\|\_s*>\)\@!\)\)\@!\)+
	\ end=++
	\ contains=jsxElement

But it's tough for me to be sure; the regex is giving me a bit of a headache 😅

[yuezk]

@davidroeca Thanks for your investigation. This regex is indeed complex because we have to use it to detect the jsx tag start boundaries and the boundaries vary greatly.

Currently, it cannot be simplified because this plugin has no assumption on which plugin (vim-javascript or typescript-vim) is used.

I will try to fix this issue in a few days.

[yuezk]

@davidroeca @sQVe @sheerun

I think this issue is hard to fix and I'm trying to explain it.

Vim syntax highlighting has its limitation because it uses the regex to match the syntax and it's a static match, for most of the cases, this mechanism works well since most of the language syntax is unambiguous.

But the test case in this issue is ambiguous.

const head = <T>(arr: T[]): T => arr[0]

<T> can be the start element of a jsx element, it can also be the generic type definition, it's based on the syntax after it. But when regex syntax matches, it can not parse the syntax after it, so it treats the <T> as the start element of the jsx element.

A possible solution is that we make an assumption that if <T> is followed by a (, we treat it as the generic type. But this may break the highlight of const head = <T>(...)</T>, which is a valid jsx element.

The workaround is to use the function expression:

const head = function <T>(arr: T[]): T { return arr[0]; }

or use the trailing comma after the type

const head = <T,>(arr: T[]): T => arr[0]

Reference: microsoft/TypeScript#15713

[sQVe]

@yuezk I understand. Thank you for diving deep into this issue 🙏

I mostly write JavaScript / TypeScript in a functional manner so the likelyhood of me writing functions like const head = <T>(arr: T[]): T => arr[0] is quite a bit larger than using a <T>(...)</T> JSX block. The JSX counterpart is also easier to repair with newlines etc.

It would be interesting to know how common the <T>(...)</T> case really is. I'm unsure on what that specific use case solves.

I vote to fixing the generic type case even though it might introduce a very specific issue with JSX.

[yuezk]

@sQVe You can update this plugin and use the workaround by adding a trailing comma after T.

const head = <T,>(arr: T[]): T => arr[0]

[sheerun]

I also agree that const head = <T>(arr: T[]): T => arr[0] is probably more popular than <T>(...)</T>, so probably checking if <T> is followed by a ( is fair indicator (or checking if </T> is present in the same line).

Also good indicator is that it's capital case e.g. in my codebase there's <span>({photos.length})</span> but it cannot be generic type because it's lower case.

[sQVe]

Also good indicator is that it's capital case e.g. in my codebase there's <span>({photos.length})</span> but it cannot be generic type because it's lower case.

Using a capital letter for a generic type is considered good practice but it is not required. The following snippet is valid TypeScript:

const head = <t>(arr: t[]): t => arr[0]
// or even
const head = <span>(arr: span[]): span => arr[0]

[sheerun]

Then it's at least good clue :)

[davidroeca]

@yuezk thanks for taking the time here!

I’m good with either approach as long as what correctly highlights is compatible with what linters and prettier expect. I know prettier removes unnecessary parens. I’d assume most standard linter configs would complain about lower-case type variables.

[yuezk]

Hi, @sheerun @sQVe @davidroeca @tsuyoshicho This issue should have been fixed in #99. Update this plugin and take a look, Thanks.