X Tutup
The Wayback Machine - https://web.archive.org/web/20220423200451/https://github.com/nodejs/node/pull/35742
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

stabilize packages features #35742

Closed
wants to merge 1 commit into from

Conversation

Copy link
Member

@MylesBorins MylesBorins commented Oct 21, 2020

These features are being used in production and are ready to be
considered stable.

Refs: nodejs/modules#564

Depends on #35741

/cc @nodejs/modules

@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented Oct 21, 2020

Review requested:

@nodejs-github-bot nodejs-github-bot added the doc label Oct 21, 2020

When a package has an [`"exports"`][] field, this will take precedence over the
`"main"` field when importing the package by name.

Copy link
Member

@GeoffreyBooth GeoffreyBooth Oct 21, 2020

Choose a reason for hiding this comment

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

I think @guybedford deliberately wanted this below "exports" because we want people to be preferring "exports" over "main".

Copy link
Member Author

@MylesBorins MylesBorins Oct 21, 2020

Choose a reason for hiding this comment

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

the ordering is changed in #35741

I can update it there

Copy link
Contributor

@guybedford guybedford Oct 21, 2020

Choose a reason for hiding this comment

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

I'm not tied to the ordering, it just a suggestion previously since there was no explicit ordering (it not being alphabetical).

Copy link
Member

@Trott Trott Oct 22, 2020

Choose a reason for hiding this comment

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

Maybe the list on L825 can have its ordering stated ("In order of precedence, the possible entries are:") while the headers can be in alphabetical order?

doc/api/packages.md Show resolved Hide resolved
@codecov-io

This comment has been minimized.

Copy link
Member

@mcollina mcollina left a comment

lgtm

@Trott
Copy link
Member

@Trott Trott commented Oct 22, 2020

This probably shouldn't land before the runtime deprecation in #35747 because otherwise that runtime deprecation is semver-major?

@Trott Trott added the blocked label Oct 22, 2020
doc/api/packages.md Outdated Show resolved Hide resolved
@MylesBorins
Copy link
Member Author

@MylesBorins MylesBorins commented Oct 22, 2020

@Trott the folder pathing stuff is currently no longer documented and unaffected by this change

These features are being used in production and are ready to be
considered stable.

Refs: nodejs/modules#564
@MylesBorins MylesBorins removed the blocked label Oct 26, 2020
@MylesBorins
Copy link
Member Author

@MylesBorins MylesBorins commented Oct 26, 2020

@nodejs/modules I would like to land this. If there are no objections within 24 hours I will land it tomorrow.

@Trott I removed your blocked label as I don't believe the feature you pointed out is affected by this change. Please let me know if you think I'm mistaken

@MylesBorins
Copy link
Member Author

@MylesBorins MylesBorins commented Oct 29, 2020

Landed in acdfc16

@MylesBorins MylesBorins deleted the mark-features-stable branch Oct 29, 2020
MylesBorins added a commit that referenced this issue Oct 29, 2020
These features are being used in production and are ready to be
considered stable.

Refs: nodejs/modules#564

PR-URL: #35742
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos pushed a commit that referenced this issue Nov 3, 2020
These features are being used in production and are ready to be
considered stable.

Refs: nodejs/modules#564

PR-URL: #35742
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@targos targos mentioned this pull request Nov 3, 2020
@guybedford guybedford mentioned this pull request Nov 23, 2020
3 tasks
@DerekNonGeneric DerekNonGeneric added the dont-land-on-v14.x label Nov 24, 2020
@MylesBorins MylesBorins added baking-for-lts and removed dont-land-on-v14.x labels Nov 24, 2020
BethGriggs pushed a commit that referenced this issue Dec 8, 2020
These features are being used in production and are ready to be
considered stable.

Refs: nodejs/modules#564

PR-URL: #35742
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
danielleadams pushed a commit that referenced this issue Apr 29, 2021
These features are being used in production and are ready to be
considered stable.

Refs: nodejs/modules#564

PR-URL: #35742
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ujjwal Sharma <ryzokuken@disroot.org>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@danielleadams danielleadams mentioned this pull request May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
baking-for-lts doc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

X Tutup