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

syncpack doesn't respect package.json 'repository' and 'bugs' fields #100

Closed
muravjev opened this issue Oct 27, 2022 · 5 comments
Closed

Comments

@muravjev
Copy link

Description

  1. Create package.json with extended fields repository and bugs as in:
  "repository": {
    "type": "git",
    "url": "https://github.com/muravjev/reprl_synpack_format_bug.git",
    "directory": "packages/mydir"
  },
  "bugs": {
    "url": "https://github.com/muravjev/reprl_synpack_format_bug/issues"
  }
  1. Run syncpack format

  2. Syncpack changes repository and bugs fields to the following values:

  "repository": "muravjev/reprl_synpack_format_bug.git",
  "bugs": "https://github.com/muravjev/reprl_synpack_format_bug/issues"

Summary

Shortening of the bugs field is may be appropriate (as syntax sugar), though some people may not like it.

Shortening of the repository field, especially if it contains directory sub field, changes the final target url that someone would have liked to point to.

Note, that there cases when package is not in the root directory (for example if it is part of a monorepo) and in this case npmjs suggests use directory sub field to point to exact package location (npmjs doc)

Suggested Solution

I see that such shortening is a feature of syncpack, but in case of valid extended syntax as in example the modification of the url fields (repository and/or bugs) maybe undesirable.

I suggest:

  1. Add boolean flag shortenUrl to control such shortening behavior with default value true (to keep existing behavior as default).

  2. In case of repository and/or bugs fields has extra sub fields besides url, the shortening should not be undertook, even if shortenUrl flag is true.

OR

To avoid over complicating and adding extra flags:

  1. In case of repository and/or bugs fields has extra sub fields besides url, the shortening should not be undertook.

Note, that in either cases the changes will not affect existed package jsons that are already formatted with syncpack.

Help Needed

If you approve either approach, I would love to apply it.

BTW I have found the syncpack accidentally today, and this this is really great tool!
Thank you, @JamieMason for creating it! 👍👍👍

MRE

  1. Clone https://github.com/muravjev/reprl_synpack_format_bug
  2. Run pnpm repro
  3. Check the git changes

Environment

  System:
    OS: Windows 10 10.0.17763
    CPU: (8) x64 Intel(R) Core(TM) i7-3632QM CPU @ 2.20GHz
    Memory: 1.92 GB / 7.95 GB
  Binaries:
    Node: 18.5.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.19 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 8.12.1 - C:\Program Files\nodejs\npm.CMD
  Managers:
    pip3: 22.2.2 - C:\Program Files\Python310\Scripts\pip3.EXE
  Utilities:
    Git: 2.37.1.
  Virtualization:
    Docker: 20.10.12 - C:\Program Files\Docker\Docker\resources\bin\docker.EXE
  IDEs:
    VSCode: 1.72.2 - C:\Program Files\Microsoft VS Code\bin\code.CMD
    Visual Studio: 17.3.32825.248 (Visual Studio Enterprise 2022), 16.11.32106.194 (Visual Studio Enterprise�2019)
  Languages:
    Java: 1.8.0_321
    Python: 3.10.7
  Browsers:
    Internet Explorer: 11.0.17763.2989
@JamieMason
Copy link
Owner

Great report @muravjev, thanks. See also #91 which is quite similar. Will pick this up when I next work on syncpack.

@muravjev
Copy link
Author

muravjev commented Oct 28, 2022

Great report @muravjev, thanks. See also #91 which is quite similar. Will pick this up when I next work on syncpack.

Thank you! I haven't checked existing issues :(

JamieMason pushed a commit that referenced this issue Oct 28, 2022
@JamieMason
Copy link
Owner

The directory part is fixed in 8.2.5.

OR

To avoid over complicating and adding extra flags:

In case of repository and/or bugs fields has extra sub fields besides url, the shortening should not be undertook.
Note, that in either cases the changes will not affect existed package jsons that are already formatted with syncpack.

I think based on the above, this issue can be closed too. I don't think there'll be much demand for this to be configurable.

Thanks again @muravjev for your great input.

@muravjev
Copy link
Author

@JamieMason, Thank you, for your job. I'm fine with changes you've made!

P.S. Sorry to bother you, but just imagine if you stick with formatters approach, then sortAz would became also a formatter and default config could look like this:

formatters: [
  ["bugs", "repository"]: ["simplifyUrl", "shortenUrl"],
  ['dependencies', 'devDependencies', 'peerDependencies']: [sortAz]
]

Isn't it really cool? :)))

Implement any formatter you would like and it could be applied to any field in config.

May be some day, new requirement will appear for another field and this could be implemented :)

@JamieMason
Copy link
Owner

Thanks @muravjev.

The kinds of formatters needed haven't changed very much since working on syncpack, so it's not something I need to optimise for just now. If in the future I find I'm having to do a lot of manual work in this area though, I've got your idea in the back pocket I can refer to.

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

No branches or pull requests

2 participants