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

no-deprecated-api: Configure which APIs to prevent #58

Closed
feross opened this issue Nov 23, 2016 · 7 comments · Fixed by singapore/lint-condo#239
Closed

no-deprecated-api: Configure which APIs to prevent #58

feross opened this issue Nov 23, 2016 · 7 comments · Fixed by singapore/lint-condo#239

Comments

@feross
Copy link
Contributor

feross commented Nov 23, 2016

I'd like to add this plugin to standard so we can use the no-deprecated-api rule. But I don't want to prevent people from using APIs that were deprecated in v7, for example, because that's an unstable version, and things may be un-deprecated.

Could we add a way to configure the rule to select which APIs to prevent? Manually specifying the individual rules is fine, or we could base it on the version that the feature was deprecated in. So I could say something like "no-deprecated-api": ["error", 4] or "no-deprecated-api": ["error", 6].

@mysticatea
Copy link
Owner

Thank you for this issue.

However, I don't have the plan to add the option for several reasons.

  • If someone is not interested in future versions, they can disable the rule merely since deprecated APIs will not be removed in the used version.
  • In most cases, the reasons of deprecating are valid in old versions. For example, Buffer constructors have security risks.

Before, fs.existsSync was un-deprecated on v6.8.0.
Does it possibly happen such event frequently? If so, I need to delay that I apply the deprecating to the rule while some period, since adding new deprecate warnings is a breaking change (it needs a major bump).

@feross
Copy link
Contributor Author

feross commented Nov 24, 2016

Does it possibly happen such event frequently?

In unstable versions of node, yes, I think this may happen again. They use the odd-numbered versions to test out more aggressive changes.

If someone is not interested in future versions, they can disable the rule merely since deprecated APIs will not be removed in the used version.

It's not that the user is not interested in future versions, but they might be running v4 and only want to see warnings for deprecations in v6, so they can be ready before migrating to v6. Maybe they don't care about v7 deprecations yet. This would let them do it in stages.

That said, no pressure to implement this. It's just a suggestion. Cheers!

@mysticatea
Copy link
Owner

Thank you.

In unstable versions of node, yes, I think this may happen again. They use the odd-numbered versions to test out more aggressive changes.

Hmm.
In my understanding, v7 is stable but is not going to LTS. In the LTS schedule image, master branch is unstable.

This would let them do it in stages.

That's right.
Hmm... I'm under considering.

@Trott
Copy link

Trott commented Nov 24, 2016

In my understanding, v7 is stable but is not going to LTS. In the LTS schedule image, master branch is unstable.

The "odd number = unstable" thing was definitely true with, for example, 0.10 vs. 0.11. But not anymore.

On the other hand, it's still kind of true. As you note, even numbers have a much longer life span due to LTS. As a result of odd numbers having shorter life spans, the breaking changes in 7.0.0 and the 9.0.0 releases may be bigger/riskier than those in 8.0.0 or 10.0.0 on the grounds that if it's a mistake, we undo it for the following major release (if undoing is a breaking change, like removing a feature) and the thing with the horrible mistake isn't supported anymore in six months.

But I think the issue here is when removal of runtime deprecation warnings are likely to occur. Un-deprecating is always an option. Introducing runtime deprecation, on the other hand, will always have to wait for a semver-major jump (about every six months).

@mcollina
Copy link

The current set of warnings work very well for application authors that are targeting a specific version of Node.

I think that Module authors have more granular needs: let's consider two functions, a() and b(), where b() supersedes a() in a semver-major change, and a() become deprecated. As a module author, I need to support the two versions of Node, and the best way might still be to keep using a(). On the other end for functions c() and d(), it's better to use a shim for d() in older nodes. Each deprecation is a completely different story.

We still need to support v0.10 in some libraries, because it is still being widely used.

@mysticatea
Copy link
Owner

Thank you for explanations.

@Trott If my understanding is correct, making deprecated API needs a major version, so it does not happen frequently? If so, I do not need to be too careful to update no-deprecated-api rule.

@mcollina My position is, using the eslint directive comments (e.g. //eslint-disable-line node/no-deprecated-api) is the best way in that case. It shows using deprecated APIs there to maintainers. Also describing "why we are using it" with comments is better. Please mind that the reasons of deprecating may be valid in old versions.


I still don't reach to enough motivation to add the option. I have seen 1 sample that un-deprecating API, fs.existsSync (this sample was happened in an even-number version). This API had been deprecated between 1.0.0 and 6.7.0. Both deprecating and un-deprecating seem to have discussions in an extended period.

@feross
Copy link
Contributor Author

feross commented Feb 8, 2017

👏

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