Skip to content

semver policy for rule changes #1787

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

Closed
bdwain opened this issue May 12, 2018 · 5 comments
Closed

semver policy for rule changes #1787

bdwain opened this issue May 12, 2018 · 5 comments

Comments

@bdwain
Copy link

bdwain commented May 12, 2018

I noticed that in v7.8.0, the no-deprecated rule was enhanced to warn about componentWillMount and the other newly deprecated lifecycle methods. I'm curious why this is not considered a breaking change that requires a major version bump. For codebases that have that rule enabled (or the recommended set) and have the disabled lifecycle methods, pulling that change in broke their builds even though it was a minor upgrade.

I know a linter is kind of weird in that new features can break apps, but It seems to me that any time a rule gets more strict or a rule is added to the recommended set, that should be considered a breaking change. If the policy is different, maybe I just need to change my wildcard to be a ~ instead of a ^. But I'd appreciate some clarification on how the semver bumps are decided.

Thanks!

@ljharb
Copy link
Member

ljharb commented May 12, 2018

Hmm, good point.

Initially, I wanted the new methods to only apply when the React version pragma is 16.3+ - since these are already runtime deprecation warnings in React 16.3+. Perhaps that's a change we should make now?

cc @sergei-startsev / #1750

@bdwain
Copy link
Author

bdwain commented May 12, 2018

That would definitely help. Though it could still break someone couldn't it? The react pragma version is an eslint thing right? If a person could have had the version set to 16.3+ before 7.8 came out, then upgrading to this new rule would break them without a major upgrade.

@ljharb
Copy link
Member

ljharb commented May 12, 2018

In that case they’d have runtime warnings, so the eslint warnings would be catching those sooner.

For the eslint ecosystem, the standard isn’t “any new errors is major”, it’s more nuanced than that. I don’t think that means you want ~, either - i think it means that if a new release suddenly causes linter errors, it’s supposed to be something you should have been doing in the first place. If you set the 16.3 pragma, it’s assumed that you’re following React 16.3’s conventions already.

@bdwain
Copy link
Author

bdwain commented May 12, 2018

Are the runtime warnings enabled yet? I haven't seen them in my apps.

Your point makes sense though. I guess linters are a special case in that they are designed to break builds. So they can't follow semver in the traditional sense.

Thanks for the explanation. I like the pragma solution.

@bdwain
Copy link
Author

bdwain commented May 12, 2018

So it turns out the rule already does respect the pragma. I just never set my version of it. Sorry for the false alarm. I'll close this. But I think react may not have enabled the runtime warnings yet because I have not seen any in my apps and they are in dev mode and still have some deprecated lifecycle methods.

@bdwain bdwain closed this as completed May 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants