-
Notifications
You must be signed in to change notification settings - Fork 3.3k
chore: Eslint migration foundation + migrate npm/grep package #32046
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
Changes from 7 commits
045ea2e
028dc0f
923f770
8c27e00
26df718
ecb9b16
bec2718
4d0e588
abb7d13
dc67077
09a88ac
8a8a7c3
b7e6630
5f2ed6b
3488c40
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,311 @@ | ||
# ESLint Migration Guide: Monorepo Alignment | ||
|
||
## Migration Checklist | ||
|
||
### Batch 1: Small npm utilities | ||
- [x] npm/grep | ||
- [ ] npm/puppeteer | ||
- [ ] npm/mount-utils | ||
- [ ] npm/cypress-schematic | ||
|
||
### Batch 2: Framework adapters | ||
- [ ] npm/react | ||
- [ ] npm/vue | ||
- [ ] npm/svelte | ||
|
||
### Batch 3: Build-related | ||
- [ ] npm/webpack-batteries-included-preprocessor | ||
- [ ] npm/webpack-preprocessor | ||
- [ ] npm/webpack-dev-server | ||
- [ ] npm/vite-plugin-cypress-esm | ||
- [ ] npm/vite-dev-server | ||
|
||
### Batch 4a: Core packages (part 1) | ||
- [ ] packages/frontend-shared | ||
- [ ] packages/icons | ||
- [ ] packages/launcher | ||
- [ ] packages/https-proxy | ||
- [ ] packages/proxy | ||
- [ ] packages/net-stubbing | ||
- [ ] packages/driver | ||
- [ ] packages/rewriter | ||
- [ ] packages/reporter | ||
- [ ] packages/server | ||
|
||
### Batch 4b: Core packages (part 2) | ||
- [ ] packages/runner | ||
- [ ] packages/extension | ||
- [ ] packages/graphql | ||
- [ ] packages/network | ||
- [ ] packages/socket | ||
- [ ] packages/telemetry | ||
- [ ] packages/launchpad | ||
- [ ] packages/errors | ||
- [ ] packages/data-context | ||
- [ ] packages/app | ||
|
||
### Batch 4c: Core packages (part 3) | ||
- [ ] packages/config | ||
- [ ] packages/root | ||
- [ ] packages/resolve-dist | ||
- [ ] packages/packherd-require | ||
- [ ] packages/v8-snapshot-require | ||
- [ ] packages/web-config | ||
- [ ] packages/types | ||
- [ ] packages/example | ||
|
||
### Batch 5: Test and script folders | ||
- [ ] system-tests | ||
- [ ] scripts | ||
|
||
--- | ||
|
||
This guide describes how to migrate all packages in the Cypress monorepo to the new unified ESLint configuration (`@packages/eslint-config`). | ||
|
||
--- | ||
|
||
## Why Migrate? | ||
- **Consistency:** Enforce the same linting rules and plugins across all packages. | ||
- **Simplicity:** Remove the need for custom plugins like `@cypress/eslint-plugin-dev`. | ||
- **Maintainability:** Make it easier to update and maintain lint rules. | ||
- **Modernization:** Replace obsolete TSLint with the appropriate ESLint TypeScript plugin (`@typescript-eslint`). | ||
|
||
--- | ||
|
||
## Migration Strategy | ||
|
||
### 1. **Batch Packages for Migration** | ||
- **Batch by directory/type** to keep PRs manageable and reduce risk. | ||
- **Batch size:** 4–8 packages per PR, grouped by similarity. | ||
|
||
### 2. **Migration Steps for Each Package** | ||
|
||
For each package in the batch: | ||
|
||
1. **Remove old ESLint config and plugin references:** | ||
- Delete `.eslintrc`, `.eslintrc.json`, or `.eslintrc.js` in the package. | ||
- Remove any references to `@cypress/eslint-plugin-dev` in `package.json` (if present). | ||
- **Remove TSLint configs:** Delete `tslint.json` and remove `tslint` dependencies from `package.json`. | ||
2. **Add a new ESLint config file:** | ||
- Create `eslint.config.ts` in the package root: | ||
```ts | ||
import baseConfig from '@packages/eslint-config' | ||
export default baseConfig | ||
``` | ||
3. **Ensure dependencies are up to date:** | ||
- Remove any package-local ESLint plugins now provided by the shared config. | ||
- Remove TSLint-related dependencies (the new config includes `@typescript-eslint`). | ||
- **Add the shared ESLint config as a dev dependency:** | ||
- In each migrated package, add `@packages/eslint-config` to `devDependencies` (use a relative file path if not published to npm). | ||
- **Add ESLint as a dev dependency:** | ||
- Since `@packages/eslint-config` has ESLint as a peer dependency, add `eslint: "^9.18.0"` to `devDependencies`. | ||
4. **Run lint and autofix:** | ||
- From the package root, run: | ||
``` | ||
npx eslint . --ext .js,.ts,.tsx,.jsx --fix | ||
``` | ||
- Manually fix any remaining lint errors. | ||
5. **Verify TypeScript configuration:** | ||
- Ensure the package has a valid `tsconfig.json` that works with the new ESLint config. | ||
- Run `npx tsc --noEmit` to check for TypeScript compilation errors. | ||
- Verify that the new ESLint config can properly parse TypeScript files in the package. | ||
6. **Run tests for the package** to ensure nothing broke. | ||
7. **Commit changes** with a clear message, e.g.: | ||
``` | ||
chore(npm/grep): migrate to @packages/eslint-config and remove legacy eslint-plugin-dev | ||
``` | ||
|
||
### 3. **Open a PR for Each Batch** | ||
- Keep each migration PR focused (one batch per PR). | ||
- List all affected packages in the PR description. | ||
- Include a checklist for each package: | ||
- [ ] Removed old ESLint config | ||
- [ ] Added new config | ||
- [ ] Ran lint and fixed errors | ||
- [ ] Ran tests | ||
|
||
### 4. **Document Issues or Gaps** | ||
- If you hit any missing rules or plugin gaps, note them for follow-up. | ||
- If a package needs a custom override, add it in a local `eslint.config.ts` (prefer to upstream to the shared config if possible). | ||
|
||
### 5. **Deprecate and Remove Old Plugin** | ||
- Once all packages are migrated, remove `@cypress/eslint-plugin-dev` from the repo and CI. | ||
|
||
### 6. **Update Lerna/Monorepo Config** | ||
- Ensure all packages reference the new config in their `package.json`/`eslint.config.ts`. | ||
- Update documentation and developer onboarding guides. | ||
|
||
--- | ||
|
||
## Example: Migrating a Batch | ||
|
||
1. **Select batch:** e.g., `npm/grep`, `npm/puppeteer`, `npm/mount-utils`, `npm/cypress-schematic` | ||
2. **For each package:** | ||
- Remove `.eslintrc*` files | ||
- Add `eslint.config.ts` as above | ||
- Remove local plugin deps | ||
- Run lint and fix errors | ||
- Run tests | ||
3. **Commit and open PR** | ||
|
||
--- | ||
|
||
## Troubleshooting Guide | ||
|
||
### Common Issues and Solutions | ||
|
||
#### 1. **Jiti Version Compatibility Error** | ||
**Error:** `Error: You are using an outdated version of the 'jiti' library. Please update to the latest version of 'jiti' to ensure compatibility and access to the latest features.` | ||
|
||
**Solution:** | ||
- Add `jiti: "^2.4.2"` to the package's `devDependencies` | ||
- ESLint 9.x requires jiti >= 2.2.0, but monorepo dependencies might provide older versions | ||
|
||
#### 2. **TypeScript Project Service Errors** | ||
**Error:** `was not found by the project service. Consider either including it in the tsconfig.json or including it in allowDefaultProject` | ||
|
||
**Solutions:** | ||
- **Create/update `tsconfig.json`** that extends the base config: | ||
```json | ||
{ | ||
"extends": "../../packages/ts/tsconfig.json", | ||
"compilerOptions": { | ||
"esModuleInterop": true, | ||
"allowJs": true, | ||
"checkJs": false | ||
}, | ||
"include": [ | ||
"src/**/*", | ||
"cypress/**/*", | ||
"*.js", | ||
"*.ts", | ||
"*.jsx", | ||
"*.tsx" | ||
], | ||
"exclude": ["node_modules", "dist"] | ||
} | ||
``` | ||
- **For Cypress packages**, ensure `cypress/tsconfig.json` includes all test files: | ||
```json | ||
{ | ||
"compilerOptions": { | ||
"types": ["cypress"] | ||
}, | ||
"include": [ | ||
"**/*.ts", | ||
"**/*.js" | ||
] | ||
} | ||
``` | ||
- **Add `allowDefaultProject: true`** to ESLint config for problematic files: | ||
```ts | ||
{ | ||
files: ['**/*.js', '**/*.ts', '**/*.jsx', '**/*.tsx'], | ||
languageOptions: { | ||
parserOptions: { | ||
allowDefaultProject: true, | ||
}, | ||
}, | ||
} | ||
``` | ||
|
||
#### 3. **Skip Comment Rule Violations** | ||
**Error:** `@cypress/dev/skip-comment` rule violations for `it.skip()` tests | ||
|
||
**Solution:** | ||
- Replace `eslint-disable-next-line @cypress/dev/skip-comment` with proper explanatory comments: | ||
```js | ||
// NOTE: This test is skipped for demonstration purposes | ||
it.skip('first test', () => {}) | ||
``` | ||
|
||
#### 4. **Rule Mismatches in Pre-commit Hook** | ||
**Error:** ESLint rule violations that don't match the package's ESLint configuration (e.g., `Unexpected console statement` when `no-console` is off in package config) | ||
|
||
**Root Cause:** Pre-commit hook runs ESLint from root directory using root-level config, not package-specific config | ||
|
||
**Solution:** | ||
- Add package-specific lint-staged rule in root `package.json`: | ||
```json | ||
"lint-staged": { | ||
"npm/grep/**/*.{js,jsx,ts,tsx}": "cd npm/grep && eslint --fix", | ||
"*.{js,jsx,ts,tsx,json,eslintrc,vue}": "eslint --fix" | ||
} | ||
``` | ||
|
||
#### 5. **ESLint Script Changes** | ||
**Before ESLint 9.x:** | ||
```json | ||
"lint": "eslint . --ext .js,.ts" | ||
``` | ||
|
||
**After ESLint 9.x:** | ||
```json | ||
"lint": "eslint ." | ||
cacieprins marked this conversation as resolved.
Show resolved
Hide resolved
|
||
``` | ||
ESLint 9.x auto-detects file extensions, so `--ext` flag is no longer needed. | ||
|
||
#### 6. **Package Dependencies** | ||
**Required additions to `package.json`:** | ||
```json | ||
{ | ||
"devDependencies": { | ||
"@packages/eslint-config": "0.0.0-development", | ||
"eslint": "^9.18.0", | ||
"jiti": "^2.4.2" | ||
} | ||
} | ||
``` | ||
|
||
#### 7. **Custom Package Rules** | ||
If a package needs custom rules, extend the base config: | ||
```ts | ||
import baseConfig from '@packages/eslint-config' | ||
|
||
export default [ | ||
...baseConfig, | ||
{ | ||
files: ['**/*.js', '**/*.ts', '**/*.jsx', '**/*.tsx'], | ||
rules: { | ||
'no-console': 'off', | ||
'no-restricted-syntax': 'off', | ||
}, | ||
}, | ||
{ | ||
files: ['cypress/**/*.js', 'cypress/**/*.ts'], | ||
languageOptions: { | ||
globals: { | ||
Cypress: 'readonly', | ||
cy: 'readonly', | ||
}, | ||
}, | ||
}, | ||
] | ||
``` | ||
|
||
### Migration Checklist Template | ||
|
||
For each package, ensure you've completed: | ||
|
||
- [ ] Removed `.eslintrc*` files | ||
- [ ] Created `eslint.config.ts` with proper configuration | ||
- [ ] Added required dependencies (`eslint`, `@packages/eslint-config`, `jiti`) | ||
- [ ] Created/updated `tsconfig.json` that extends base config | ||
- [ ] Updated ESLint scripts (removed `--ext` flag) | ||
- [ ] Fixed skip comment violations | ||
- [ ] Resolved console statement issues | ||
- [ ] Added package-specific lint-staged rule (if needed) | ||
- [ ] Ran `yarn lint` successfully | ||
- [ ] Ran tests to ensure nothing broke | ||
|
||
--- | ||
|
||
## Tips | ||
- Use a tracking issue or project board to coordinate and document progress. | ||
- If a package is especially noisy, consider splitting it into its own PR. | ||
- Communicate with the team about the migration timeline and process. | ||
- **Test the pre-commit hook** before pushing to ensure lint-staged works correctly. | ||
|
||
--- | ||
|
||
**Happy linting!** |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,11 @@ | ||
/* eslint-disable @cypress/dev/skip-comment */ | ||
/// <reference types="cypress" /> | ||
describe('tests that use .skip', () => { | ||
// use a template literal | ||
it(`works`, () => {}) | ||
|
||
// NOTE: This test is pending for demonstration | ||
it.skip('is pending', () => {}) | ||
|
||
// NOTE: This test is also pending for demonstration | ||
it.skip('is pending again', () => {}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,11 @@ | ||
{ | ||
"compilerOptions": { | ||
"types": ["cypress"] | ||
"types": ["cypress"], | ||
"allowJs": true, | ||
"checkJs": false | ||
}, | ||
"include": ["e2e/**/*.ts"] | ||
"include": [ | ||
"**/*.ts", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you remember why we needed to touch the tsconfig here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To get eslint to pick up on everything. The new eslint config runs .js files through typescript linter - this helps it lint interactions between js and ts. If typescript eslint matches a file that isn't in the "default project" (e.g., not matched by the closest tsconfig), it throws an error. |
||
"**/*.js" | ||
] | ||
} |
Uh oh!
There was an error while loading. Please reload this page.