[v14.x backport] modules: deprecate module.parent

[aduh95]

Backport of #32217

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[BethGriggs]

ping @nodejs/modules-active-members, would appreciate some reviews before landing this in v14.x

[aduh95]

Let me rebase to make the diff more readable!

[ljharb]

LGTM

[aduh95]

This didn't make it to 14.5.0, is there something I can do to help it land in the next semver-minor?

[MylesBorins]

I'm about to head on vacation but can help bring this over the finish line after I get back in a week

[MylesBorins]

@nodejs/lts @nodejs/tsc

There was a conflict with this file because another deprecation landed first and the deprecation number this PR was planning to use (and what it uses on master). It dawned on me that we might have inconsistent dep numbers across release streams. Is this intentional? Is this what we want to see happen? Happy to just move forward but wanted to flag.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/32336/

[MylesBorins]

@nodejs/lts do we have a policy around landing deprecations like this? Should it come in a semver-minor/

[jkrems]

Looks good in general but I share Myles' confusion about how deprecation numbering is expected to work here. But that may be a larger issue that this backport.

[targos]

I'm generally in favor of backporting documentation-only deprecations as far back as possible.
I think deprecations are semver-minor by policy, but maybe I don't remember correctly.

[targos]

About numbering, my opinion is that we should have consistent numbers across all release lines (equal to the numbers assigned when the commit lands on the master branch). I think it's OK to skip deprecation numbers if something isn't backported.

[jkrems]

Did we already release conflicting deprecation numbers? If so, does it make sense to do a renumbering PR to each release line, potentially even dropping the numbers that may be ambiguous now (and having a footnote that they may refer to different deprecations, depending on where they have been seen)?

[richardlau]

I'm kind of surprised the deprecation numbers between 14.x and master got out of sync. Did we land a deprecation in 14.x that didn't land in master first?

[MylesBorins]

Adding the semver-minor label as per the original PR

[BethGriggs]

I'm kind of surprised the deprecation numbers between 14.x and master got out of sync. Did we land a deprecation in 14.x that didn't land in master first?

I believe so - #33126 (which was a slightly special case).

+1 to skipping numbers for consistency across release lines. I'm also leaning towards @jkrems suggestion of a PR to patch up the recent numbers.

I think it would make sense for Transform._transformState(#33126) to keep DEP0143, and deprecate module.parent to get bumped to the next available number as deprecate module.parent (#32217) has not gone out in a formal release yet (excluding nightlies).

[MylesBorins]

So after looking into it a bit more it seems like the deprecation number issue we are running into is because the original PR assigned a number instead of waiting for the release to assign the number, this is why we have two number assigned.

[richardlau]

Deprecation numbers are supposed to be assigned when the PR lands: https://github.com/nodejs/node/blob/master/doc/guides/releases.md#step-3-update-any-replaceme-and-dep00xx-tags-in-the-docs

[MylesBorins]

@richardlau that step in the release guide is saying to replace DEP00XX with a deprecation code during the release. The code is not supposed to be assigned until that point

[richardlau]

@MylesBorins it says

The code must be assigned a number (e.g. DEP0012). This assignment should occur when the PR is landed, but a check will be made when the release build is run.

We have definitely been assigning the codes directly on the master branch and not at release time, e.g.

• doc: assign missing deprecation code #33109
• doc,src,test: assign missing deprecation code #31674
• doc: add missing deprecation code #29820
• worker: assign missing deprecation code #28395

[MylesBorins]

I guess I'm just not up to date on the process changes that have happened. Not a huge deal.

Seems like not assigning until release and then updating on master is a good way to ensure codes stay in sync, but understand why we may not want to do that.

Don't really have strong feelings here.

[MylesBorins]

landed in 8795ee5