Skip to content
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

es6/7 class parsing failure - property declarations without semicolons - [bounty][test repo available] #1908

Closed
rosskevin opened this issue Jun 8, 2016 · 8 comments

Comments

@rosskevin
Copy link

rosskevin commented Jun 8, 2016

version: 0.26.0

bountrsource

Bug description

This shows an ES2015/ES6/ES7 parsing bug - it will fail if there isn't a semicolon or static method after property declarations and before the first member method.

I first found in the test case that a constructor without a static method above it causes linting to fail on the first method with ^^^^^^^^^^^ Unexpected identifier. After suggestion, I tried adding a semicolon which proves the parsing error.

flowconfig

Same results with and without the following:

esproposal.class_static_fields=enable
esproposal.class_instance_fields=enable

Test cases

https://github.com/rosskevin/react-flow-classes

Works (static method above)

// @flow
import React from 'react'

type DefaultProps = { x: 1 }
type Props = { x: number }
type State = { y: number }

class Foo extends React.Component {
  props:Props
  state:State
  static defaultProps:DefaultProps

  static bar ():void {}

  constructor (props) {
    super(props)
  }

  render () {
    return <div>Hello World</div>
  }
}

Fails (no static method above)

// @flow
import React from 'react'

type DefaultProps = { x: 1 }
type Props = { x: number }
type State = { y: number }

class Foo extends React.Component {
  props:Props
  state:State
  static defaultProps:DefaultProps

  // fails without this static method above constructor
  //static bar ():void {}

  constructor (props) {
    super(props)
  }

  render () {
    return <div>Hello World</div>
  }
}

Works (a single semicolon)

// @flow
import React from 'react'

type DefaultProps = { x: 1 }
type Props = { x: number }
type State = { y: number }

class Foo extends React.Component {
  props:Props
  state:State
  static defaultProps:DefaultProps;

  // fails without this static method or semicolon above first constructor/member method
  //static bar ():void {}

  constructor (props) {
    super(props)
  }

  render () {
    return <div>Hello World</div>
  }
}

I proved the same in the sample repo with parameterized classes. May be related to #1171
cc @STRML

Expectation

All current failing classes in the test repo should succeed. I should not need to use any semicolons, and I don't need static methods above my constructor or member method.

@rosskevin rosskevin changed the title es6 React Component class confusion with parameterized classes [test repo available] es6 React Component class failure with parameterized classes [test repo available] Jun 8, 2016
@rosskevin rosskevin changed the title es6 React Component class failure with parameterized classes [test repo available] es6 React Component class failure - [hello world test repo available] Jun 8, 2016
@rosskevin rosskevin changed the title es6 React Component class failure - [hello world test repo available] es6 React Component class failure with constructor - [hello world test repo available] Jun 8, 2016
@rosskevin rosskevin changed the title es6 React Component class failure with constructor - [hello world test repo available] es6 React Component class failure on first instance method - [hello world test repo available] Jun 8, 2016
@STRML
Copy link
Contributor

STRML commented Jun 8, 2016

I think there may still be some odd parsing rules with semicolons, try
that?
On Jun 8, 2016 5:46 PM, "Kevin Ross" notifications@github.com wrote:

version: 0.26.0

I'm at a loss, where I can get one thing to work, I cannot get a
combination of Props and State to work. I found in the test case that a
constructor without a static method above it causes linting to fail on
the first method with ^^^^^^^^^^^ Unexpected identifier

https://github.com/rosskevin/react-flow-classes
Works

// @flowimport React from 'react'

type DefaultProps = { x: 1 }
type Props = { x: number }
type State = { y: number }
class Foo extends React.Component {
props:Props
state:State
static defaultProps:DefaultProps

static bar ():void {}

constructor (props) {
super(props)
}

render () {
return

Hello World

}
}

Fails

// @flowimport React from 'react'

type DefaultProps = { x: 1 }
type Props = { x: number }
type State = { y: number }
class Foo extends React.Component {
props:Props
state:State
static defaultProps:DefaultProps

// fails without this static method above constructor
//static bar ():void {}

constructor (props) {
super(props)
}

render () {
return

Hello World

}
}

I proved the same with parameterized classes. May be related to #1171
#1171
cc @STRML https://github.com/STRML


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1908, or mute the thread
https://github.com/notifications/unsubscribe/ABJFP6RZmGKyKVdvEVKn3Sfay63tIXI1ks5qJ0ZRgaJpZM4IxXs8
.

@rosskevin
Copy link
Author

Indeed a single semicolon and it works (repo updated):
rosskevin/react-flow-classes@3c56fd9

@rosskevin rosskevin changed the title es6 React Component class failure on first instance method - [hello world test repo available] es6 class parsing failure - without semicolons - [test repo available] Jun 9, 2016
@rosskevin rosskevin changed the title es6 class parsing failure - without semicolons - [test repo available] es6/7 class parsing failure - property declarations without semicolons - [test repo available] Jun 9, 2016
@rosskevin rosskevin changed the title es6/7 class parsing failure - property declarations without semicolons - [test repo available] es6/7 class parsing failure - property declarations without semicolons - [bounty][test repo available] Jun 9, 2016
@mroch mroch self-assigned this Jun 9, 2016
@mroch
Copy link
Contributor

mroch commented Jun 9, 2016

fix incoming

@ghost ghost closed this as completed in fc744c2 Jun 9, 2016
@mroch
Copy link
Contributor

mroch commented Jun 9, 2016

@rosskevin thanks for offering the bounty, but I can't accept it :) it's generous and a cool idea, so maybe move it to your next favorite bug instead!

@mroch mroch added the parsing label Jun 9, 2016
@rosskevin
Copy link
Author

Thanks for the quick fix! Will definitely move it to the next one.

@rosskevin
Copy link
Author

@mroch - please reopen, my sample cases still fail with 0.27.0.

src/es6/Fails.js:16
 16:   constructor (props) {
       ^^^^^^^^^^^ Unexpected identifier

src/es6/FooFails.js:14
 14:   constructor (props) {
       ^^^^^^^^^^^ Unexpected identifier

src/es6/ParameterizedFails.js:16
 16:   constructor (props) {
       ^^^^^^^^^^^ Unexpected identifier

@mroch
Copy link
Contributor

mroch commented Jun 13, 2016

@rosskevin this didn't make it into the 0.27 branch. should be fixed in master, though.

@rosskevin
Copy link
Author

ah, thank you.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants