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

[Discussion] Deactivating new-cap rule (Immutable.js user) #465

Closed
rricard opened this issue Aug 20, 2016 · 6 comments
Closed

[Discussion] Deactivating new-cap rule (Immutable.js user) #465

rricard opened this issue Aug 20, 2016 · 6 comments
Milestone

Comments

@rricard
Copy link
Contributor

rricard commented Aug 20, 2016

Hello!

I just tried out create-react app and it's pretty cool! However, I'm an heavy user of Immutable and the eslint new-cap rule does not behave well with it (activated by default).

For now, I'm deactivating the rule on a per-file basis so that's not such a big issue! But I can't seem to be able to deactivate it project-wide in the package.json (it actually works on my editor, but not on the dev server ...).

To tackle this issue, I see two possibilities:

  • Find a way to get the dev server reading the project's eslint configuration in package.json (that would be cool to have that working anyway! => I'll try to take a look myself 😉 if it's possible)
  • Have a discussion around the new-cap rule that isn't universally liked (and discouraged when used with immutable, I'll point to you one comment from @leebyron on this: new-cap's capIsNewExceptions false warning eslint/eslint#2023 (comment), not saying we should get rid of it because he said so but I'd like to see what others think about it too!)

Thank you again for this project, hope this discussion will help make it even better!

@gaearon
Copy link
Contributor

gaearon commented Aug 20, 2016

We should disable that rule. It shouldn't have been in our config in the first place.

@gaearon gaearon added this to the 0.3.0 milestone Aug 22, 2016
@gaearon
Copy link
Contributor

gaearon commented Aug 22, 2016

Happy to merge a PR that removes it.

@rricard
Copy link
Contributor Author

rricard commented Aug 22, 2016

@gaearon Was planning to do that 😉, lemme just finish a small thing at work here…

rricard added a commit to rricard/create-react-app that referenced this issue Aug 22, 2016
This rule is considered dangerous in certain situations. This is especially true for Immutable.js users. See the discussion at issue facebook#465 for more information about this.
@rricard
Copy link
Contributor Author

rricard commented Aug 22, 2016

@gaearon, that didn't took much time anyway so here it is: #470

gaearon pushed a commit that referenced this issue Aug 22, 2016
This rule is considered dangerous in certain situations. This is especially true for Immutable.js users. See the discussion at issue #465 for more information about this.
gaearon pushed a commit that referenced this issue Aug 22, 2016
This rule is considered dangerous in certain situations. This is especially true for Immutable.js users. See the discussion at issue #465 for more information about this.
@gaearon gaearon modified the milestones: 0.2.2, 0.3.0 Aug 22, 2016
stayradiated pushed a commit to stayradiated/create-react-app that referenced this issue Sep 7, 2016
This rule is considered dangerous in certain situations. This is especially true for Immutable.js users. See the discussion at issue facebook#465 for more information about this.
feiqitian pushed a commit to feiqitian/create-react-app that referenced this issue Oct 25, 2016
This rule is considered dangerous in certain situations. This is especially true for Immutable.js users. See the discussion at issue facebook#465 for more information about this.
@yvele
Copy link

yvele commented Nov 15, 2016

Why don't you add an exception for Immutable?

"new-cap": [2, { "capIsNewExceptionPattern" : "^Immutable\\.." }]

PS: About immutable new-cap convention: immutable-js/immutable-js#10

@gaearon
Copy link
Contributor

gaearon commented Nov 21, 2016

We don't want to special-case Immutable here, IMO it would just be more confusing.

@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants