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

Don't parse class properties without initializers when classProperties plugin is disabled, and Flow is enabled #300

Merged
merged 1 commit into from
Jan 23, 2017
Merged

Don't parse class properties without initializers when classProperties plugin is disabled, and Flow is enabled #300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/parser/statement.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -771,8 +771,13 @@ pp.parseClassBody = function (node) {
};

pp.parseClassProperty = function (node) {
const noPluginMsg = "You can only use Class Properties when the 'classProperties' plugin is enabled.";
if (!node.typeAnnotation && !this.hasPlugin("classProperties")) {
Copy link
Member Author

@DrewML DrewML Jan 15, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not super happy about a flow-specific check leaking into the general parser, but I didn't see a straight-forward way to do this otherwise. Only other way I could see was to just have the flow extension for parseClassProperty invoke this with an optional second param of hadType, but even then, Flow-related stuff still leaks in.

Copy link
Member

Choose a reason for hiding this comment

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

This code will not stay forever anyway, as as soon as class-properties are final, we can remove all this.

Copy link
Member

Choose a reason for hiding this comment

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

We really need to do #178

Copy link
Member Author

Choose a reason for hiding this comment

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

This code will not stay forever anyway, as as soon as class-properties are final, we can remove all this.

Good point

this.raise(node.start, noPluginMsg);
}

if (this.match(tt.eq)) {
if (!this.hasPlugin("classProperties")) this.unexpected();
if (!this.hasPlugin("classProperties")) this.raise(this.state.start, noPluginMsg);
Copy link
Member Author

@DrewML DrewML Jan 15, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Pretty sure it's safe to provide a better message here. We should only get to this point if we're inside the top level of a class body and we've found either a = or linebreak after an identifier.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should note I'm using this.state.start rather than node.start here, so we're pointing out the error where the = is, rather than what could be the start of a type annotation in the scenario that the code being parsed is:

class Foo {
   bar: string = 'bizz';
}

Copy link
Member

Choose a reason for hiding this comment

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

Yes this was the same before, as this.unexpected() uses this.state.start

this.next();
node.value = this.parseMaybeAssign();
} else {
Expand Down
3 changes: 3 additions & 0 deletions test/fixtures/experimental/class-properties/with-initializer-and-type-no-plugin/actual.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class Foo {
bar: string = 'bizz';
}
4 changes: 4 additions & 0 deletions test/fixtures/experimental/class-properties/with-initializer-and-type-no-plugin/options.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"plugins": ["flow"],
"throws": "You can only use Class Properties when the 'classProperties' plugin is enabled. (2:14)"
}
3 changes: 3 additions & 0 deletions test/fixtures/experimental/class-properties/with-initializer-missing-plugin/actual.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class Foo {
bar = 'bizz';
}
3 changes: 3 additions & 0 deletions test/fixtures/experimental/class-properties/with-initializer-missing-plugin/options.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"throws": "You can only use Class Properties when the 'classProperties' plugin is enabled. (2:2)"
}
3 changes: 3 additions & 0 deletions test/fixtures/experimental/class-properties/without-initializer-missing-plugin/actual.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class Foo {
bar;
}
3 changes: 3 additions & 0 deletions test/fixtures/experimental/class-properties/without-initializer-missing-plugin/options.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"throws": "You can only use Class Properties when the 'classProperties' plugin is enabled. (2:2)"
}