Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

A created project comes with linting issues #608

Closed
armenzg opened this issue Dec 12, 2017 · 16 comments
Closed

A created project comes with linting issues #608

armenzg opened this issue Dec 12, 2017 · 16 comments
Labels

Comments

@armenzg
Copy link

armenzg commented Dec 12, 2017

I chose to create a React project with no testing and with the airbnb configuration (created with npx @neutrinojs/create-project ci-metrics.
After that I would run yarn start and I get linting errors.

armenzg@armenzg-mbp ci-metrics$ yarn start
yarn run v1.3.2
$ neutrino start
✔ Development server running on: http://localhost:5000
✔ Build completed

ERROR in ./src/index.jsx
error: Expected a newline after '(' (function-paren-newline) at src/index.jsx:6:26:
  4 |
  5 | const root = document.getElementById('root');
> 6 | const load = () => render((
    |                          ^
  7 |   <AppContainer>
  8 |     <App />
  9 |   </AppContainer>


error: Expected a newline before ')' (function-paren-newline) at src/index.jsx:10:8:
   8 |     <App />
   9 |   </AppContainer>
> 10 | ), root);
     |        ^
  11 |
  12 | // This is needed for Hot Module Replacement
  13 | if (module.hot) {


2 errors found.
2 errors potentially fixable with the `--fix` option.
 @ multi (webpack)-dev-server/client?http://localhost:5000 (webpack)/hot/dev-server.js ./node_modules/react-hot-loader/patch.js ./src/index

ERROR in ./src/App.jsx
error: Missing trailing comma (comma-dangle) at src/App.jsx:6:23:
  4 | export default class App extends Component {
  5 |   state = {
> 6 |     name: 'ci-metrics'
    |                       ^
  7 |   };
  8 |
  9 |   render() {


1 error found.
1 error potentially fixable with the `--fix` option.
 @ ./src/index.jsx 4:0-24 15:2-34 15:29-33
 @ multi (webpack)-dev-server/client?http://localhost:5000 (webpack)/hot/dev-server.js ./node_modules/react-hot-loader/patch.js ./src/index
@eliperelman
Copy link
Member

Hmm, this is another thing I don't replicate. I wonder if you have some global install of eslint or something that is conflicting with Neutrino. Here is the output of me running the same command:

❯ npx @neutrinojs/create-project ci-metrics
                          _          _
      _ __    ___  _   _ | |_  _ __ (_) _ __    ___
     | '_ \  / _ \| | | || __|| '__|| || '_ \  / _ \
     | | | ||  __/| |_| || |_ | |   | || | | || (_) |
     |_| |_| \___| \__,_| \__||_|   |_||_| |_| \___/

Welcome to Neutrino! 👋
To help you create your new project, I am going to ask you a few questions.

? 🤔  First up, what would you like to create? A web or Node.js application
? 🤔  Next, what kind of application would you like to create? React
? 🤔  Would you like to add a test runner to your project? None
? 🤔  Would you like to add linting to your project? Airbnb style rules

👌  Looks like I have all the info I need. Give me a moment while I create your project!

   create ci-metrics/package.json
   create ci-metrics/.neutrinorc.js
   create ci-metrics/src/App.css
   create ci-metrics/src/App.jsx
   create ci-metrics/src/index.jsx
   create ci-metrics/.eslintrc.js

⏳  Installing dependencies: prop-types, react, react-dom, react-hot-loader
⏳  Installing devDependencies: @neutrinojs/react, neutrino, @neutrinojs/airbnb
⏳  Performing one-time lint

Hooray, I successfully created your project!

I have added a few yarn scripts to help you get started:
  • To build your project run:  yarn build
  • To start your project locally run:  yarn start
  • To lint your project manually run:  yarn lint
    You can also fix some linting problems with:  yarn lint --fix

Now change your directory to the following to get started:
  ci-metrics

❤️  Neutrino

/private/var/folders/dl/9lrzd5110zlft676qdpr7xn40000gp/T 53s
❯ cd ci-metrics

/private/var/folders/dl/9lrzd5110zlft676qdpr7xn40000gp/T/ci-metrics
❯ gs
fatal: Not a git repository (or any of the parent directories): .git

/private/var/folders/dl/9lrzd5110zlft676qdpr7xn40000gp/T/ci-metrics
❯ yarn start
yarn run v1.3.2
$ neutrino start
✔ Development server running on: http://localhost:5000
✔ Build completed
✔ Build completed
^C

/private/var/folders/dl/9lrzd5110zlft676qdpr7xn40000gp/T/ci-metrics 7s
❯ yarn lint
yarn run v1.3.2
$ neutrino lint
✔ Running lint completed

@armenzg
Copy link
Author

armenzg commented Dec 13, 2017

I've probably installed dependencies on my early days by mistake [1].
I've brought it down to [2].
I still get linting issues.
I've also removed my linting file [3]
I've tried from scratch with the same results [4]
Here are the versions of the binaries involved:

armenzg@armenzg-mbp ci-metrics$ node --version
v8.8.0
armenzg@armenzg-mbp ci-metrics$ npm --version
5.5.1
armenzg@armenzg-mbp ci-metrics$ npx --version
9.6.0
armenzg@armenzg-mbp ci-metrics$ yarn --version
1.3.2

[1]

armenzg@armenzg-mbp ci-metrics$ npm ls -g --depth=0
/usr/local/lib
├── create-react-app@1.4.3
├── eslint@4.11.0
├── eslint-config-airbnb@16.1.0
├── eslint-plugin-import@2.8.0
├── eslint-plugin-jsx-a11y@6.0.2
├── eslint-plugin-react@7.4.0
├── exp@45.1.0
├── js-beautify@1.6.14
├── minimatch@3.0.2
├── mocha@4.0.1
├── npm@5.5.1
├── npm-check-updates@2.13.0
├── opencollective@1.0.3
├── platform-health-dashboard@0.0.1 -> /Users/armenzg/repos/platform-health
├── telemetry-next-node@2.0.0 -> /Users/armenzg/repos/telemetry-next-node
├── webpack@3.8.1
└── yarn@1.0.1

npm ERR! peer dep missing: react@^0.14.0 || ^15.0.0-0 || ^16.0.0-0, required by react-redux@5.0.6
npm ERR! peer dep missing: jquery@>=1.11.1, required by metrics-graphics@2.11.0
npm ERR! peer dep missing: superagent@~0.17.0, required by superagent-retry@0.3.0

[2]

armenzg@armenzg-mbp ci-metrics$ npm ls -g --depth=0
/usr/local/lib
├── npm@5.5.1
├── npm-check-updates@2.13.0
└── yarn@1.0.

[3]

armenzg@armenzg-mbp ci-metrics$ cat ~/.eslintrc.js
module.exports = {
"extends": "airbnb",
"rules": {}
};

[4]

armenzg@armenzg-mbp repos$ rm -rf ci-metrics/
armenzg@armenzg-mbp repos$ npx @neutrinojs/create-project ci-metrics
npx: installed 210 in 8.007s
_ _
_ __ ___ _ _ | |_ _ __ () _ __ ___
| '
\ / _ | | | || || '|| || '_ \ / _
| | | || /| || || | | | | || | | || () |
|
| |_| _
| _,| _||| |||| |_| ___/

Welcome to Neutrino! 👋
To help you create your new project, I am going to ask you a few questions.

? 🤔 First up, what would you like to create? A web or Node.js application
? 🤔 Next, what kind of application would you like to create? React
? 🤔 Would you like to add a test runner to your project? None
? 🤔 Would you like to add linting to your project? Airbnb style rules

👌 Looks like I have all the info I need. Give me a moment while I create your project!

create ci-metrics/package.json
create ci-metrics/.neutrinorc.js
create ci-metrics/src/App.css
create ci-metrics/src/App.jsx
create ci-metrics/src/index.jsx
create ci-metrics/.eslintrc.js

⏳ Installing dependencies: prop-types, react, react-dom, react-hot-loader
⏳ Installing devDependencies: @neutrinojs/react, neutrino, @neutrinojs/airbnb
⏳ Performing one-time lint

Hooray, I successfully created your project!

I have added a few yarn scripts to help you get started:
• To build your project run: yarn build
• To start your project locally run: yarn start
• To lint your project manually run: yarn lint
You can also fix some linting problems with: yarn lint --fix

Now change your directory to the following to get started:
ci-metrics

❤️ Neutrino
armenzg@armenzg-mbp repos$ cd ci-metrics/

@eliperelman
Copy link
Member

We added a --debug flag to the command now, could you try and it report its output?

yarn create @neutrinojs/project ci-metrics --debug

@armenzg
Copy link
Author

armenzg commented Dec 14, 2017

I can't use yarn as per the other issue filed.
Using npx instead.
npx @neutrinojs/create-project asd --debug

How can I know that debug flag worked? I don't see anything different.

@eliperelman
Copy link
Member

I'm not sure if npx proxies CLI flags, so you may need to do:

npx @neutrinojs/create-project asd -- --debug

If it works, you should see the full stdout from the installation of deps, devDeps, and running the one-time lint.

Also, looking at your log, it appears that this does in fact run the lint and it wasn't skipped, as previous postulated in spectrum:

Performing one-time lint

https://spectrum.chat/?t=339e83d6-7c69-4c86-883f-de77e709e802

@armenzg
Copy link
Author

armenzg commented Dec 20, 2017

No changes.

armenzg@armenzg-mbp repos$ npx @neutrinojs/create-project asd -- --debug
npx: installed 210 in 9.2s
_ _
_ __ ___ _ _ | |_ _ __ () _ __ ___
| '
\ / _ | | | || || '|| || '_ \ / _
| | | || /| || || | | | | || | | || () |
|
| |_| _
| _,| _||| |||| |_| ___/

Welcome to Neutrino! 👋
To help you create your new project, I am going to ask you a few questions.

? 🤔 First up, what would you like to create? A web or Node.js application
? 🤔 Next, what kind of application would you like to create? React
? 🤔 Would you like to add a test runner to your project? None
? 🤔 Would you like to add linting to your project? Airbnb style rules

👌 Looks like I have all the info I need. Give me a moment while I create your project!

create asd/package.json
create asd/.neutrinorc.js
create asd/src/App.css
create asd/src/App.jsx
create asd/src/index.jsx
create asd/.eslintrc.js

⏳ Installing dependencies: prop-types, react, react-dom, react-hot-loader
⏳ Installing devDependencies: @neutrinojs/react, neutrino, @neutrinojs/airbnb
⏳ Performing one-time lint

Hooray, I successfully created your project!

I have added a few yarn scripts to help you get started:
• To build your project run: yarn build
• To start your project locally run: yarn start
• To lint your project manually run: yarn lint
You can also fix some linting problems with: yarn lint --fix

Now change your directory to the following to get started:
cd asd

❤️ Neutrino

@jorgemuza
Copy link

@armenzg I had the same issue...running yarn lint --fix before executing yarn start seems to work....

@edmorley
Copy link
Member

I can reproduce this (Windows 10, node 8.9.4, npm 5.6.0, npx 9.7.1):

tmp $ npx @neutrinojs/create-project ci-metrics
npx: installed 212 in 11.953s
C:\Users\Ed\AppData\Roaming\npm-cache\_npx\5700\node_modules\@neutrinojs\create-project\bin\create-project.js
                          _          _
      _ __    ___  _   _ | |_  _ __ (_) _ __    ___
     | '_ \  / _ \| | | || __|| '__|| || '_ \  / _ \
     | | | ||  __/| |_| || |_ | |   | || | | || (_) |
     |_| |_| \___| \__,_| \__||_|   |_||_| |_| \___/

Welcome to Neutrino! 👋
To help you create your new project, I am going to ask you a few questions.

? 🤔  First up, what would you like to create? A web or Node.js application
? 🤔  Next, what kind of application would you like to create? React
? 🤔  Would you like to add a test runner to your project? None
? 🤔  Would you like to add linting to your project? Airbnb style rules

👌  Looks like I have all the info I need. Give me a moment while I create your project!

   create ci-metrics\package.json
   create ci-metrics\.neutrinorc.js
   create ci-metrics\src\App.css
   create ci-metrics\src\App.jsx
   create ci-metrics\src\index.jsx
   create ci-metrics\.eslintrc.js

⏳  Installing dependencies: prop-types, react, react-dom, react-hot-loader
⏳  Installing devDependencies: @neutrinojs/react, neutrino, @neutrinojs/airbnb
⏳  Performing one-time lint

Hooray, I successfully created your project!

I have added a few yarn scripts to help you get started:
  • To build your project run:  yarn build
  • To start your project locally run:  yarn start
  • To lint your project manually run:  yarn lint
    You can also fix some linting problems with:  yarn lint --fix

Now change your directory to the following to get started:
  cd ci-metrics

❤️  Neutrino
tmp $ cd ci-metrics/
ci-metrics $ yarn start
yarn run v1.5.1
$ neutrino start
√ Development server running on: http://localhost:5000
√ Build completed

ERROR in ./src/index.jsx
error: Expected a newline after '(' (function-paren-newline) at src\index.jsx:6:26:
  4 |
  5 | const root = document.getElementById('root');
> 6 | const load = () => render((
    |                          ^
  7 |   <AppContainer>
  8 |     <App />
  9 |   </AppContainer>


error: Expected a newline before ')' (function-paren-newline) at src\index.jsx:10:8:
   8 |     <App />
   9 |   </AppContainer>
> 10 | ), root);
     |        ^
  11 |
  12 | // This is needed for Hot Module Replacement
  13 | if (module.hot) {


2 errors found.
2 errors potentially fixable with the `--fix` option.
 @ multi (webpack)-dev-server/client?http://localhost:5000 (webpack)/hot/dev-server.js ./node_modules/@neutrinojs/react/node_modules/react-hot-loader/patch.js ./src/index

ERROR in ./src/App.jsx
error: Missing trailing comma (comma-dangle) at src\App.jsx:6:23:
  4 | export default class App extends Component {
  5 |   state = {
> 6 |     name: 'ci-metrics'
    |                       ^
  7 |   };
  8 |
  9 |   render() {


1 error found.
1 error potentially fixable with the `--fix` option.

@edmorley
Copy link
Member

edmorley commented Mar 17, 2018

So this is because the monorepo's .neutrinorc.js doesn't (a) include .jsx files in the lint, (b) uses the airbnb-base preset not airbnb, (c) possibly misses a few directories due to the way directories are manually specified rather than using globs (which is to work around not including the test directories, which we should probably just lint as well, or else switch to using the exclude option as a cleaner way of ignoring them).

@edmorley
Copy link
Member

I've opened #754 to allow using --debug to see which files are being linted, to make it easier to come up with the necessary include/exclude rules here.

@edmorley
Copy link
Member

edmorley commented Mar 22, 2018

Hmmm so there are multiple problems here:

  1. The Neutrino monorepo's .neutrinorc.js only checks JS files and a subset of all directories.
  2. Once the JS file type hardcoding of (1) is fixed, the ESLint preset uses neutrino.options.extensions which includes file types that really shouldn't be linted (eg json). I think we've stuffed too many things in neutrino.options.extensions.
  3. If fixing (1) by taking the "opt out a few directories, rather than opting in a few directories" approach, there needs to be a way to exclude packages/*/node_modules/, however the current ESLint exclude option doesn't support globs due to the use of basename. This can be worked around by using an .eslintignore in the monorepo root instead, but it demonstrates an issue end users will hit too.
  4. The Neutrino monorepo's .neutrinorc.js is inheriting from airbnb-base rather than airbnb.
  5. The Neutrino monorepo's .neutrinorc.js lint config uses prettier, which is incompatible with many parts of the airbnb preset. This means that as-is there is no way we can format the template files in a way that keeps both the neutrino Travis run happy and fix the lint errors seen here post create app.
  6. With all of the above resolved, there are still some eslint failures from import/no-unresolved and import/extensions in the create-project templates, since the dependencies like react and preact are not official dependencies. Perhaps we could make them devDependencies as a workaround?

@eliperelman thoughts about the prettier issue? Could we just stop using it? Or else override it to more closely match airbnb?

The only other options I could think of were:

  • adding eslint ignore markers to the known problematic files
  • updating the create-project feature such that after initially bootstrapping a project it also runs neutrino lint --fix, thereby bringing the generated files in line with the chosen airbnb/standard/... lint config. This does seem like overkill however - plus there may be some failures that aren't --fixable.

@eliperelman
Copy link
Member

I want to address the issue here, before I give an answer to your other questions. We don't want to lint create-project templated files the same way as the monorepo. A user can use standardjs, airbnb, or no linting with create-project. One uses semicolons, the other doesn't. Therefore, it's impossible to fix linting issues in the repo prior to them landing in the project: we always have to run a lint fix in the project before handing it off to the user to resolve these types of discrepancies in linting preferences. We have to make sure that whatever we land in the repo is auto-fixable by any of the projects we support out of the box.

This means the root of this issue lies in a failing or non-running linting command during the create-project process. This is what needs to be debugged.


  • updating the create-project feature such that after initially bootstrapping a project it also runs neutrino lint --fix, thereby bringing the generated files in line with the chosen airbnb/standard/... lint config. This does seem like overkill however - plus there may be some failures that aren't --fixable.

https://github.com/mozilla-neutrino/neutrino-dev/blob/d73f307ce73c1e80aed5cb785b9828658e36a19e/packages/create-project/commands/init/index.js#L186-L192

@edmorley
Copy link
Member

edmorley commented Mar 22, 2018

Oh create-app already tries to perform a one-off lint haha I missed that.
Yeah agree it must not be working for myself and Armen then.

@edmorley
Copy link
Member

Ah seeing #714 I think I know what the problem is. The one time lint command is run as neutruno lint --fix, which only works if Neutrino is on PATH. Instead it should be using {yarn,npm} lint.

@eliperelman presumably this worked locally for you since yarn link adds neutrino to the global yarn bin directory, which is likely on PATH.

@edmorley
Copy link
Member

This should be fixed in v8.2.3 - could someone confirm? (I don't currently have npm/npx installed, and currently have the monorepo yarn linked globally, which hides the original issue here anyway)

@armenzg
Copy link
Author

armenzg commented May 7, 2018

This works. Thanks for fixing it!

I run the following commands:

  • npx @neutrinojs/create-project foobar
  • cd foobar && yarn start

My versions:

armenzg@armenzg-mbp repos$ node --version
v9.3.0
armenzg@armenzg-mbp repos$ npx --version
6.0.0

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

No branches or pull requests

4 participants