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

🐛 Bug: package-json/order-properties changes scripts keys #80

Closed
3 tasks done
AndreasLindbergPAF opened this issue Dec 8, 2023 · 8 comments · Fixed by #312
Closed
3 tasks done

🐛 Bug: package-json/order-properties changes scripts keys #80

AndreasLindbergPAF opened this issue Dec 8, 2023 · 8 comments · Fixed by #312
Labels
status: accepting prs Please, send a pull request to resolve this! type: bug Something isn't working :(

Comments

@AndreasLindbergPAF
Copy link

AndreasLindbergPAF commented Dec 8, 2023

Bug Report Checklist

  • I have tried restarting my IDE and the issue persists.
  • I have pulled the latest main branch of the repository.
  • I have searched for related issues and found none that matched my issue.

Expected

It should only sort the top-level keys

Actual

It complains about key order in deeper paths as well

Additional Info

With this config

{
  "files": ["package.json"],
  "plugins": ["package-json"],
  "rules": {
    "package-json/order-properties": [
      "error",
      {
        "order": [
          "name",
          "scripts",
          "dependencies"
        ]
      }
    ],
    "package-json/sort-collections": [
      "error", 
      ["dependencies", "devDependencies", "peerDependencies"]
    ]
  }
}

I'm expecting this

{
  "scripts": {
    "start": "",
    "dev": ""
  }
  "name": "",
  "dependencies": []
}

To only reorder on top-level, like this

{
  "name": "",
  "scripts": {
    "start": "",
    "dev": ""
  },
  "dependencies": []
}

But it seems to change scripts order as well

{
  "name": "",
  "scripts": {
    "dev": "",
    "start": ""
  },
  "dependencies": []
}

I'm stuck on ^0.1.5 because of this 😞

@AndreasLindbergPAF AndreasLindbergPAF added the type: bug Something isn't working :( label Dec 8, 2023
@JoshuaKGoldberg
Copy link
Owner

👋 thanks for filing! Could you provide a little more detail please?

  • What is your package.json before running with --fix?
  • What is your package.json after running with --fix?
  • Why don't you want the ordering to be changed the way it is?

@JoshuaKGoldberg JoshuaKGoldberg added the status: waiting for author Needs an action taken by the original poster label Dec 8, 2023
@AndreasLindbergPAF
Copy link
Author

AndreasLindbergPAF commented Dec 8, 2023

👋 thanks for filing! Could you provide a little more detail please?

  • What is your package.json before running with --fix?
  • What is your package.json after running with --fix?

Sorry about that, kind of rushed the issue creation a bit 😉
I've updated the post.

  • Why don't you want the ordering to be changed the way it is?

The rule package-json/order-properties is described as only re-ordering top-level keys.
But now is seem that it also acts as package-json/sort-collections even tho I'm not enabling it for scripts.

@JoshuaKGoldberg
Copy link
Owner

Got it! Thanks, that makes sense. Looks like this is an issue with the "order": "sort-package-json" logic added in #55. I'd defaulted it to 'standard-order' in #40 (#31 (comment)).

cc @kendallgassner as FYI 👋

@JoshuaKGoldberg JoshuaKGoldberg added status: accepting prs Please, send a pull request to resolve this! and removed status: waiting for author Needs an action taken by the original poster labels Dec 11, 2023
@JoshuaKGoldberg
Copy link
Owner

@all-contributors please add @AndreasLindbergPAF for bug.

🤖 Beep boop! This comment was added automatically by all-contributors-auto-action.
Not all contributions can be detected from Git & GitHub alone. Please comment any missing contribution types this bot missed.
...and of course, thank you for contributing! 💙

Copy link
Contributor

@JoshuaKGoldberg

I've put up a pull request to add @AndreasLindbergPAF! 🎉

I couldn't determine any contributions to add, did you specify any contributions?
Please make sure to use valid contribution names.

JoshuaKGoldberg added a commit that referenced this issue Jan 20, 2024
Adds @AndreasLindbergPAF as a contributor for bug.

This was requested by JoshuaKGoldberg [in this
comment](#80 (comment))

---------

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Co-authored-by: Josh Goldberg <[email protected]>
@hyoban
Copy link
Contributor

hyoban commented Apr 14, 2024

@JoshuaKGoldberg sort-package-json chooses to make it less configurable, so what method do you recommend to fix this problem?

  1. fork sort-package-json (I can help to maintain it and keep it synced with the source)
  2. auto-fix manually by using sort-object-keys and detect-indent

@JoshuaKGoldberg JoshuaKGoldberg changed the title 🐛 Bug: package-json/order-properties changes scripts keys 🚀 Feature: add option to package-json/order-properties to not change nested keys Apr 23, 2024
@JoshuaKGoldberg JoshuaKGoldberg changed the title 🚀 Feature: add option to package-json/order-properties to not change nested keys 🐛 Bug: package-json/order-properties changes scripts keys Apr 23, 2024
@JoshuaKGoldberg
Copy link
Owner

Hmm, I think the more manual strategy (2) is probably more sustainable long-term. I haven't looked deeply at which packages are best for that these days.

JoshuaKGoldberg pushed a commit that referenced this issue Apr 29, 2024
<!-- 👋 Hi, thanks for sending a PR to eslint-plugin-package-json! 💖.
Please fill out all fields below and make sure each item is true and [x]
checked.
Otherwise we may not be able to review your PR. -->

## PR Checklist

-   [x] Addresses an existing open issue: fixes #80 
- [x] That issue was marked as [`status: accepting
prs`](https://github.com/JoshuaKGoldberg/eslint-plugin-package-json/issues?q=is%3Aopen+is%3Aissue+label%3A%22status%3A+accepting+prs%22)
- [x] Steps in
[CONTRIBUTING.md](https://github.com/JoshuaKGoldberg/eslint-plugin-package-json/blob/main/.github/CONTRIBUTING.md)
were taken

## Overview

<!-- Description of what is changed and how the code change does that.
-->
Copy link

🎉 This is included in version v0.13.0 🎉

The release is available on:

Cheers! 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepting prs Please, send a pull request to resolve this! type: bug Something isn't working :(
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants