Building with babel-eslint fails on windows with class-properties

[JeffBaumgardt]

Is this a bug report?

Yes

Did you try recovering your dependencies?

Yes

Which terms did you search for in User Guide?

Yes

Environment

Note npx create-react-app --info is not working on Windows 10

System: Windows 10
NPM: 6.0.0
Node: 10.10.0
React-scripts: 1.1.5

Steps to Reproduce

1. npx create-react-app my-app-name
2. npm install -D babel-eslint
3. Modify App.js to include state as a class-property

class App extends Component {
  state = {
    message: "Hello React App"
  };
  render() {
    const { message } = this.state;
    return (
      <div className="App">
        <header className="App-header">
          <img src={logo} className="App-logo" alt="logo" />
          <h1 className="App-title">{message}</h1>
        </header>
        <p className="App-intro">
          To get started, edit <code>src/App.js</code> and save to reload.
        </p>
      </div>
    );
  }
}

4. Run npm run build

Expected Behavior

I would expect the build to complete as expected.

Actual Behavior

The build fails with the error no-undef for the state that was defined in the component

PS C:\Node\windows-build-failure> npm run build

> windows-build-failure@0.1.0 build C:\Node\windows-build-failure
> react-scripts build

Creating an optimized production build...
Failed to compile.

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

Search for the keywords to learn more about each error.


npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! windows-build-failure@0.1.0 build: `react-scripts build`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the windows-build-failure@0.1.0 build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\jbaumgardt\AppData\Roaming\npm-cache\_logs\2018-10-01T19_01_21_162Z-debug.log

Reproducible Demo

As shown above, I can clone the repo and share if you wish, I only added a state class-property and installed babel-eslint.

In my real-world example I have a .eslintrc file for additional lint rules for VSCode to enforce more lint rules in development and pre-commit.

If I remove babel-eslint dependency the build will work.

[Timer]

@gaearon should we add babel-eslint to https://github.com/facebook/create-react-app/blob/master/packages/react-scripts/scripts/utils/verifyPackageTree.js ?

[gaearon]

yeah.

[gaearon]

also maybe babel-loader

[JeffBaumgardt]

As a strange additional side-note. This is happening on Windows only. A co-worker has a Mac with High-Sierra and it is working as expected for them.

Yet another co-worker on Windows 10 has the same error

[gaearon]

@JeffBaumgardt In general installing babel-eslint or similar packages at the top level is not supported and will break our setup in weird ways. I understand you want to run more lint rules, and it's something we should add support for, but currently you're going into an unsupported territory.

[gaearon]

Which lint rules do you want to add btw?

[JeffBaumgardt]

Ok. Thanks @gaearon

[JeffBaumgardt]

here's my .eslintrc

{
	"parser": "babel-eslint",
	"plugins": ["flowtype", "cypress", "import"],
	"extends": ["react-app", "prettier"],
	"env": {
		"cypress/globals": true
	},
	"rules": {
		"import/no-unresolved": [2, {"commonjs": true, "amd": true}],
		"import/named": 2,
		"import/namespace": 2,
		"import/default": 2,
		"import/export": 2,
		"import/no-cycle": 2,
		"import/no-self-import": 2,
		"import/order": 1,
		"import/newline-after-import": 1,
		"react-app/jsx-a11y/href-no-hash": "off"
	}
}

[JeffBaumgardt]

I would expect there would be others who would want more lint rules in their project. Is that something on the radar for CRA v2?

I know .eslintrc files are not picked up by the custom error overlay which is fine and I'm not expecting a way to extend that.

[JeffBaumgardt]

I take back my previous statement about it working just fine on Mac. I had my co-worker run a fresh CRA install and they got the same error about no-undef.

[bugzpodder]

Can you confirm the version of react-scripts and babel-eslint? Specifically are you using v1 of CRA as it appears in the first post?

[JeffBaumgardt]

I've deduced that by extending "react-app" the config is looking for babel-eslint, hence why I had it installed. I was extending react-app to disable href-no-hash, for reasons that are escaping me right now.

@bugzpodder
react-scripts: 1.1.5
babel-eslint: ^10.0.1

[JeffBaumgardt]

If I run eslint direct using the above config and installed plugins, flowtype, cypress, import, Configs for react-app and prettier. I do not get the errors as seen in the build.

[gaearon]

I would expect there would be others who would want more lint rules in their project. Is that something on the radar for CRA v2?

Probably.

[JeffBaumgardt]

Which lint rules do you want to add btw?

I'm using a number of rules and plugins that would require babel to be the parser, such as flowtype and prettier.

[JeffBaumgardt]

Note: This is not a problem on react-scripts@1.1.4

[gaearon]

Flowtype plugin is already on AFAIK in our default setup. Which rules did you want to add?

For Prettier, what is the purpose of linting it, as opposed to actually running Prettier?

[JeffBaumgardt]

To be honest some of the config was borrowed from ksc-scripts by Kent Dodds. I'm going to try running eslint local and not by kcd-scripts lint extension.

[JeffBaumgardt]

Really I only want to ensure cypress globals are ok with the env and the import rules. The rest I'm fiddling with now.

[bugzpodder]

I had cypress running on cra 2.0.1. Not sure why but I had to include a fetch polyfill but it worked after that.

[JeffBaumgardt]

From the looks of it I am extending react-app rules as I want them to also run in VSCode. For example no-unused-vars and the other react-app rules so that I get errors in editor.

[JeffBaumgardt]

Just to verify #5217 adds babel-eslint as a package warning if you have it installed?

I know you don't support eslint as a top level package but I presume there must be a lot of people running some form of linting in their editor, and many of those are interested in running rules from both within the CRA scope and other plugins as well.

[gaearon]

You can opt out of that message — the message says how to do it at the end. But it will hopefully prevent more people from shooting themselves in the foot with this.

[JeffBaumgardt]

@gaearon Ok thanks. Hopefully someday we'll get better IDE support for eslint configs other than "react-app". I have editor lints and lint tasks for pre-commit and CI scripts that will continually lint my code for more than just the basic react-app errors.

But for now this works

[gaearon]

I understand the use case. :-) Yes, we'd like to support that later.