Skip to content
This repository was archived by the owner on May 19, 2018. It is now read-only.

Conversation

@vkurchatkin
Copy link
Contributor

@vkurchatkin vkurchatkin commented Dec 22, 2016

Q A
Bug fix? yes
Breaking change? no
New feature? no
Deprecations? no
Spec compliancy? yes
Tests added/pass? yes
Fixed tickets #264
License MIT

Previously inType flag was set when parsing property names. Among other things it was used to prevent jsx tokenization. This patch adds specialized inPropertyName flag and prevent jsx tokenization in jsx plugin.

@xtuc xtuc self-requested a review December 24, 2016 09:53
@@ -0,0 +1,3 @@
const map = {
[age <= 17] : 'Too young'
Copy link
Member

@xtuc xtuc Dec 24, 2016

Choose a reason for hiding this comment

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

Do we need the original use case? It's not important in the expected AST.

I would suggest something like that:

[1 <= 0]: null

this.inFunction =
this.inGenerator =
this.inAsync =
this.inPropertyName =
Copy link
Member

Choose a reason for hiding this comment

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

Can you please also add the flow type, like the other fields?

Copy link
Member

Choose a reason for hiding this comment

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

I did it in 1d5932c

Copy link
Member

@danez danez left a comment

Choose a reason for hiding this comment

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

I haven't tested, but I think that this won't work anymore now:

var x = {
  [<div></div>]: 0
}

This is not really useful, but should be still valid.

Although just noticed it is not working in 6.13 either.


if (this.state.inPropertyName) {
return inner.call(this, code);
}
Copy link
Member

Choose a reason for hiding this comment

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

We could move this before the curContext() call.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants