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

feat: adds pkg sort command #7954

Draft
wants to merge 1 commit into
base: latest
Choose a base branch
from
Draft

feat: adds pkg sort command #7954

wants to merge 1 commit into from

Conversation

reggi
Copy link
Contributor

@reggi reggi commented Dec 2, 2024

adds npm pkg sort command to sort package.json fields

@reggi reggi requested a review from a team as a code owner December 2, 2024 20:48
@ljharb
Copy link
Contributor

ljharb commented Dec 2, 2024

whoa, this is a large feature. how is it sorting things? how can the order be customized? was there an RFC or public discussion anywhere?

@reggi
Copy link
Contributor Author

reggi commented Dec 2, 2024

@ljharb
Copy link
Contributor

ljharb commented Dec 2, 2024

ok, so it doesn't seem like this has any customization capability, but many hundreds of thousands of projects likely have orderings this would break:

  • fake comment sections where precise placement is critical (eg "//": {})
  • main-adjacent fields, like module, jsnext:main, sideEffects; any package using yargs.config to configure itself; etc
  • why is engines not precisely next to devEngines by default? that makes no sense
  • never heard of licenses but why isn't it right next to license? that also makes no sense
  • isPrivate is the most important field in there imo, and should go at the top, above name/version

and i could go on for pages. iow, by shipping this, even opt-in, without customizability, npm will be opening the floodgates of custom ordering requests (i speak from experience maintaining multiple eslint plugins that handle orderings).

I strongly urge caution and slowness with a feature like this.

@reggi reggi marked this pull request as draft December 2, 2024 21:10
@wraithgar
Copy link
Member

@ljharb this is in draft status. Many of these questions would be answered by the docs that we have yet to add to the PR.

@ljharb
Copy link
Contributor

ljharb commented Dec 2, 2024

fair, it wasn't in draft until after my comments :-) nothing in the code suggests that it's customizable tho, which seems like a requirement to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants