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

Add environment variable DISABLE_ESLINT #7961

Closed
wants to merge 4 commits into from

Conversation

giuband
Copy link

@giuband giuband commented Nov 11, 2019

The idea behind this new environment DISABLE_ESLINT_DEV variable is to:

  1. speed up compilation time on dev mode
  2. allow developers to write temporary code without caring about ESLint too much.

At the moment 2) is pretty hard, because depending on your ESLint config you might not even able to have a console.log or an unused variable in your code without seeing your dev build failing, making the experience pretty frustrating.

Open for feedback. I am actually uncertain whether we should have instead just a DISABLE_ESLINT setting that would disable ESLint for both environments.

Related issues:

@facebook-github-bot
Copy link

Hi giuband! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@giuband
Copy link
Author

giuband commented Nov 12, 2019

I've changed it to DISABLE_ESLINT as it would make this more useful for all the projects that already have a lint step in CI and/or precommit hooks, so to avoid duplicated linting.

@giuband giuband changed the title Add flag DISABLE_ESLINT_DEV to disable eslint on dev server Add environment variable DISABLE_ESLINT Nov 12, 2019
@stale
Copy link

stale bot commented Dec 12, 2019

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Dec 12, 2019
@giuband
Copy link
Author

giuband commented Dec 12, 2019

Hi team, any updates on this one?

@stale stale bot removed the stale label Dec 12, 2019
@mrmckeb
Copy link
Contributor

mrmckeb commented Dec 15, 2019

I think this will likely be a config flag, we've been talking about adding a config file in the next major release of CRA. Does anyone else in the team have thoughts? @ianmcnally or @ianschmitz perhaps?

@michaelsbradleyjr
Copy link

Just my 2¢, but I think it would be awesome if a CRA project could have a cra.config.js file at the top-level of the project that worked similarly to next.config.js, i.e. CRA's config could be passed to a function-export from cra.config.js and tweaked as needed.

@mrmckeb
Copy link
Contributor

mrmckeb commented Dec 18, 2019

@michaelsbradleyjr That's on the cards for v4 at this stage.

@Bessonov
Copy link

I don't care about CRAs eslint rules for two reasons. First one, I have own rule set, which I use through multiple projects. And the second, I don't want that eslint slows me down while development. Therefore a big +1 for this PR.

@stale
Copy link

stale bot commented Jan 24, 2020

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Jan 24, 2020
@Bessonov
Copy link

further activity occurs

@battaglr
Copy link

battaglr commented Feb 5, 2020

Thanks, @giuband! This is a great thing to have supported natively, since it's so frustrating having the build break on you while you're actively coding, so now you have to stop, add a disable comment, etc. It just breaks the flow.

Please, let me know if I can be of any help!

@AbraaoAlves
Copy link

It's very useful for this issue: #8096

@tomasbruckner
Copy link

any update on this?

@giuband
Copy link
Author

giuband commented Oct 13, 2020

@mrmckeb @ianmcnally any updates on this one? Is it still under the radar to offer more flexibility regarding running or not eslint?

@giuband
Copy link
Author

giuband commented Feb 5, 2021

Closing as this has been implemented in #10170. Thanks @mrmckeb !

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

Successfully merging this pull request may close these issues.

10 participants