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

[RRFC] format package.json based on config values #423

Closed
lukekarrys opened this issue Jul 29, 2021 · 10 comments
Closed

[RRFC] format package.json based on config values #423

lukekarrys opened this issue Jul 29, 2021 · 10 comments

Comments

@lukekarrys
Copy link
Contributor

Motivation ("The Why")

Allow package.json to be formatted according to a set of config values. This will establish default config values for how package.json is written and add a command to rewrite an existing package.json files based on those config values. Since these config values will live in an .npmrc file, it will allow for users to format their package.json differently per project or globally.

Example

# New subcommand in `npm pkg` to format `package.json` based on config values
npm pkg format [--dry-run]

# Any command that writes to `package.json` will do so based on the config values
# All of these could update any part of `package.json` if the relevant configs are set
npm install foo
npm pkg set foo="bar
npm init --yes

How

In the initial implementation, default config values will be set to mirror current behavior where possible, with the goal being that formatting during commands that write to package.json will not be a breaking change.

Sorting will be done via localeCompare.

Configs

  • pkg-format-indent
    • Specifies how the file will be indented. If the value is an integer, spaces will be used.
    • Defaults to current (current|n|tabs)
    • Defaulting to current is new behavior based on the discussion in [RRFC] Don't expand tabs by default #420. To not have this change any behavior, the default would be 2.
  • pkg-format-eol
    • Specifies what the line endings in the file will be
    • Defaults to current (current|lf|cr|crlf)
    • Defaulting to current is new behavior based on the discussion in [RRFC] Don't expand tabs by default #420. To not have this change any behavior, the default would be lf.
  • pkg-format-sort-children
    • Specifies which top level items will have their children sorted
    • If the item is an object, it will be sorted by key. If the item is an array of strings, it will be sorted by string. Otherwise it will be ignored.
    • Defaults to dependencies,devDependencies,peerDependencies,optionalDependencies
  • pkg-format-keys-start
    • Specifies a list of top level keys that will always be moved to the start of package.json
    • Defaults to ``
  • pkg-format-keys-end
    • Specifies a list of top level keys that will always be moved to the end of package.json
    • Defaults to ``
  • pkg-format-keys-sort
    • Specifies whether other top level keys not specified by keys-start and keys-end will be sorted
    • Defaults to false (true|false)

Open Questions

  • How should the presence of indent_style|indent_size and end_of_line from EditorConfig affect the comparable config values? The EditorConfig spec relies on walking the directory tree to get the values.
  • Should pkg-format-indent and pkg-format-eol be applied to package-lock.json as well?

References

  • This should cover the use cases mentioned in [RRFC] Don't expand tabs by default #420
  • Prior art: fixpack
    • This RFC covers only the configs necessary to format a package.json file. fixpack also will warn/error on missing top level keys, which is not covered by this RFC.
@isaacs
Copy link
Contributor

isaacs commented Aug 4, 2021

Any reason why this is specific to pkg and not just json? It seems surprising that package-lock.json would use a different setting than package.json, or the output from npm <cmd> --json.

Some other prior art (the module we use to organize package-lock.json): https://github.com/isaacs/json-stringify-nice

Note that changes to the defaults, if it causes churn in the git history, will be cause for much user outrage!

@lukekarrys
Copy link
Contributor Author

Any reason why this is specific to pkg and not just json?

I see value in eol and indent settings applying to all json output, and the rest I think make sense as pkg only. I think if that's the case, it makes sense to split the naming of the settings to pkg-format-* and json-format-*.

@darcyclarke darcyclarke added the Agenda will be discussed at the Open RFC call label Aug 16, 2021
@lukekarrys
Copy link
Contributor Author

Note that changes to the defaults, if it causes churn in the git history, will be cause for much user outrage!

Yes, the goal is to have no churn in git history when running any commands unless config values have been changed. For eol and indent specifically I think the following should work in a minor version:

  1. Use config item if it exists
  2. If no config item exists, use whatever is currently in package.json
  3. If no package.json exists (eg npm init) and no config item exists, try .editorconfig
  4. Otherwise, use default values ('\n' and 2)

@dgchrt
Copy link

dgchrt commented Aug 18, 2021

Note that changes to the defaults, if it causes churn in the git history, will be cause for much user outrage!

Changes to defaults should cause no churn, because they will not affect old files, only new files. Or do you see it differently?

@ljharb
Copy link
Contributor

ljharb commented Aug 18, 2021

@diogoeichert defaults include "editing package.json when you run npm install", so it would definitely affect old files.

@dgchrt
Copy link

dgchrt commented Aug 18, 2021

@diogoeichert defaults include "editing package.json when you run npm install", so it would definitely affect old files.

Oh, I see. I think @lukekarrys' proposal would prevent that, though:

  1. Use config item if it exists
  2. If no config item exists, use whatever is currently in package.json
  3. If no package.json exists (eg npm init) and no config item exists, try .editorconfig
  4. Otherwise, use default values ('\n' and 2)

@isaacs
Copy link
Contributor

isaacs commented Aug 18, 2021

Summarizing discussion:

  1. when implicitly editing a lockfile or package.json, whatever is in the file is authoritative.
    • this prevents churn when two users have different configs, so we don't have a diff war
  2. npm pkg format will rewrite the file based on the configs it loads.
  3. new files created use the configs.

Getting the configs:

  1. If indent/eol are set explicitly (ie, npm.config.find('eol') !== 'defaults'), use that.
  2. If not, use .editorconfig if it exists.
  3. If it doesn't exist, use \n and 2.

@isaacs
Copy link
Contributor

isaacs commented Aug 18, 2021

  • Add a npm pkg format-validate or npm pkg format --dry-run or something to check whether it would be changed by npm pkg format

@ljharb
Copy link
Contributor

ljharb commented Aug 18, 2021

For "would be changed", the critical part is that it exits 0 when no changes, and exits nonzero when changes.

@darcyclarke darcyclarke removed the Agenda will be discussed at the Open RFC call label Aug 25, 2021
@lukekarrys
Copy link
Contributor Author

This is now being tracked in npm/statusboard#384.

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

No branches or pull requests

5 participants