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

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

Open
VanTanev opened this issue Oct 16, 2024 · 8 comments · May be fixed by #236
Open

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

VanTanev opened this issue Oct 16, 2024 · 8 comments · May be fixed by #236

Comments

@VanTanev
Copy link
Contributor

VanTanev commented Oct 16, 2024

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
Copy link
Collaborator

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
Copy link
Contributor Author

I would like to force all dependencies to be devDependencies.

@Andarist
Copy link
Collaborator

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

@VanTanev
Copy link
Contributor Author

VanTanev commented Nov 1, 2024

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

@Andarist
Copy link
Collaborator

Andarist commented Nov 1, 2024

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

@VanTanev
Copy link
Contributor Author

VanTanev commented Nov 1, 2024

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
Copy link
Collaborator

Andarist commented Nov 1, 2024

I prefer the first option too.

@kachkaev
Copy link

kachkaev commented Nov 8, 2024

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!

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

Successfully merging a pull request may close this issue.

3 participants