Don't let ':' terminate JSX classname#143
Conversation
Flow uses syntax like `<T: Object>` to say "generic in type T, provided that it's an object." JSX technically allows ':' in a "JSXNamespacedName" (see [here][1]). However, this is rarely ever used, and even still, a ':' can't be used to terminate a namespaced name. This commit explicitly checks for the ':' at the end. [1]: https://facebook.github.io/jsx/
|
This seems reasonable to me, but my vim-regexp is rusty. Would you mind describing the regex you added in words? Think of this as a very lazy form of code review... |
| syn region jsxRegion | ||
| \ contains=@Spell,@XMLSyntax,jsxRegion,jsxChild,jsBlock,javascriptBlock | ||
| \ start=+\%(<\|\w\)\@<!<\z([a-zA-Z][a-zA-Z0-9:\-.]*\)+ | ||
| \ start=+\%(<\|\w\)\@<!<\z([a-zA-Z][a-zA-Z0-9:\-.]*\>:\@!\)+ |
There was a problem hiding this comment.
@mxw This regex makes it so that
- we find the word boundary of the XML-ish thing with
\> - we check that
:does not occur at the current position::\@!
:help \@!
\@! Matches with zero width if the preceding atom does NOT match at the
current position. /zero-width
So for example:
<div>is fine<div className="foo">is fine<T: Object>is not fine, because there's a:after the first word
We don't to treat this last example as JSX because it conflicts with the syntax for "bounded polymorphism" in Flow:
https://flow.org/en/docs/types/generics/#toc-adding-types-to-generics
"bounded polymorphism" is just a fancy way to say "any type T, provided that T is a subtype of Object (in the example here, at least).
| syn region jsxRegion | ||
| \ contains=@Spell,@XMLSyntax,jsxRegion,jsxChild,jsBlock,javascriptBlock | ||
| \ start=+\%(<\|\w\)\@<!<\z([a-zA-Z][a-zA-Z0-9:\-.]*\>:\@!\)+ | ||
| \ start=+\%(<\|\w\)\@<!<\z([a-zA-Z][a-zA-Z0-9:\-.]*\>[:,]\@!\)\([^>]*>(\)\@!+ |
There was a problem hiding this comment.
@mxw This is a change That makes it so that you can add flow generic type annotations to functions. Here's the same regex, but with some whitespace for readability:
\( [^>]* > ( \) \@!
1 2 3 4 5
- Start a group, so that we can check it all at zero width with (5)
- match any number of characters that aren't a literal
>... - ... up until we find the first
>- This basically matches all the way to the end of the starting JSX tag
- Look one character after the JSX tag, and see if it's a literal
( - Make sure that this whole grouped pattern doesn't match, with zero width
Basically:
const foo = <TS>(x: T, y: S) => {x, y};
^
This is a flow generic type annotation because there's a `(`
<div>Hello, world</div>
^
This is a JSX tag, because there's no `(`
So this rules in favor of Flow over JSX in cases like this:
<div>(foo)</div>
I'm fine if you don't want to have this change this PR and just do the first commit, but for me the former happens way more often than the latter, so it's a tradeoff I'm fine with.
There was a problem hiding this comment.
That fail case seems pretty bizarre, so I'm fine with this.
|
This is a great code review—thanks! Say hi to ptarjan for me. |
2 participants
Flow uses syntax like
<T: Object>to say "generic in type T, providedthat it's an object."
JSX technically allows ':' in a "JSXNamespacedName" (see here).
However, this is rarely ever used, and even still, a ':' can't be used
to terminate a namespaced name. This commit explicitly checks for the
':' at the end.
Without this change, vim-jsx tries to treat
<T: Object>as the openingtag of a JSX block, meaning that everything after it gets highlighted
as if it were inside an XML tag.