search

Thinkmill / manypkg

☔️ An umbrella for your monorepo
MIT License
911 stars 49 forks source link

Should there be an option to force devDependencies in root package.json? #233

Open VanTanev opened 3 weeks ago

VanTanev commented 3 weeks ago

I think that there is a valid case to be made that manypkg should optionally enforce all dependencies be in devDependencies in the root.

We have a monorepo that needs to build docker containers. It has separate pnpm installs for the build step and the production step.

When it builds the production step, it uses turbo prune and then pnpm install --prod - this will unfortunately also install all monorepo management tools defined in the root package.json#dependencies, needlessly increasing the docker image size.

Forcing root to use devDependencies instead would solve for this, and to some degree, I think it makes more intuitive sense if stuff like typescript, prettier, lefthook etc are dev dependencies instead of prod dependencies.

If there is interest from maintainers and guidance on desired implementation, I would be happy to contribute a PR.

Andarist commented 3 weeks ago

Do you want to be able to mix root dependencies and devDependencies in the root? Or would you like to enforce the root to use devDependencies instead of dependencies?

For reference, we currently have this rule

VanTanev commented 3 weeks ago

I would like to force all dependencies to be devDependencies.

Andarist commented 3 weeks ago

this change would make sense to me, cc @emmatown - any objections?

VanTanev commented 1 week ago

I still want to champion this change, I think it would be a positive one.

Andarist commented 1 week ago

I think it’s worth doing - so if you want to prepare a PR for it: let’s do it

VanTanev commented 1 week ago

Just to make sure, the PR will switch the check to prefer devDependencies instead of prod ones. It will be a breaking change. There won't be an option to go back to the old behavior.

Alternatively, we could put this behind an option and set devDependencies as the new default, but I prefer the first approach.

Thoughts?

Andarist commented 1 week ago

I prefer the first option too.

kachkaev commented 16 hours ago

I like this change. We use Renovate bot and allow it to update devDependencies without human reviews. PRs are auto-merged as long as CI passes. With ROOT_HAS_DEV_DEPENDENCIES rule, we have to manually update global linters like Prettier and CSpell, which is OK but not perfect. I’m glad that this won’t be needed once #236 is released!