Skip to content

Add template literals to object region #1158

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

Conversation

NoahTheDuke
Copy link

Adds jsTemplateString to the jsObject region.

Fixes #1138

@amadeus
Copy link
Collaborator

amadeus commented Feb 5, 2019

This PR isn't going to fix the problem you are having, the issues from that PR are based on invalid syntax.

The way we syntax highlight within an object is very specific, and template strings are actually supported:

const example = {
  test: `${2 + 3}`
};

I don't want to accept this PR because it will actually create broken highlighting.

For a deep dive on how the syntax highlighting works:

We create a jObject region. Within that region we initially only allow very specific things - basically things that define keys for an object, like strings, keywords, spreads, async stuff, function names. Then once you type those things, we create a syntax scope to enable more things, a bit like a poor mans version of a tokenizer. This is done through the nextgroup API, which you will see littered throughout the syntax file.

We can use jsObjectKey as an example here. If you look at the definition for jsObjectKey - you will see that it then contains a nextGroup for a jsObjectValue. This means, that by typing a valid jsObjectKey, you can then follow it up with a jsObjectValue. If you then look at the definition for jsObjectValue, you will see it contains a syntax group called @jsExpression. If you look at this, you will see that it then contains a tooon of syntax groups, including jsTemplateString.

The way this plugin works - it fully supports jsTemplateString in the correct way (as a value of an object key). Introducing this change would allow invalid syntax. You cannot use template strings as keys in an object unless you wrap it in [ and ]

Invalid:

const Invalid = {
  `test${2}`: true
}
const Valid = {
  [`test${2}`]: true
}

The bug in that particular issue is actually do a potential plugin conflict or improper settings

@amadeus amadeus closed this Feb 5, 2019
@NoahTheDuke NoahTheDuke deleted the nb/fix-template-literal-inside-objects branch February 5, 2019 22:11
@NoahTheDuke
Copy link
Author

Thanks for the long explanation! I appreciate you spending the time to tell me all that. Turns out, I did have my configuration messed up and in fixing that the highlighting corrected itself.

@TrySound
Copy link

I have this problem with templates. I have enabled flow and vim-jsx extension
image

@amadeus
Copy link
Collaborator

amadeus commented Feb 19, 2019

@TrySound I believe this is a bug with using the shorthand fragment syntax <>.

As you can see in my example here:

example

example is highlighted differently depending on whether I have the text Fragment or not between the <>.

Basically this is a bug with vim-jsx I believe.

@amadeus
Copy link
Collaborator

amadeus commented Feb 19, 2019

I just submitted a PR to the the mxw/jsx repo to fix this since the issue has been around for a while.

mxw/vim-jsx#188

@TrySound
Copy link

Awesome thanks!

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