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: Prefer package.yaml (or .json5) to package.json when both are present #5541

Open
1 task done
qyurila opened this issue Oct 23, 2022 · 6 comments
Open
1 task done

Comments

@qyurila
Copy link

qyurila commented Oct 23, 2022

Describe the user story

pnpm supports package.yaml and package.json5 since #1799. But, there are a bunch of npm packages that directly read package.json file, hence cannot be used without additional package.json file.

I think, as an end user, the best way to handle this issue is using some monitoring tools and/or converters to automatically convert package.yaml to package.json in order to allow some packages to find it (and we'd probably want to add package.json in .gitignore). In my opinion, it's barely possible to hope every single packages there to support alternative manifest files because of its little popularity (and the presence of YAML haters).

However, this workaround is currently not very doable in the real life because pnpm tries to read and write package.json file when it's found, regardless package.yaml is at the same place or not. It makes an additional json-to-yaml converter to be required and makes the workaround very complicate.

Describe the solution you'd like

Make @pnpm/read-importer-manifest and @pnpm/write-importer-manifest to find package.yaml and package.json5 before package.json, so that this alternative manifest files are prefered to package.json, which means package.json would be only read by some packages and package.yaml would be modified and read most of the time.

Describe the drawbacks of your solution

I think most of the project which has both of package.yaml and package.json would eventually benefit by this change, but it can introduce breaking change and unexpected behavior.

Describe alternatives you've considered

Adding --manifest-ext (json|yaml|json5) option to pnpm CLI: It'd be inconvenient because it probably should be followed to every pnpm commands.
Adding manifestExt option to pnpm-workspace.yaml: Seems not bad at least for me, but I'm not sure if this is proper config file to have this kind of option.

@gusbemacbe
Copy link

Pryvit i dobroho ranku, @zkochan!

Please, let me create a YAML file with a pnpm command. I prefer YAML to JSON or JSON5.

@bmulholland
Copy link

FWIW, we're migrating to pnpm specifically for JSON5 support, but this issue prevents us from even using workarounds of compiling package.json for everything that needs it. We're holding off on our migration until then.

@gusbemacbe
Copy link

Dobryi den', @zkochan!

Any news about this choice of YAML over JSON?

@rlidwka
Copy link

rlidwka commented Dec 13, 2023

FWIW, we're migrating to pnpm specifically for JSON5 support, but this issue prevents us from even using workarounds of compiling package.json for everything that needs it.

@bmulholland, what other tools are you using that already support it?

@rlidwka
Copy link

rlidwka commented Dec 13, 2023

When both files (package.json5 and package.json) are present, it is very likely that:

  • either package.json is directly compiled from package.json5
  • or package.json is just a stub for tools that don't support json5

So preferring package.json5 is the right choice from usability standpoint.

Are you're worrying about performance for the extra .stat call? If that's the case, perhaps we should make some effort to change other tools to support json5, so package.json won't ever be needed.

@bmulholland
Copy link

bmulholland commented Dec 13, 2023

@rlidwka We have a package.json5 that is really only used by us, developers. I wrote a short script that compiled the JSON5 to package.json. That's the only tool that really uses or touches it. It adds a step to any change we make to our package.json(5), but it's well worth it -- the whole thing is probably 25% comments and they're all so useful.

FYI: It also means that we can't really use version pinning in our package.json, at least if we use Dependabot upgrades, which goes against the grain of the JS community. That's alright with us, we set version pins to "latest," which is IMHO what all applications (i.e. not packages others depends on) should use. Different discussion, point is that it works for us but has blockers for most people following more standard conventions.

Here's our scripts entry that compiled package.json5 to package.json, complete with explanation and references... feel free to use the script.

    // See https://github.com/nodejs/node/issues/40714
    // Our json5 is sorted sanely, while yarn sometimes sorts it alphabetically
    // (e.g. during upgrades), so the sort package makes it consistent between
    // the json5 conversion and the upgrade-sorting
    // The echo adds a newline to the end of the file; the preceding commands
    // don't, and without it yarn upgrade makes a change to the file to add it
    "compile-package-json": "json5 -o package.json --space 2 package.json5 && npx sort-package-json && echo '' >> package.json",

I think this also makes the point about why comments in these files are useful!

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

4 participants