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

ESLint config not found when missing from application root #197

Closed
amcgee opened this issue Dec 3, 2019 · 4 comments · Fixed by #222
Closed

ESLint config not found when missing from application root #197

amcgee opened this issue Dec 3, 2019 · 4 comments · Fixed by #222
Labels

Comments

@amcgee
Copy link
Member

amcgee commented Dec 3, 2019

And eslint plugin dependency resolution gets funky when adding a .eslintrc.js to /shell.

(Credit @awgaan)

@amcgee
Copy link
Member Author

amcgee commented Dec 3, 2019

@varl this seems to be an issue when eslint plugins aren't hoisted up to node_modules from @dhis2/cli-style/node_modules?

./node_modules/.bin/d2-style js check
d2-style > javascript 
Cannot find module 'eslint-config-prettier'
Referenced from: /Users/awm/dev/dhis2/frontend/tools/platform/shell/node_modules/
@dhis2/cli-style/config/js/eslint.config.js
Referenced from: /Users/awm/dev/dhis2/frontend/tools/platform/shell/node_modules/
@dhis2/cli-style/config/js/eslint-react.config.js
Referenced from: /Users/awm/dev/dhis2/frontend/tools/platform/shell/.eslintrc.js
➜  shell git:(feat/compiler-2dot0) ✗ ./node_modules/.bin/d2-style --version
5.0.3
➜  shell git:(feat/compiler-2dot0) ✗ ls ./node_modules | grep eslint
@typescript-eslint
eslint
eslint-import-resolver-node
eslint-module-utils
eslint-plugin-react
eslint-scope
eslint-utils
eslint-visitor-keys
➜  shell git:(feat/compiler-2dot0) ✗ ls ./node_modules/@dhis2/cli-style/node_modules | grep eslint
babel-eslint
eslint
eslint-config-prettier
eslint-plugin-eslint-plugin
eslint-plugin-react

@varl
Copy link
Contributor

varl commented Dec 3, 2019

Hm. Any branch I can reproduce this on?

First thing I would try is to remove ESLint 5 from the app-shell: https://github.com/dhis2/app-platform/blob/master/shell/package.json#L28 as that might cause conflicts with ESLint 6 that cli-style depends on.

ESLint should only live in the root package, so it doesn't have a place in the shell sub-package.

The ESLint 5 => 6 upgrade changes how they resolve plugins and that sounds similar to what we are seeing here: https://eslint.org/docs/user-guide/migrating-to-6.0.0#plugins-and-shareable-configs-are-no-longer-affected-by-eslints

@amcgee
Copy link
Member Author

amcgee commented Dec 3, 2019

Ah hmm, yeah that could be it. Unfortunately I think we need to keep it in the shell, otherwise a platform app which doesn't have its own eslint config will fail with an "eslint config not found" error (you can repro that one by just running d2 app scripts init test and then cd test && yarn build, which is what @awgaan saw. I ran into the resolution issue trying to add a config to the shell)

Will try with eslint 6... not sure how we want to solve the "eslint somewhere higher in the tree" issue (we pass SKIP_PREFLIGHT_CHECK to CRA to supress a warning about this)

@dhis2-bot
Copy link
Contributor

🎉 This issue has been resolved in version 3.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging a pull request may close this issue.

3 participants