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

please remove eslint-config-react-app from dependencies, it is required only for react projects #182

Closed
JustFly1984 opened this issue Aug 20, 2019 · 11 comments
Labels
solution: invalid This doesn't seem right

Comments

@JustFly1984
Copy link

JustFly1984 commented Aug 20, 2019

warning "tsdx > [email protected]" has unmet peer dependency "[email protected]".
warning "tsdx > [email protected]" has incorrect peer dependency "[email protected]".
@JustFly1984
Copy link
Author

also please update @typescript-eslint/[email protected] to 2.0.0

@jaredpalmer
Copy link
Owner

Can you submit a PR?

@JustFly1984
Copy link
Author

@jaredpalmer
Which node version do you use to build? I will add .nvmrc to the project.

I'm running tests and getting an error:
Screenshot 2019-08-22 03 50 30

can't update stuff until you fix tests

@skvale
Copy link
Contributor

skvale commented Sep 1, 2019

  • eslint-config-react-app is used for more than just React projects
  • Bumping @typescript-eslint/eslint-plugin to 2.0.0 has breaking changes not supported by the current version of eslint-config-react-app
  • The warning of unmet/incorrect peer dependencies are coming from here: https://github.com/facebook/create-react-app/blob/master/packages/eslint-config-react-app/package.json. babel-eslint doesn't need to be a peer dependency for 100% TypeScript apps and it looks like eslint-plugin-flowtype could be `3.x || 4.x'

@JustFly1984
Copy link
Author

@skvale have you tried to run tests?

@skvale
Copy link
Contributor

skvale commented Sep 2, 2019

It looks like the tests are passing from your screenshot

There is a noisy test for an invalid build: https://github.com/palmerhq/tsdx/blob/master/test/fixtures/build-invalid/src/index.ts

@skvale
Copy link
Contributor

skvale commented Sep 19, 2019

re:

also please update @typescript-eslint/[email protected] to 2.0.0

eslint-config-react-app just bumped their versions of typescript-eslint deps: https://github.com/facebook/create-react-app/pull/7540/files

tsdx could bump eslint-config-react-app and @typescript-eslint/eslint-plugin now

@JustFly1984
Copy link
Author

JustFly1984 commented Sep 20, 2019

it is better to ditch eslint-config-react-app - it messes up linting projects without create-react-app

@agilgur5 agilgur5 added solution: invalid This doesn't seem right version: minor Increment the minor version when merged labels Oct 12, 2020
@agilgur5
Copy link
Collaborator

As was written previously above (#182 (comment)), it's definitely not just for React projects -- can view the rules and see for yourself.

I'm not sure what "messes up linting projects without create-react-app" refers to. I'm open to changing to a different default ESLint config (I personally use Standard), however that's a separate story -- please propose an RFC if interested.
It would likely be a very breaking change and folks who use lint configs tend to not want to make stylistic choices themselves and so may be adverse to that. TSDX also has overlap with CRA / CRL users (and has a good bit of inspiration from CRA), so similar config is probably more consistent for those folks.

@JustFly1984
Copy link
Author

@agilgur5 the issue for me is mostly out of date versions of dependencies in CRA. I use carefully managed list of eslint plugins myself, and would love to have smaller dependency tree, and less clutter in npm i logs about missing/outdated dependencies.

@agilgur5 agilgur5 removed the version: minor Increment the minor version when merged label Oct 12, 2020
@agilgur5
Copy link
Collaborator

Ok, that's different from "messing up linting"; that would mean it works but just is outdated. #810 has this issue too. I suspect CRA has somewhat of the same issue as TSDX where a major bump in any sub-package causes a major in CRA so as a result you end up being slow to update dependencies as you batch them into majors.
In any case, I don't particularly want to maintain an ESLint config myself for reasons I've written in #890 (comment) . For ecosystem consistency also would prefer using an existing, well-used config. If you've got suggestions, feel free to make an RFC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
solution: invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

4 participants