Skip to content

Conversation

mtpc
Copy link
Contributor

@mtpc mtpc commented Jan 15, 2018

Add support for namespaced tsx as per jsx specification.
This is not a breaking change. All tests are green, I added an additional test for good measure.

The change is pretty simple, but it ends up being quite big due to the sheer amount of similar regexes:
A namespaced tag or attribute is defined by an identifier (namespace), followed by a colon, followed by another identifier (name), which gives us this addition:

///
(?: #start optional group
  ([_$a-zA-Z][-$\w.]*) #namespace
  (?<!\.|-) #prevent illegal end of namespace identifier
  : #colon
)? #end optional group
///

@msftclas
Copy link

msftclas commented Jan 15, 2018

CLA assistant check
All CLA requirements met.

^
source.tsx
>const valid2 = <namespaced:tag namespaced:boolean-attr>foo</namespaced:tag>;
^^^^^
Copy link
Member

Choose a reason for hiding this comment

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

:tag and :boolean-attr is getting scoped here instead of just tag and boolean-attr

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was intended, to highlight the colon the same color as the rest of the tag/attribute.
Would you prefer the colon to have its own scope? (punctuation.separator.namespace.tsx?)

Copy link
Member

Choose a reason for hiding this comment

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

yes please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be good

@@ -0,0 +1,9 @@
const valid1 = <standard-tag standard-attribute="foo" />;
Copy link
Member

Choose a reason for hiding this comment

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

Add comment // @onlyOwnGrammar - As this has jsx at the top to avoid scoping this for TypeScript without react support.

Copy link
Member

@sheetalkamat sheetalkamat left a comment

Choose a reason for hiding this comment

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

Please fix the test baseline

@mtpc
Copy link
Contributor Author

mtpc commented Mar 3, 2018

@sheetalkamat sheetalkamat merged commit e8fea97 into microsoft:master Mar 3, 2018
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