Skip to content

Commit

Permalink
[infra] Configure eslint import/no-extraneous-dependencies rule (#3114)
Browse files Browse the repository at this point in the history
Configures a new eslint rule which enforces that, when we import a module, that module is referenced in the "dependencies" of the immediate "package.json" file.

Docs: https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/no-extraneous-dependencies.md

This is especially important now that we're using npm workspaces, because npm workspaces hoists as many dependencies as possible to the root `node_modules/` folder, which can make it easy to forget to include a dependency.

Exceptions are made for internal files like tests, which are free to import dependencies that are only declared in the top-level "package.json".

### Violations

- `@lit-labs/cli` was missing a dependency on `chalk`. Added the missing dependency.
- `@lit-labs/cli` depended on `@lit-labs/analyzer` as `devDependency`. Switched to regular `dependency`.
- `@lit-labs/react` imports `react`, which is in `devDependencies`. However, this is intentional, because it's a type-only import. Added a special-case to allow it to be imported from `devDependencies`.

### Also

- Found a few more spots that were referencing `npm run bootstrap`.
  • Loading branch information
aomarks committed Jul 1, 2022
1 parent fc3155d commit c93a8ee
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 29 deletions.
2 changes: 2 additions & 0 deletions .changeset/fifty-oranges-marry.md
@@ -0,0 +1,2 @@
---
---
38 changes: 38 additions & 0 deletions .eslintrc.json
Expand Up @@ -31,6 +31,21 @@
{
"argsIgnorePattern": "^_"
}
],
// Enforces that when a module is imported, that module is listed in the
// package's immediate "dependencies". Note internal-only files are excluded
// in an "overrides" configuration below.
"import/no-extraneous-dependencies": [
"error",
{
// These modules are also allowed to import from their immediate
// "devDependencies".
"devDependencies": [
// This package has a type-only import of "react", but it's not a
// dependency.
"packages/labs/react/**"
]
}
]
},
"overrides": [
Expand Down Expand Up @@ -61,6 +76,29 @@
"rules": {
"@typescript-eslint/no-explicit-any": "off"
}
},
{
// Files listed here don't need to declare their imports in their
// immediate package.json "dependencies". This should include all
// internal-only files, like tests, which are allowed to import from
// "devDependencies", and also from packages declared only in the root
// monorepo "package.json".
"files": [
"**/goldens/**",
"**/rollup.config.js",
"**/src/test-gen/**",
"**/src/test/**",
"**/src/tests/**",
"**/test-output/**",
"**/web-test-runner.config.js",
"packages/benchmarks/**",
"packages/labs/ssr/src/demo/**",
"packages/tests/**",
"rollup-common.js"
],
"rules": {
"import/no-extraneous-dependencies": "off"
}
}
]
}
3 changes: 1 addition & 2 deletions README.md
Expand Up @@ -75,8 +75,7 @@ Initialize repo:
```sh
git clone https://github.com/lit/lit.git
cd lit
npm install
npm run bootstrap
npm ci
```

Build all packages:
Expand Down
60 changes: 40 additions & 20 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -31,7 +31,7 @@
"changeset": "changeset",
"version": "npm run changeset version && npm run update-version-vars",
"update-version-vars": "node scripts/update-version-variables.js",
"release": "npm run bootstrap && npm run build && npm run changeset publish"
"release": "npm run build && npm run changeset publish"
},
"wireit": {
"build": {
Expand Down
5 changes: 2 additions & 3 deletions packages/benchmarks/README.md
Expand Up @@ -4,10 +4,9 @@ Benchmarks for lit-html and LitElement.

```bash
git clone git@github.com:lit/lit.git
cd lit-html
cd lit

npm install
npm run bootstrap
npm ci
npm run build

cd packages/benchmarks
Expand Down
7 changes: 4 additions & 3 deletions packages/labs/cli/package.json
Expand Up @@ -76,15 +76,17 @@
}
},
"dependencies": {
"@lit/localize-tools": "^0.6.1",
"@lit-labs/analyzer": "^0.2.0",
"@lit-labs/gen-utils": "^0.0.1",
"@lit/localize-tools": "^0.6.1",
"chalk": "^5.0.1",
"command-line-args": "^5.2.1",
"command-line-commands": "^3.0.2",
"command-line-usage": "^6.1.1",
"tslib": "^1.14.1"
},
"devDependencies": {
"@lit-labs/analyzer": "^0.2.0",
"@lit-internal/tests": "^0.0.0",
"@types/command-line-args": "^5.2.0",
"@types/command-line-commands": "^2.0.1",
"@types/command-line-usage": "^5.0.2",
Expand All @@ -93,7 +95,6 @@
"eslint": "^5.16.0",
"globby": "^10.0.2",
"stdout-stderr": "^0.1.13",
"@lit-internal/tests": "^0.0.0",
"typescript": "~4.6.2",
"uvu": "^0.5.3"
},
Expand Down

0 comments on commit c93a8ee

Please sign in to comment.