util: move deprecated utils to runtime deprecation by marco-ippolito · Pull Request #50488 · nodejs/node

nodejs-github-bot

Conversation

The deprecation code has been created in 6.12.0, I think we can move to runtime deprecation in 22
Moving the following APIs from documentation-only deprecation to runtime deprecation:

util._extend
util.isArray
util.isBoolean
util.isBuffer
util.isDate
util.isError
util.isFunction
util.isNull
util.isNullOrUndefined
util.isNumber
util.isObject
util.isPrimitive
util.isRegExp
util.isString
util.isSymbol
util.isUndefined
util.log

Is there any particular reason to do so? If not, we might as well keep it as a doc-only deprecation.

Not a particular reason per se but if it was deprecated it means it somehow needs to be removed, if we dont want to remove it, why deprecate it? It's dead code, we can probably runtime deprecate it in 22 and drop in 23.
And remove a part of documentation in utils called 'deprecated APIs' 😆

why deprecate it?

Because it fits one of those cases:

	Node.js APIs might be deprecated for any of the following reasons:

	* Use of the API is unsafe.
	* An improved alternative API is available.
	* Breaking changes to the API are expected in a future major release.

Being doc-deprecated doesn't mean it has to ever be runtime deprecated:

	cycle. There is no rule that deprecated code must progress to End-of-Life.
	Documentation-Only and Runtime Deprecations can remain in place for an unlimited
	duration.

I feel like that's more of the case of features like Domain which are probably gonna stay there forever.
Might want to add this to TSC agenda?

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @marco-ippolito.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

Anyone can add (or request to add) the tsc-agenda Issues and PRs to discuss during the meetings of the TSC. label, but at this step, there's not much for the TSC to discuss, it's up to the collaborators to accept or reject this PR. (well because it is semver-major PRs that contain breaking changes and should be released in the next major version. , it also needs approval from at least 2 TSC voting members, but still the TSC doesn't need to weigh in unless collaborators fail to find consensus).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM

@marco-ippolito please update the PR description to include the list of functions that will be moved from doc-deprecation to runtime deprecation.

I think this is adding unnecessary pain to our users for very little benefit for us (if any at all). Anyway, ping @nodejs/tsc since this is semver major.

ronag left a comment •

Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any benefit of this and a lot of annoyance. IMHO. We should only runtime deprecate things that we eventually intend to remove. There is small to no maintenance overhead on these so I don't see why we would.

Most of these API have been deprecated since v4, almost 9 years ago.
We have litteraly a section in our documentation just for these API

function isString(arg) {
  return typeof arg === 'string';
}

It's moving to runtime deprecation so no work from package maintainers required.
I dont like the idea to keep those like in a museum, I would agree if it was something that actually had some utility, saw some use or the ecosystem is based on them (like Domain)

I dont like the idea to keep those like in a museum

But why? Is it just out of principle? Are you on board with causing some (tiny) amount of pain to our users just for a question of principle? It seems to me we would need some other justification for that.

It's moving to runtime deprecation so no work from package maintainers required.

As soon as people start seeing a runtime deprecation, they will ask package maintainers to handle it.

I dont like the idea to keep those like in a museum

We're not keeping them like in a museum. We're keeping them to avoid "breaking" people for little benefit. That's also why the Buffer constructor is still not runtime-deprecated.