-
Notifications
You must be signed in to change notification settings - Fork 29.1k
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
doc: more stability guarantees in deprecation policy #18682
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -364,13 +364,13 @@ there is expected impact on the user community. | |
|
||
Node.js uses three Deprecation levels: | ||
|
||
* *Documentation-Only Deprecation* refers to elements of the Public API that are | ||
being staged for deprecation in a future Node.js major release. An explicit | ||
notice indicating the deprecated status is added to the API documentation | ||
but no functional changes are implemented in the code. There will be no | ||
runtime deprecation warnings emitted for such deprecations by default. | ||
* *Documentation-Only Deprecation* refers to elements of the Public API that | ||
should be avoided by developers. An explicit notice indicating the deprecation | ||
status is added to the API documentation but no functional changes are | ||
implemented in the code. | ||
There will be no warnings emitted for such deprecations at runtime by default. | ||
Documentation-only deprecations may trigger a runtime warning when launched | ||
with [`--pending-deprecation`][] flag (or its alternative, | ||
with [`--pending-deprecation`][] flag (or its alternative, the | ||
`NODE_PENDING_DEPRECATION=1` environment variable). | ||
|
||
* *Runtime Deprecation* refers to the use of process warnings emitted at | ||
|
@@ -391,6 +391,27 @@ Runtime Deprecations and End-of-life APIs (internal or public) must be | |
handled as semver-major changes unless there is TSC consensus to land the | ||
deprecation as a semver-minor. | ||
|
||
Possible reasons for Runtime deprecations can be: | ||
|
||
* Node.js will no longer be able to support the API, for example if required | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: comma after example
|
||
code is being removed in an upstream project. | ||
|
||
* The API is too hard or impossible to use correctly, increasing the risk of | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
undetected bugs or security issues in existing code. | ||
|
||
* Breaking the API makes way for simplification of its implementation that is | ||
significant enough to make maintainability of Node.js easier, or enables | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Compared to the ecosystem usage / taking ecosystem into account / something like that? E.g. something huge and ugly but widely used is still kept, but something that adds only a bit of churn but isn't used by anyone can be runtime-deprecated quickly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ChALkeR I think this is at least implicitly covered by the paragraph below … Either way, this doesn’t feel like the right place for that – this is listing the reasons to deprecate, and all decisions to deprecate should take ecosystem usage into account, right? |
||
implementation of features that would otherwise be impossible. | ||
|
||
In addition, a Documentation-Only deprecation may be issued if it is desirable | ||
to recommend a single canonical way out of multiple different ones | ||
which achieve a particular goal with Node.js. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is probably correct in most cases but I would not want to strictly limit it to documentation only. I would rather keep that part open and individually decide what to do. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we please change that to:
|
||
|
||
In particular, a Runtime deprecation should *not* be added to an API that | ||
that requires only trivial maintenance, or only because it has | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A nit: |
||
never been documented or has only accidentally been exposed and there is | ||
usage of said API in existing ecosystem code. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I disagree that a runtime deprecation should not be used in case an API has never been documented before. Especially in such a case I would want a runtime deprecation as otherwise it is highly unlikely that anyone will ever realize that a specific code is actually deprecated and was never documented. Also accidentally exposing a API to the ecosystem should receive a runtime deprecation out of my perspective. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I fail to see how that is a bad thing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I am closer to the viewpoint that @addaleax has on this, i.e. this is not necessarily a bad thing. However, one has to recognize that, over time, we end up with a large number of documentation-only deprecated stuff (i.e. bloat). That's the downside, and I personally think it is okay. I don't think we are too bloated ATM. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we add this, I would like a note that runtime deprecation may occur if a previously undocumented or accidentally exposed API is given a documented and intentional API. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would agree with @bmeck's caveat here. If there is a supported replacement, we ought to be able to runtime deprecate the trivial unsupported thing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There is noticeable/significant usage? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think one reason for the different point of views here are what we understand with a breaking change and more importantly: how "bad" a runtime deprecation is for each of us. For me a runtime deprecation means there is just something logged so people get notified that it is not recommended to use a specific API anymore / in a specific way. It does not even mean that something might or might not be removed. I feel like that is nothing bad - almost ever. Without that notification we reach probably less than a thousandths of the users and probably especially those for whom the deprecation is actually meant for. A documentation deprecation is only useful out of my perspective when we want to only prevent new people from using something. But even that is often probably not a reason for people to not use something, especially as a lot of people do not read a documentation properly and they might just check a online example that they found somewhere else than in the official documentation. So I feel like we should always have the possibility of runtime deprecating things that were never meant to be exposed or that were never documented. In some cases it might be useful to add a official API instead, on other cases there might be nothing to do besides deprecating it to tell people to better switch and in even more cases there are probably other solutions. Documenting something and than runtime deprecating is the opposite of what I would see as logical because a lot more people will actually read the documentation about the deprecation (especially new people) than people who try to use a API they stumble upon somewhere in the Node.js source code. So for me it is like the opposite step of what is meant to be achieved. Deciding on what to do should out of my perspective be individual per case. I do not want to have a strict limitation one way or the other. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think nobody's disagreeing with that. :)
Okay, I can see that. Let me think about it.
The feedback that we've gotten from the Buffer deprecation discussions is different: A runtime deprecation is functional breakage in itself.
I agree.
Please note that that's not what this PR is suggesting; I am totally fine with considering an undocumented API to be implicitly docs-deprecated for the purposes of this policy. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I just mentioned that because it was brought up.
Well, I personally think that is a strange view. The code itself still works perfectly fine, so having a warning is something that I would actually want. And especially the Buffer deprecation is something where there are a lot of different opinions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This paragraph contains one sentence that seems overly long and perhaps difficult to understand. Consider breaking it into multiple sentences. Perhaps something like this?:
|
||
|
||
All Documentation-Only and Runtime deprecations will be assigned a unique | ||
identifier that can be used to persistently refer to the deprecation in | ||
documentation, emitted process warnings, or errors thrown. Documentation for | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This paragraph says that "Documentation-Only" is a deprecation level, however the "pending" implies that it has not been deprecated yet.
I do not think this terminology is consistent with the
--pending-deprecation
flag.IMHO
--pending-deprecation
needs to be better explained as another level, something like "pending a runtime deprecation".There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mcollina I don't think we can rename the flag anymore, although I agree that the name is unfortunate. I think the text is explicit enough about what the flag does, but I'm not sure we should codify that
--pending-deprecations
means that we are definitely going to runtime-deprecate something at some point?(For context, this text is from the rather recent #18433, /cc @ChALkeR)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should name "pending runtime deprecation" as its own deprecation level. If not, we should try to define this better. Before this change, "doc-only deprecations" were a step towards "runtime deprecations" (in most cases). after this change, they became their own thing, which not necessarily would lead to a more severe deprecation. I think there is a space for something in between, were a doc-only deprecation would lead to a runtime deprecation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mcollina See #18417.
I don't think there needs to be yet another separate deprecation level, in fact I think that most of the new doc-deprecations should come with
--pending-deprecation
support (where possible).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what it means though, doesn't it? We currently use it for unsafe
new Buffer()
calls and I believe the plan is to warn more forcefully in the future.Documentation-only deprecations are more for steering people towards the canonical solution.
--pending-deprecations
doesn't need to warn about that because we're not going to runtime-deprecate it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
documentation-only could but doesn't mean must be runtime deprecated. The only time it definitely should mean runtime deprecation is if we ultimately plan to end-of-life the code.
pending-deprecation should mean we'll eventually runtime deprecate.
Any documentation-only deprecation that we expect to move to runtime deprecation in the future should emit pending deprecation.