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

Build error no-undef for flowtypes with class properties #2604

Closed
ubershmekel opened this issue Jun 23, 2017 · 16 comments
Closed

Build error no-undef for flowtypes with class properties #2604

ubershmekel opened this issue Jun 23, 2017 · 16 comments

Comments

@ubershmekel
Copy link

ubershmekel commented Jun 23, 2017

Can you reproduce the problem with npm 4.x?

Yes

Description

An error when running yarn run build occurs if I add a class property with a type to a react component. E.g.

interface MyType {
  what?: string;
}

// works fine
var status: MyType = {};
console.log(status);

class App extends Component {
  // no-undef build error
  state: MyType = {};
  render() {
...

Expected behavior

✨ Done in 11.24s.

Actual behavior

./src/App.js
  Line 16:  'state' is not defined  no-undef

Search for the keywords to learn more about each error.

error Command failed with exit code 1.

Note that yarn run flow raises no issue. So I'm guessing it's an eslint problem but there's no way to tell from the build output. Would be nice if the errors were tagged with their source (vs code does this in the problems panel).

Environment

Run these commands in the project folder and fill in their results:

  1. npm ls react-scripts (if you haven’t ejected): react-scripts@1.0.7
  2. node -v: v7.9.0
  3. npm -v: 4.6.1

Then, specify:

  1. Operating system: macOS 10.12.5 (16F73), uname -a: Darwin yuv 16.6.0 Darwin Kernel Version 16.6.0: Fri Apr 14 16:21:16 PDT 2017; root:xnu-3789.60.24~6/RELEASE_X86_64 x86_64
  2. Browser and version: This is a command line build issue

Reproducible Demo

https://github.com/ubershmekel/create-react-app-flowtypes

Related to

kitze#12
#567

@ubershmekel ubershmekel changed the title Eslint no-undef error for flowtypes class properties Build error no-undef for flowtypes with class properties Jun 23, 2017
@JeffreyATW
Copy link
Contributor

JeffreyATW commented Jun 26, 2017

I gave this a 👍 earlier, but I just downloaded your example and can't reproduce the error. I'm using Node 7.10.0, if it matters.

The weird part is, I can still reproduce similar problems on my own project. For one of my components:

export default class Something extends Component {
  props: {
    foo: string,
  };

results in:

'props' is not defined no-undef

Weirder still, and I'm not sure if it's related, I have another component where adding an arrow function property:

export default class SomethingElse extends Component {
  handleChange = (event) => {
    console.log(event);
  };

results in:

'handleChange' is not defined no-undef

But weirder still, changing the method's name to test works fine:

export default class SomethingElse extends Component {
  test = (event) => {
    console.log(event);
  };

I can't pin down an exact pattern, but I feel like this is all related. Thoughts?

@ubershmekel
Copy link
Author

I've also experienced the issue with arrow functions. Though I think running flow by itself found other issues that later alleviated the arrow-no-undef issue.

@gaearon gaearon added this to the 1.0.x milestone Jun 26, 2017
@gaearon
Copy link
Contributor

gaearon commented Jun 26, 2017

I can't reproduce this with https://github.com/ubershmekel/create-react-app-flowtypes on Node 7.9.0.

I tried both with npm 4.6.1 and with Yarn 0.24.3.

We'll need more details about how to reproduce this.

@ubershmekel
Copy link
Author

On a fresh clone I wasn't able to reproduce the issue either. I folder-diffed vs the reproducing check out and saw no differences. Then I moved my bugged folder elsewhere and it stopped bugging out.

I narrowed it down to a node_modules two folder levels above my original checkout. I've updated the repository https://github.com/ubershmekel/create-react-app-flowtypes so now if you want to see the error you need to:

# at the root
yarn

cd folder2/folder1/

yarn

yarn run build

./src/App.js
Line 16: 'state' is not defined no-undef

I guess for me the surprise is that the build isn't folder-local.

@kennethtruong
Copy link

kennethtruong commented Jun 26, 2017

I think this issue might be from eslint 4.0 since when I upgraded from 3.19 to 4.0 a while ago I got this issue. So I had to stick with eslint 3.19. Though I'm not entirely sure why upgrading eslint would cause this

@gaearon
Copy link
Contributor

gaearon commented Jun 26, 2017

Oh, I see. Thank you so much for the repro case, it will be very useful.

The incompatibility with ESLint 4.0 is a known issue. We haven’t dug into it yet because nothing is forcing us to upgrade.

I’m surprised why react-scripts doesn’t “find” the right eslint though. Have you tried not using yarn? It might be this Yarn bug: yarnpkg/yarn#3535. Although I’d expect it to only break eslint bin command rather than npm start.

@JeffreyATW
Copy link
Contributor

In my case, I ran yarn add eslint --dev because I wanted to use the CLI. Doing so installed eslint@4.0.0 into my own package.json. That's likely the issue - for now I'll make sure to install the same version that react-scripts uses.

@gaearon
Copy link
Contributor

gaearon commented Jun 26, 2017

Aah okay that explains it. We probably need to do something to resolve eslint from react-scripts rather than top level.

@gaearon
Copy link
Contributor

gaearon commented Jun 26, 2017

I think this will be fixed once webpack-contrib/eslint-loader#181 is unreverted and we can use that option.

@JeffreyATW
Copy link
Contributor

In the meantime, downgrading my own version to 3.19 cleared everything up.

@gaearon
Copy link
Contributor

gaearon commented Jun 27, 2017

I'll close this as it's not a bug, but caused by using incompatible version of ESLint.
We will update to ESLint 4 eventually, and as part of that work will also solve hoisting issues.

This is tracked in #2644 now.

@chrisdrackett
Copy link
Contributor

@gaearon I just updated to the newest CRA and this seems to have popped up again for me. I'm wondering now that CRA uses eslint 4 is this is a bug now? Is anyone else running into this?

@zzuhan
Copy link

zzuhan commented Dec 20, 2017

In v4.13.1, stil pop

@dancherb
Copy link

+1, still getting this

@vladimirvorobiev
Copy link

i my case conflict with react-script
removing react-script solved my problem

@facebook facebook locked and limited conversation to collaborators May 17, 2018
@gaearon
Copy link
Contributor

gaearon commented May 17, 2018

Commenting on a closed issue is futile. This is what the notification list looks like for me (and many other maintainers):

screen shot 2018-05-17 at 6 38 06 pm

This is 2024 unread issues. And I only watch this and a few other projects. Do you expect anyone to be able to read through this?

If you're commenting in a closed issue, the only place where your comment appears is in the notifications. If I fail to read the notifications I won't see it. If you just add "thumbs up" to a comment that's a guarantee I'll never even have any idea you were here. :-)

This is why if you experience a problem after an issue was closed you should file a new issue. With a reproducing case. Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants