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

Installed goodparts from npm and got .eslintrc.js which requires './rules ... #248

Closed
nelsonic opened this issue Sep 28, 2016 · 11 comments
Closed

Comments

@nelsonic
Copy link
Member

Hi @eliasCodes / @Shouston3 / @SimonLab / @jrans
Hope your evening is going well...
I just tried to npm install goodparts and it creates an .eslintrc.js file in the root of my project:
eslintrc-js-requires-rules
Which attempts to require('./rules') ... is this the desired behaviour...?

@eliasmalik
Copy link
Contributor

eliasmalik commented Sep 28, 2016

@nelsonic see #247 and dwyl/abase#46

This is our SHORT TERM HACK so we can use goodparts with atom. The file you're seeing is just a symlink. The ./rules file it's requiring is relative to the goodparts directory in node_modules, not the project root.

The proper long term solution is to create an atom plugin #243.

@nelsonic
Copy link
Member Author

@eliasCodes so it's ok to include this file in my .gitignore ?

nelsonic added a commit to dwyl/technology-stack that referenced this issue Sep 28, 2016
@eliasmalik
Copy link
Contributor

@nelsonic yes I would recommend that you do.

@jrans
Copy link
Member

jrans commented Oct 6, 2016

@nelsonic @eliasCodes symlinking isn't ideal. Just committed my .eslintrc.js symlink into my new repo and not too pleased. Obviously my bad but if I'm going to the effort of adding it to my .gitignore i'd rather just create an my own .eslintrc.js and extend the goodparts module saving having to use global eslint (in atom) which is again causing me problems working on other projects which have their own .eslintrc

@eliasmalik
Copy link
Contributor

eliasmalik commented Oct 6, 2016

@jrans @nelsonic what do you think about this:

  • Remove the post-install script from goodparts package.json
  • Default usage would then be to create a .eslintrc file which has extends: goodparts
  • Add functionality to the goodparts CLI to do the symlinking if that's still something we want to keep (e.g. you would run goodparts link or something like that)

@eliasmalik
Copy link
Contributor

@jrans @nelsonic there is a problem with the approach I outlined above: any shareable configs must have package names of the form "eslint-config-packageName". This is documented here, and you can see the code that I think corresponds to it here. I've tried it out locally and get an error thrown when eslint can't find the config. In my .eslintrc.json:

{
  "extends": "goodparts"
}

Then when I try to run the goodparts CLI, I get:

Error: Cannot find module 'eslint-config-goodparts'

So a npm package by the name of goodparts cannot be used as a shareable config.

I've pushed a branch where I've implemented steps 1 and 3 from my last comment. Let me know if you want me to PR it.

@nelsonic
Copy link
Member Author

@eliasCodes ideally we would not have an .eslintrc or .eslintrc.json in any of our projects and simply use the goodparts .bin script e.g: npm run goodparts ...

@eliasmalik
Copy link
Contributor

@nelsonic well happily that's already possible 😄

I can see why it is handy to have a real-time linter running in your editor though. If we do want to make that happen, it might just be better to bite the bullet and actually write an atom plugin at some point.

Anyway, please let me know if you would like me to submit a PR. If so, I need to write a couple more tests because I introduced a couple branches into the bin/cmd.js that aren't covered by a simple require.

@eliasmalik
Copy link
Contributor

Has this issue now been fully addressed (can it be closed)?

@nelsonic
Copy link
Member Author

@eliasCodes great question. we will know soon ... dwyl/hapi-auth-jwt2#211 😉

@nelsonic
Copy link
Member Author

hapi-auth-jwt2-no-eslintirc-file

#Succcess
Thanks @eliasCodes ❤️

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