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

Don't set inType flag when parsing property names #266

Merged
merged 3 commits into from
Jan 5, 2017

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

@@ -16,6 +16,7 @@ export default class State {
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.

@@ -416,6 +416,10 @@ export default function(instance) {
return function(code) {
let context = this.curContext();

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.

None yet

4 participants