init-package-json icon indicating copy to clipboard operation
init-package-json copied to clipboard

feat: explicitly include type

Open benmccann opened this issue 1 year ago • 5 comments

I've read through several issues in both npm/cli and npm/rfcs and there seems to be universal agreement that adding "type": "commonjs" would be an improvement as it's better to explicit than implicit both for understandability and to improve performance by eliminating the need to go down detection code paths.

Several people have also suggested prompting for the type field and that is much more controversial and has been decided against, so I have not done that.

References

https://github.com/npm/rfcs/discussions/75#discussioncomment-11943

From https://nodejs.org/en/blog/release/v20.10.0:

Detection increases startup time, so we encourage everyone—especially package authors—to add a type field to package.json, even for the default "type": "commonjs".

https://publint.dev/ and the publint package also validate that the type field is present and warn package authors if it is not. It would be best to create a package.json without warnings out of the box: https://github.com/bluwy/publint/blob/master/site/rules.md#use_type

benmccann avatar Sep 18 '24 16:09 benmccann

This probably does need a re-evaluation in light of the node 20 behavior.

I don't think it's the best idea to have a value set that the user can't control at least through config. (This comment touches on that, correctly I think).

It's been quite awhile since I looked under the hood on this repo. Would this PR allow npm to pass this in as a defined value (again, not from a prompt but from the --init- config namespace).

wraithgar avatar Sep 24 '24 18:09 wraithgar

I'm not quite familiar with the --init- config namespace you're talking about, but this PR won't prevent other values from being set as it only defaults to commonjs if another value is not provided

benmccann avatar Sep 24 '24 19:09 benmccann

These are the configs that npm can set, starting at https://docs.npmjs.com/cli/v10/using-npm/config#init-author-email.

The tracking issue for adding the new config is at https://github.com/npm/statusboard/issues/654

wraithgar avatar Sep 25 '24 15:09 wraithgar

I'm adding this to the npm 11 roadmap along with the config changes.

wraithgar avatar Sep 25 '24 16:09 wraithgar

Maybe a CC to @arcanis (Yarn) and @zkochan (pnpm) would be good here too?

Feels like a big change in default npm init behavior, and users will probably ask other package managers to follow.

karlhorky avatar Oct 01 '24 15:10 karlhorky

@benmccann Thank you so much for your contribution here and helping us get started on this work. Unfortunately, the implementation needs to happen in the default-input.js file where we have access to the config, which allows us to utilize npm init config settings to incorporate things like flags and .npmrc values. This was a great forcing function and jumping-off point for this work though, and I really appreciate it. 🙏

If you'd like to checkout my revision it's here https://github.com/npm/init-package-json/pull/313

reggi avatar Nov 22 '24 20:11 reggi