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

Add an eslint rule to forbid adding devDependencies in packages #32390

Closed
simison opened this issue Apr 18, 2019 · 5 comments
Closed

Add an eslint rule to forbid adding devDependencies in packages #32390

simison opened this issue Apr 18, 2019 · 5 comments

Comments

@simison
Copy link
Member

simison commented Apr 18, 2019

If adding devDependencies to package.json in packages/apps can cause trouble (see e.g. #32383), we'll need an Eslint rule to stop folks doing this.

Monorepo docs say:

The only exception are devDependencies which must be declared in the wp-calypso root package.json. devDependencies of sub-packages in a monorepo are not reliably installed and cannot be relied on.

Note also conflicting import/no-extraneous-dependencies rule in packages (#32294 (comment))

FYI @Automattic/team-calypso

@griffbrad
Copy link
Contributor

I'll give this a shot. Looks like the no-extraneous-dependencies rule is actually a decent model for what we need.

@stale
Copy link

stale bot commented Jan 13, 2020

This issue has been marked as stale and will be closed in seven days. This happened because:

  • It has been inactive in the past 9 months.
  • It isn't a project or a milestone, and hasn’t been labeled `[Pri] Blocker`, `[Pri] High`, `[Status] Keep Open`, or `OSS Citizen`.

You can keep the issue open by adding a comment. If you do, please provide additional context and explain why you’d like it to remain open. You can also close the issue yourself — if you do, please add a brief explanation.

@stale stale bot added the [Status] Stale label Jan 13, 2020
@blowery
Copy link
Contributor

blowery commented Jan 13, 2020

I think we're still working on this. Should be made easier / proper by #38190

@stale stale bot removed the [Status] Stale label Jan 13, 2020
@jsnajdr
Copy link
Member

jsnajdr commented Jan 14, 2020

Big part of this issue was solved in #36534 by @simison himself when he added the npm-package-json-lint linter and enabled its prefer-no-devDependencies rule. But the config still allows adding dev depenencies to the apps/*/package.json files.

I pushed a commit to #38190 (its sha is ae80bab as I write this, but will change after the next rebase) that modifies the linter config to allow dev dependencies only in the root package.json.

@scinos
Copy link
Contributor

scinos commented Dec 21, 2020

The original issue was raised when we were using lerna + npm. Since then we have moved to yarn and reduced our usage of lerna.

I do think having packages with devDependencies is a good thing and helps having a sane dependency tree without a bloated root package and packages relying on undeclared dependencies.

I'm closing this for now, if anybody has strong opinions about it we can revisit.

@scinos scinos closed this as completed Dec 21, 2020
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

6 participants