-
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
Merged
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
045ea2e
foundational monorepo pkg for shared eslint config
cacieprins 028dc0f
add a migration guide
cacieprins 923f770
adds new eslint to grep
cacieprins 8c27e00
add troubleshooting guide
cacieprins 26df718
Merge branch 'develop' into eslint-migration-foundation
jennifer-shehane ecb9b16
fix eslint config jiti issue
cacieprins bec2718
fix a bit
cacieprins 4d0e588
Update guides/eslint-migration.md
cacieprins abb7d13
restore console.error to grep, export collection of cli related rules…
cacieprins dc67077
Merge branch 'develop' into eslint-migration-foundation
cacieprins 09a88ac
fix linting of primary eslint config pkg
cacieprins 8a8a7c3
fix glob exclusion
cacieprins b7e6630
Merge branch 'develop' into eslint-migration-foundation
cacieprins 5f2ed6b
uses an explicit expansion of the package patterns for eslint migrati…
cacieprins 3488c40
lockfile
cacieprins File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,339 @@ | ||
# ESLint Migration Guide: Monorepo Alignment | ||
|
||
## Migration Checklist | ||
|
||
### Batch 1: Small npm utilities | ||
- [x] npm/grep ✅ **COMPLETED** - Uses unified ESLint config with package-specific lint-staged handling | ||
- [ ] 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. **Lint-Staged Configuration** | ||
During the migration period, the root `package.json` uses explicit lint-staged patterns to ensure each package gets the correct linting treatment: | ||
|
||
- **Migrated packages** (like `npm/grep`): Use `yarn lint:fix` which runs ESLint from the package directory with the correct config | ||
- **Non-migrated packages**: Use `eslint --fix` which runs from the root with the root config | ||
- **Root files**: Covered by `*.{js,jsx,ts,tsx,json,eslintrc,vue}` pattern | ||
|
||
This configuration will be simplified once all packages are migrated to the unified ESLint setup. | ||
|
||
### 2. **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. **Simplify Lint-Staged Configuration** | ||
After all packages are migrated, simplify the lint-staged configuration in root `package.json`: | ||
|
||
```json | ||
{ | ||
"lint-staged": { | ||
"**/*.{js,jsx,ts,tsx,json,eslintrc,vue}": "eslint --fix", | ||
"*workflows.yml": "node scripts/format-workflow-file.js" | ||
} | ||
} | ||
``` | ||
|
||
### 7. **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:** | ||
- The root `package.json` now includes explicit lint-staged patterns for each package: | ||
```json | ||
"lint-staged": { | ||
"npm/grep/**/*.{js,jsx,ts,tsx}": "yarn lint:fix", | ||
"*.{js,jsx,ts,tsx,json,eslintrc,vue}": "eslint --fix", | ||
"cli/**/*.{js,jsx,ts,tsx,json,eslintrc,vue}": "eslint --fix", | ||
"packages/**/*.{js,jsx,ts,tsx,json,eslintrc,vue}": "eslint --fix", | ||
// ... explicit patterns for each directory | ||
} | ||
``` | ||
- **For migrated packages:** Use `yarn lint:fix` which runs the lerna command to execute ESLint from the package directory | ||
- **For non-migrated packages:** Use `eslint --fix` which runs from the root with the root config | ||
- **Note:** This verbose configuration is temporary during migration and will be simplified once all packages are migrated | ||
|
||
#### 5. **ESLint Script Changes** | ||
**Before ESLint 9.x:** | ||
```json | ||
"lint": "eslint . --ext .js,.ts" | ||
``` | ||
|
||
**After ESLint 9.x:** | ||
```json | ||
"lint": "eslint" | ||
``` | ||
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) | ||
- [ ] 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. | ||
|
||
## Recent Updates | ||
- **Fixed lint-staged configuration** to properly handle migrated vs non-migrated packages | ||
- **Added explicit directory patterns** to ensure complete coverage during migration period | ||
|
||
--- | ||
|
||
**Happy linting!** |
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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', () => {}) | ||
}) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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", | ||
"**/*.js" | ||
] | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The 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 comment
The 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.