From d55fe1ae64d6b5b68e4eecf9d0b75b71746df36b Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 26 Nov 2024 01:20:12 -0500 Subject: [PATCH 01/23] feat: Introduce ecosystem tests for popular third-party plugins --- .../README.md | 139 ++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 designs/2024-repo-ecosystem-plugin-tests/README.md diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md new file mode 100644 index 00000000..88e519c0 --- /dev/null +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -0,0 +1,139 @@ +- Repo: eslint/eslint +- Start Date: 2024-11-25 +- RFC PR: +- Authors: [Josh Goldberg](https://github.com/JoshuaKGoldberg) + +# Introduce ecosystem tests for popular third-party plugins + +## Summary + +Adding an CI job to the `eslint/eslint` repo that checks changes against a small selection of third-party plugins. + +## Motivation + +Changes in ESLint occasionally break downstream plugins in unexpected ways. +Those changes might be unintentional breaking changes, or even non-breaking changes that happen to touch edge case behaviors relied on by plugins. + +[Bug: Error while loading rule '@typescript-eslint/no-unused-expressions](https://github.com/eslint/eslint/issues/19134) is an example change in ESLint's that caused downstream breakages in third-party plugins. +At least two popular plugins -[`eslint-plugin-unicorn`](https://github.com/sindresorhus/eslint-plugin-unicorn/issues/2496) and [`typescript-eslint`](https://github.com/typescript-eslint/typescript-eslint/issues/10338)- were broken by that change. + +The plugins broke because they were relying on non-public implementation details of ESLint rules per [Docs: Formalize recommendation against plugins calling to rules via use-at-your-own-risk](https://github.com/eslint/eslint/issues/19169). +When the root cause is a bug in the downstream plugins, an "early warning" system would help them fix their issues before the incompatible changes to ESLint are published. + +## Detailed Design + +### CI Job + +The new CI job will, for each plugin: + +1. Create a new directory containing a `package.json`, `eslint.config.js`, and small set of files known to be parsed and not cause lint reports with the plugin +2. Run a lint command (i.e. `npx eslint .`) in that directory +3. Assert that the lint command passed with 0 lint reports. + +This will all be runnable locally with a `package.json` script like `npm run test:ecosystem --plugin unicorn`. + +An addition to `.github/workflows/ci.yml` under `jobs` would approximately look like: + +```yml +test_ecosystem: + name: Test Ecosystem Plugins + runs-on: ubuntu-latest + strategy: + matrix: + plugin: + - eslint-plugin-unicorn + - eslint-plugin-vue + - typescript-eslint + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: "lts/*" + - name: Install Packages + run: npm install + - name: Test ${{ matrix.plugin }} + run: npm run test:ecosystem --plugin ${{ matrix.plugin }} +``` + +A `test/ecosystem` directory will be created with a directory for each plugin. +The `test:ecosystem` script will copy the contents of the provided `--plugin` directory into a clean `test/${plugin}-scratch` directory. + +Asserting that plugins successfully produce reports will not be part of this job. +Depending on specifics of plugin rule reports would make the job prone to failure on arbitrary plugin rule updates. + +### Failure Handling + +It is theoretically possible that the ecosystem CI job will occasionally be broken by updates to ecosystem plugins. +However, this RFC believes that case will be exceedingly rare and short-lived: + +- Per [Plugin Selection](#plugin-selection), only very stable plugins that test on multiple ESLint versions including the latest will be selected +- Today, plugin breakages are typically resolved within a week - even without this RFC's proposed "early warning" detection + +In the case of a breakage being discovered, this RFC proposes the following process: + +1. An ESLint team member should file a bug report on the plugin's repository -if it doesn't yet exist-, as well as an issue on `eslint/eslint` linking to that bug report +2. If the issue isn't resolved within two weeks: + 1. The plugin will be removed from ESLint's ecosystem CI job + 2. An ESLint team member should file a followup issue to re-add it once the breakage is fixed + +### Major Releases + +Upcoming new major versions of ESLint are an expected failure case for ecosystem plugins. +The ecosystem CI job will skip running any plugin that doesn't explicitly support the version of ESLint being tested. + +Plugin version support will be determined by the maximum `eslint` peer dependency range in the plugin's published `package.json`, if it exists. +Otherwise the ESLint repository will assume only supporting up to the currently stable version of ESLint. + +### Plugin Selection + +The plugins that will be included to start will be: + +- [`eslint-plugin-unicorn`](https://github.com/sindresorhus/eslint-plugin-unicorn): to capture a large selection of miscellaneous rules +- [`eslint-plugin-vue`](https://github.com/vuejs/eslint-plugin-vue): to capture support for a framework with nested parsing of a non-JavaScript/TypeScript-standard syntax +- [`typescript-eslint`](https://github.com/typescript-eslint/typescript-eslint): to capture testing TypeScript APIs and intricate uses of parsing in general + +Plugins will be selectively added if meet the following criteria: + +- >1 million npm downloads a week: arbitrary large size threshold to avoid small packages +- Adding a notable new API usage not yet covered: to avoid duplicate equivalent plugins +- Has had a breakage reported on ESLint: to be cautious in adding to the list +- Is under active maintenance: to avoid packages that won't be updated quickly on failures + +The number of plugins should remain small. +Each added plugin brings adds the risk of third-party breakage, so plugins will only be added after filing a new issue and gaining team consensus. + +### Rollout + +This RFC expects the added ecosystem CI job to _likely_ consistently pass. +However, to be safe, this RFC proposes adding a CI job in three steps: + +1. On a branch that and updated from `main` several times a week +2. On the `main` branch only +3. On all branches, alongside existing CI jobs + +At least one month should be held between steps to make sure the job is consistently passing. + +## Open Questions + +Automation could be added for at least the filing of issues on plugin failures. +That does not seem worth the time expenditure given how rarely plugins are expected to fail. +Is that accurate? + +Are there other plugins we should include that satisfy the criteria? + +## Help Needed + +I expect to implement this change. + +## Frequently Asked Questions + +### Given ESLint respects semver, why add tests for plugins that are relying on internals? + +It's exceedingly difficult to be sure when changes to a large published package break contracts with downstream consumers. +Even when all packages in an ecosystem are well-tested the way ESLint and its major plugins are, the sheer project size and duration of maintenance make unfortunate edge cases likely to happen. + +> [Venerable xkcd "Workflow" comic](https://xkcd.com/1172) + +## Related Discussions + +- [Repo: add end-to-end/integration tests for popular 3rd party plugins](https://github.com/eslint/eslint/issues/19139) From 9baeb50538d7113a0bad620b0d0a7954e610de95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Tue, 3 Dec 2024 09:44:46 -0500 Subject: [PATCH 02/23] Update designs/2024-repo-ecosystem-plugin-tests/README.md Co-authored-by: Francesco Trotta --- designs/2024-repo-ecosystem-plugin-tests/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index 88e519c0..5b048b2d 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -30,7 +30,7 @@ The new CI job will, for each plugin: 2. Run a lint command (i.e. `npx eslint .`) in that directory 3. Assert that the lint command passed with 0 lint reports. -This will all be runnable locally with a `package.json` script like `npm run test:ecosystem --plugin unicorn`. +This will all be runnable locally with a `package.json` script like `npm run test:ecosystem --plugin eslint-plugin-unicorn`. An addition to `.github/workflows/ci.yml` under `jobs` would approximately look like: From a52ca5ea021de72559f3803d36ebe485d2854664 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 3 Dec 2024 09:45:56 -0500 Subject: [PATCH 03/23] on all PRs --- designs/2024-repo-ecosystem-plugin-tests/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index 5b048b2d..790f244d 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -109,7 +109,7 @@ However, to be safe, this RFC proposes adding a CI job in three steps: 1. On a branch that and updated from `main` several times a week 2. On the `main` branch only -3. On all branches, alongside existing CI jobs +3. On all PRs targeting the `main` branch, alongside existing CI jobs At least one month should be held between steps to make sure the job is consistently passing. From 1329c1a4365386a678d5809d87956f5332c07b84 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 3 Dec 2024 09:51:02 -0500 Subject: [PATCH 04/23] Split issue/PR steps for main vs. PR --- designs/2024-repo-ecosystem-plugin-tests/README.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index 790f244d..e70ed27d 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -69,13 +69,21 @@ However, this RFC believes that case will be exceedingly rare and short-lived: - Per [Plugin Selection](#plugin-selection), only very stable plugins that test on multiple ESLint versions including the latest will be selected - Today, plugin breakages are typically resolved within a week - even without this RFC's proposed "early warning" detection -In the case of a breakage being discovered, this RFC proposes the following process: +In the case of a breakage being discovered on the `main` branch, this RFC proposes the following process: 1. An ESLint team member should file a bug report on the plugin's repository -if it doesn't yet exist-, as well as an issue on `eslint/eslint` linking to that bug report 2. If the issue isn't resolved within two weeks: - 1. The plugin will be removed from ESLint's ecosystem CI job + 1. An ESLint team member should send a PR to resolve the issue that removes the plugin from ESLint's ecosystem CI job 2. An ESLint team member should file a followup issue to re-add it once the breakage is fixed +In the case of a breaking being discovered on a PR branch, this RFC proposes the following process: + +1. If the failure is an indication of an issue in the PR, the PR should be updated as usual +2. Otherwise, if the failure is an indication the plugin needs to be updated, the PR's author should file a bug report on the plugin's repository - if it doesn't yet exist +3. If the issue isn't resolved within two weeks: + 1. The PR's author should remove the plugin from ESLint's ecosystem CI job in the PR + 2. The PR's author should file a followup issue to re-add it once the breakage is fixed + ### Major Releases Upcoming new major versions of ESLint are an expected failure case for ecosystem plugins. From cdb2677db4ef646883fdaa515fa1e10c29e09f8c Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 3 Dec 2024 09:51:56 -0500 Subject: [PATCH 05/23] Out of Scope: automation --- designs/2024-repo-ecosystem-plugin-tests/README.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index e70ed27d..de283ae8 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -121,11 +121,13 @@ However, to be safe, this RFC proposes adding a CI job in three steps: At least one month should be held between steps to make sure the job is consistently passing. -## Open Questions +## Out of Scope Automation could be added for at least the filing of issues on plugin failures. That does not seem worth the time expenditure given how rarely plugins are expected to fail. -Is that accurate? +This RFC's discussion settled on it not being worth it. + +## Open Questions Are there other plugins we should include that satisfy the criteria? From 071532d82bc3a89c60b0068fac1b7d25396c3c5e Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 3 Dec 2024 09:54:37 -0500 Subject: [PATCH 06/23] Added eslint-plugin-eslint-comments --- designs/2024-repo-ecosystem-plugin-tests/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index de283ae8..01675942 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -96,6 +96,7 @@ Otherwise the ESLint repository will assume only supporting up to the currently The plugins that will be included to start will be: +- [`eslint-plugin-eslint-comments`](https://github.com/eslint-community/eslint-plugin-eslint-comments): to capture an `eslint-community` project and AST edge cases around comments - [`eslint-plugin-unicorn`](https://github.com/sindresorhus/eslint-plugin-unicorn): to capture a large selection of miscellaneous rules - [`eslint-plugin-vue`](https://github.com/vuejs/eslint-plugin-vue): to capture support for a framework with nested parsing of a non-JavaScript/TypeScript-standard syntax - [`typescript-eslint`](https://github.com/typescript-eslint/typescript-eslint): to capture testing TypeScript APIs and intricate uses of parsing in general From 7d6194736f4fa10297be7ce38d9cc4c08f985fe7 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 3 Dec 2024 09:56:03 -0500 Subject: [PATCH 07/23] if (they) meet - typo --- designs/2024-repo-ecosystem-plugin-tests/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index 01675942..214a06f6 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -101,7 +101,7 @@ The plugins that will be included to start will be: - [`eslint-plugin-vue`](https://github.com/vuejs/eslint-plugin-vue): to capture support for a framework with nested parsing of a non-JavaScript/TypeScript-standard syntax - [`typescript-eslint`](https://github.com/typescript-eslint/typescript-eslint): to capture testing TypeScript APIs and intricate uses of parsing in general -Plugins will be selectively added if meet the following criteria: +Plugins will be selectively added if they meet the following criteria: - >1 million npm downloads a week: arbitrary large size threshold to avoid small packages - Adding a notable new API usage not yet covered: to avoid duplicate equivalent plugins From d1ab08bee6b9faa5068863fa7aae3ead5a9d43d7 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 3 Dec 2024 11:55:15 -0500 Subject: [PATCH 08/23] Mention examples of fixed breakages within a week --- designs/2024-repo-ecosystem-plugin-tests/README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index 214a06f6..5a115ae5 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -68,6 +68,9 @@ However, this RFC believes that case will be exceedingly rare and short-lived: - Per [Plugin Selection](#plugin-selection), only very stable plugins that test on multiple ESLint versions including the latest will be selected - Today, plugin breakages are typically resolved within a week - even without this RFC's proposed "early warning" detection + - Example: [typescript-eslint#10191](https://github.com/typescript-eslint/typescript-eslint/issues/10338) was reported on October 21st, 2024 and a fix published on October 28th, 2024 + - Example: [typescript-eslint#10338](https://github.com/typescript-eslint/typescript-eslint/issues/10338) was reported on November 15th, 2024 and a fix published on November 18th, 2024 + - Example: [eslint-plugin-unicorn#10191](https://github.com/sindresorhus/eslint-plugin-unicorn/issues/2496) was reported on November 15th, 2024 and a fix published on November 19th, 2024 In the case of a breakage being discovered on the `main` branch, this RFC proposes the following process: From 994a1a8c86b3eb2fbe53c87c7987eeb8250dfcd1 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 3 Dec 2024 12:01:08 -0500 Subject: [PATCH 09/23] Note 1-week fix examples and add to selection criteria --- designs/2024-repo-ecosystem-plugin-tests/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index 5a115ae5..f945fede 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -109,7 +109,7 @@ Plugins will be selectively added if they meet the following criteria: - >1 million npm downloads a week: arbitrary large size threshold to avoid small packages - Adding a notable new API usage not yet covered: to avoid duplicate equivalent plugins - Has had a breakage reported on ESLint: to be cautious in adding to the list -- Is under active maintenance: to avoid packages that won't be updated quickly on failures +- Is under active maintenance and has taken a week or less to fix any ESLint breakages within the last year: to avoid packages that won't be updated quickly on failures The number of plugins should remain small. Each added plugin brings adds the risk of third-party breakage, so plugins will only be added after filing a new issue and gaining team consensus. From 2dcf2be477a0294eb19cb4ad00e51a6bccc42d2e Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 3 Dec 2024 12:05:38 -0500 Subject: [PATCH 10/23] Small fixes: clarification, typo --- designs/2024-repo-ecosystem-plugin-tests/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index f945fede..bde60779 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -104,7 +104,7 @@ The plugins that will be included to start will be: - [`eslint-plugin-vue`](https://github.com/vuejs/eslint-plugin-vue): to capture support for a framework with nested parsing of a non-JavaScript/TypeScript-standard syntax - [`typescript-eslint`](https://github.com/typescript-eslint/typescript-eslint): to capture testing TypeScript APIs and intricate uses of parsing in general -Plugins will be selectively added if they meet the following criteria: +Plugins will be selectively added if they meet all of the following criteria: - >1 million npm downloads a week: arbitrary large size threshold to avoid small packages - Adding a notable new API usage not yet covered: to avoid duplicate equivalent plugins @@ -119,7 +119,7 @@ Each added plugin brings adds the risk of third-party breakage, so plugins will This RFC expects the added ecosystem CI job to _likely_ consistently pass. However, to be safe, this RFC proposes adding a CI job in three steps: -1. On a branch that and updated from `main` several times a week +1. On a branch that is manually updated from `main` several times a week 2. On the `main` branch only 3. On all PRs targeting the `main` branch, alongside existing CI jobs From 5a79c72b836e37fc82c51fa758cec5e128495c85 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 3 Dec 2024 12:07:14 -0500 Subject: [PATCH 11/23] Clarification: 'those' steps --- designs/2024-repo-ecosystem-plugin-tests/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index bde60779..505bee38 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -123,7 +123,7 @@ However, to be safe, this RFC proposes adding a CI job in three steps: 2. On the `main` branch only 3. On all PRs targeting the `main` branch, alongside existing CI jobs -At least one month should be held between steps to make sure the job is consistently passing. +At least one month should be held between those steps to make sure the job is consistently passing. ## Out of Scope From e663bf3cf0d932c794eae776968116a54434e737 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 6 Dec 2024 06:21:40 -0500 Subject: [PATCH 12/23] Added: dogfooding not being enough; 'all' enablements --- designs/2024-repo-ecosystem-plugin-tests/README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index 505bee38..8835cb6c 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -18,6 +18,7 @@ Those changes might be unintentional breaking changes, or even non-breaking chan At least two popular plugins -[`eslint-plugin-unicorn`](https://github.com/sindresorhus/eslint-plugin-unicorn/issues/2496) and [`typescript-eslint`](https://github.com/typescript-eslint/typescript-eslint/issues/10338)- were broken by that change. The plugins broke because they were relying on non-public implementation details of ESLint rules per [Docs: Formalize recommendation against plugins calling to rules via use-at-your-own-risk](https://github.com/eslint/eslint/issues/19169). +ESLint core's [`eslint-config-eslint`](https://github.com/eslint/eslint/tree/main/packages/eslint-config-eslint) does not use all rules of downstream plugins and is not always up-to-date with their latest versions, so its internal usage of plugins is not sufficient to flag all high visibility compatibility issues. When the root cause is a bug in the downstream plugins, an "early warning" system would help them fix their issues before the incompatible changes to ESLint are published. ## Detailed Design @@ -26,7 +27,10 @@ When the root cause is a bug in the downstream plugins, an "early warning" syste The new CI job will, for each plugin: -1. Create a new directory containing a `package.json`, `eslint.config.js`, and small set of files known to be parsed and not cause lint reports with the plugin +1. Create a new directory containing: + - `package.json` + - `eslint.config.js` with the closest equivalent to an _"enable all rules"_ preset from the plugin + - A small set of files known to be parsed and not cause lint reports with the plugin 2. Run a lint command (i.e. `npx eslint .`) in that directory 3. Assert that the lint command passed with 0 lint reports. From 91aa5027aeec983236f823f9b507d1ee61d22666 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Tue, 10 Dec 2024 12:41:24 -0500 Subject: [PATCH 13/23] Apply suggestions from code review Co-authored-by: Milos Djermanovic --- designs/2024-repo-ecosystem-plugin-tests/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index 8835cb6c..97a7441e 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -72,9 +72,9 @@ However, this RFC believes that case will be exceedingly rare and short-lived: - Per [Plugin Selection](#plugin-selection), only very stable plugins that test on multiple ESLint versions including the latest will be selected - Today, plugin breakages are typically resolved within a week - even without this RFC's proposed "early warning" detection - - Example: [typescript-eslint#10191](https://github.com/typescript-eslint/typescript-eslint/issues/10338) was reported on October 21st, 2024 and a fix published on October 28th, 2024 + - Example: [typescript-eslint#10191](https://github.com/typescript-eslint/typescript-eslint/issues/10191) was reported on October 21st, 2024 and a fix published on October 28th, 2024 - Example: [typescript-eslint#10338](https://github.com/typescript-eslint/typescript-eslint/issues/10338) was reported on November 15th, 2024 and a fix published on November 18th, 2024 - - Example: [eslint-plugin-unicorn#10191](https://github.com/sindresorhus/eslint-plugin-unicorn/issues/2496) was reported on November 15th, 2024 and a fix published on November 19th, 2024 + - Example: [eslint-plugin-unicorn#2496](https://github.com/sindresorhus/eslint-plugin-unicorn/issues/2496) was reported on November 15th, 2024 and a fix published on November 19th, 2024 In the case of a breakage being discovered on the `main` branch, this RFC proposes the following process: From 46128007d4bbe41b2f19855f8bc89a11d74a7248 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 10 Dec 2024 12:42:32 -0500 Subject: [PATCH 14/23] Expand to first-(second-?)party plugins --- .../README.md | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index 97a7441e..9e8bf0f6 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -3,11 +3,11 @@ - RFC PR: - Authors: [Josh Goldberg](https://github.com/JoshuaKGoldberg) -# Introduce ecosystem tests for popular third-party plugins +# Introduce ecosystem tests for popular plugins ## Summary -Adding an CI job to the `eslint/eslint` repo that checks changes against a small selection of third-party plugins. +Adding an CI job to the `eslint/eslint` repo that checks changes against `@eslint/*` plugins as well as a small selection of third-party plugins. ## Motivation @@ -67,7 +67,7 @@ Depending on specifics of plugin rule reports would make the job prone to failur ### Failure Handling -It is theoretically possible that the ecosystem CI job will occasionally be broken by updates to ecosystem plugins. +It is theoretically possible that the ecosystem CI job will occasionally be broken by updates to plugins. However, this RFC believes that case will be exceedingly rare and short-lived: - Per [Plugin Selection](#plugin-selection), only very stable plugins that test on multiple ESLint versions including the latest will be selected @@ -103,20 +103,21 @@ Otherwise the ESLint repository will assume only supporting up to the currently The plugins that will be included to start will be: +- All `@eslint/*` plugins, including [`@eslint/css`](https://www.npmjs.com/package/@eslint/css), [`@eslint/json`](https://www.npmjs.com/package/@eslint/json), and [`@eslint/markdown`](https://www.npmjs.com/package/@eslint/markdown) - [`eslint-plugin-eslint-comments`](https://github.com/eslint-community/eslint-plugin-eslint-comments): to capture an `eslint-community` project and AST edge cases around comments - [`eslint-plugin-unicorn`](https://github.com/sindresorhus/eslint-plugin-unicorn): to capture a large selection of miscellaneous rules - [`eslint-plugin-vue`](https://github.com/vuejs/eslint-plugin-vue): to capture support for a framework with nested parsing of a non-JavaScript/TypeScript-standard syntax - [`typescript-eslint`](https://github.com/typescript-eslint/typescript-eslint): to capture testing TypeScript APIs and intricate uses of parsing in general -Plugins will be selectively added if they meet all of the following criteria: +Third-party plugins will be selectively added if they meet all of the following criteria: - >1 million npm downloads a week: arbitrary large size threshold to avoid small packages - Adding a notable new API usage not yet covered: to avoid duplicate equivalent plugins - Has had a breakage reported on ESLint: to be cautious in adding to the list - Is under active maintenance and has taken a week or less to fix any ESLint breakages within the last year: to avoid packages that won't be updated quickly on failures -The number of plugins should remain small. -Each added plugin brings adds the risk of third-party breakage, so plugins will only be added after filing a new issue and gaining team consensus. +The number of third-party plugins should remain small. +Each added plugin brings adds a risk of breakage, so plugins will only be added after filing a new issue and gaining team consensus. ### Rollout @@ -152,6 +153,14 @@ Even when all packages in an ecosystem are well-tested the way ESLint and its ma > [Venerable xkcd "Workflow" comic](https://xkcd.com/1172) +### What if a breakage causes rules to report incorrectly, but doesn't cause `npm lint` to crash? + +Checking for incorrect rule reports is not handled by this RFC. +All recent significant downstream breakages caused rules to fully crash. + +Any kind of rule report verification would necessitate ecosystem tests taking a dependency on the specific reports from downstream plugins. +This RFC does not believe the effort of keeping snapshots of those reports up-to-date as worthwhile. + ## Related Discussions - [Repo: add end-to-end/integration tests for popular 3rd party plugins](https://github.com/eslint/eslint/issues/19139) From 8fd5da4890c0b598cc0efb081d0fbcc4f6882334 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 10 Dec 2024 12:49:19 -0500 Subject: [PATCH 15/23] Automate updates --- designs/2024-repo-ecosystem-plugin-tests/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index 9e8bf0f6..8435afb4 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -124,7 +124,7 @@ Each added plugin brings adds a risk of breakage, so plugins will only be added This RFC expects the added ecosystem CI job to _likely_ consistently pass. However, to be safe, this RFC proposes adding a CI job in three steps: -1. On a branch that is manually updated from `main` several times a week +1. On a branch that is updated from `main` by a CI cron job several times a week 2. On the `main` branch only 3. On all PRs targeting the `main` branch, alongside existing CI jobs From 0f518bf95b737b2792b5c8fa2f849e1ffedfc6a0 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 13 Dec 2024 15:59:06 -0500 Subject: [PATCH 16/23] Explain the off-main branch --- designs/2024-repo-ecosystem-plugin-tests/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index 8435afb4..08665643 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -128,6 +128,7 @@ However, to be safe, this RFC proposes adding a CI job in three steps: 2. On the `main` branch only 3. On all PRs targeting the `main` branch, alongside existing CI jobs +Starting with a branch outside `main` ensures that unexpectedly high frequencies of breakages are caught early, without blocking `main` branch builds. At least one month should be held between those steps to make sure the job is consistently passing. ## Out of Scope From c176a7e4c2e5190c1b36f5aa0b5fcebd828548f9 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 18 Dec 2024 08:58:15 -0500 Subject: [PATCH 17/23] Switch cron job to be actually just a cron job --- designs/2024-repo-ecosystem-plugin-tests/README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index 08665643..cddc3f6c 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -122,13 +122,14 @@ Each added plugin brings adds a risk of breakage, so plugins will only be added ### Rollout This RFC expects the added ecosystem CI job to _likely_ consistently pass. -However, to be safe, this RFC proposes adding a CI job in three steps: +A CI job will be added to the `eslint/eslint` issue, but will not immediately be a part of `main` branch or PR branch builds. +To be safe, this RFC proposes rolling out CI job in three steps: -1. On a branch that is updated from `main` by a CI cron job several times a week +1. On a CI cron job once a day, targeting the `main` branch but not blocking its builds 2. On the `main` branch only 3. On all PRs targeting the `main` branch, alongside existing CI jobs -Starting with a branch outside `main` ensures that unexpectedly high frequencies of breakages are caught early, without blocking `main` branch builds. +Starting with a job separately from `main` ensures that unexpectedly high frequencies of breakages are caught early, without blocking `main` branch builds. At least one month should be held between those steps to make sure the job is consistently passing. ## Out of Scope From 7fef2efad3e684c9b6fd035569ff3ac1f48a3656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Mon, 20 Jan 2025 09:50:54 -0500 Subject: [PATCH 18/23] Apply suggestions from code review Co-authored-by: Nicholas C. Zakas --- designs/2024-repo-ecosystem-plugin-tests/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index cddc3f6c..25377614 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -117,12 +117,12 @@ Third-party plugins will be selectively added if they meet all of the following - Is under active maintenance and has taken a week or less to fix any ESLint breakages within the last year: to avoid packages that won't be updated quickly on failures The number of third-party plugins should remain small. -Each added plugin brings adds a risk of breakage, so plugins will only be added after filing a new issue and gaining team consensus. +Each added plugin adds a risk of breakage, so plugins will only be added after filing a new issue and gaining team consensus. ### Rollout This RFC expects the added ecosystem CI job to _likely_ consistently pass. -A CI job will be added to the `eslint/eslint` issue, but will not immediately be a part of `main` branch or PR branch builds. +A CI job will be added to the `eslint/eslint` repo, but will not immediately be a part of `main` branch or PR branch builds. To be safe, this RFC proposes rolling out CI job in three steps: 1. On a CI cron job once a day, targeting the `main` branch but not blocking its builds From c767f0f235cb5dc1aaebcb7763e0ebd50f51dd4c Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 20 Jan 2025 12:39:23 -0500 Subject: [PATCH 19/23] Clarifications: rollout and some small points --- designs/2024-repo-ecosystem-plugin-tests/README.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index 25377614..201fb2f6 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -7,14 +7,14 @@ ## Summary -Adding an CI job to the `eslint/eslint` repo that checks changes against `@eslint/*` plugins as well as a small selection of third-party plugins. +Adding an CI job to the `eslint/eslint` repository that checks changes against `@eslint/*` plugins as well as a small selection of third-party plugins. ## Motivation Changes in ESLint occasionally break downstream plugins in unexpected ways. Those changes might be unintentional breaking changes, or even non-breaking changes that happen to touch edge case behaviors relied on by plugins. -[Bug: Error while loading rule '@typescript-eslint/no-unused-expressions](https://github.com/eslint/eslint/issues/19134) is an example change in ESLint's that caused downstream breakages in third-party plugins. +[Bug: Error while loading rule '@typescript-eslint/no-unused-expressions'](https://github.com/eslint/eslint/issues/19134) reports an example change in ESLint that caused downstream breakages in third-party plugins. At least two popular plugins -[`eslint-plugin-unicorn`](https://github.com/sindresorhus/eslint-plugin-unicorn/issues/2496) and [`typescript-eslint`](https://github.com/typescript-eslint/typescript-eslint/issues/10338)- were broken by that change. The plugins broke because they were relying on non-public implementation details of ESLint rules per [Docs: Formalize recommendation against plugins calling to rules via use-at-your-own-risk](https://github.com/eslint/eslint/issues/19169). @@ -122,13 +122,16 @@ Each added plugin adds a risk of breakage, so plugins will only be added after f ### Rollout This RFC expects the added ecosystem CI job to _likely_ consistently pass. -A CI job will be added to the `eslint/eslint` repo, but will not immediately be a part of `main` branch or PR branch builds. +A CI job will be added to the `eslint/eslint` repository, but will not immediately be a part of `main` branch or PR branch builds. To be safe, this RFC proposes rolling out CI job in three steps: 1. On a CI cron job once a day, targeting the `main` branch but not blocking its builds -2. On the `main` branch only +2. On the `main` branch only, with failures showing as failures in its builds 3. On all PRs targeting the `main` branch, alongside existing CI jobs +Each step will replace the previous step. +Once all three are done, running ecosystem tests will be a standard part of `main` branch and pull request CI along with existing tasks like linting and testing. + Starting with a job separately from `main` ensures that unexpectedly high frequencies of breakages are caught early, without blocking `main` branch builds. At least one month should be held between those steps to make sure the job is consistently passing. From 8ddf9eaffc10e4ca3f634a3534b371d332e585f8 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Mon, 20 Jan 2025 12:42:47 -0500 Subject: [PATCH 20/23] Noted internal/private API usage as out of scope --- designs/2024-repo-ecosystem-plugin-tests/README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index 201fb2f6..75e4fafa 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -141,6 +141,10 @@ Automation could be added for at least the filing of issues on plugin failures. That does not seem worth the time expenditure given how rarely plugins are expected to fail. This RFC's discussion settled on it not being worth it. +Plugins using internal/private ESLint APIs are one of the canonical examples of what this process is meant to flag. +However, this process intentionally does not include remediations for using those APIs beyond filing an issue on the downstream repository. +The intent is that consumers will own correcting uses of ESLint, and can ask for help in the standard ESLint channels as needed. + ## Open Questions Are there other plugins we should include that satisfy the criteria? From 480a8bb7398695e5b4cdf408702878dd87dcd27b Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 6 Feb 2025 16:37:15 -0500 Subject: [PATCH 21/23] Increase scope: test:eslint-compat --- .../README.md | 55 ++++++++++--------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index 75e4fafa..25d083bf 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -23,16 +23,20 @@ When the root cause is a bug in the downstream plugins, an "early warning" syste ## Detailed Design +This RFC proposes creating a small list of popular third-party plugins that will be tested as part of ESLint's CI. +Each plugin will have a `test:eslint-compat` script in their `package.json` that runs lint rule tests. + +See [Selection Criteria](#plugin-selection) below for specifics on which plugins will be included. + +> ⚠️ Plugins are currently being asked for feedback on the `test:eslint-compat` script. + ### CI Job The new CI job will, for each plugin: -1. Create a new directory containing: - - `package.json` - - `eslint.config.js` with the closest equivalent to an _"enable all rules"_ preset from the plugin - - A small set of files known to be parsed and not cause lint reports with the plugin -2. Run a lint command (i.e. `npx eslint .`) in that directory -3. Assert that the lint command passed with 0 lint reports. +1. Clone the plugin into a directory named `test/ecosystem/${plugin}` +2. Run the plugin's package installation and build commands with [ni](https://github.com/antfu-collective/ni) +3. Run the plugin's `test:eslint-compat` script with [ni](https://github.com/antfu-collective/ni) This will all be runnable locally with a `package.json` script like `npm run test:ecosystem --plugin eslint-plugin-unicorn`. @@ -59,12 +63,6 @@ test_ecosystem: run: npm run test:ecosystem --plugin ${{ matrix.plugin }} ``` -A `test/ecosystem` directory will be created with a directory for each plugin. -The `test:ecosystem` script will copy the contents of the provided `--plugin` directory into a clean `test/${plugin}-scratch` directory. - -Asserting that plugins successfully produce reports will not be part of this job. -Depending on specifics of plugin rule reports would make the job prone to failure on arbitrary plugin rule updates. - ### Failure Handling It is theoretically possible that the ecosystem CI job will occasionally be broken by updates to plugins. @@ -76,20 +74,25 @@ However, this RFC believes that case will be exceedingly rare and short-lived: - Example: [typescript-eslint#10338](https://github.com/typescript-eslint/typescript-eslint/issues/10338) was reported on November 15th, 2024 and a fix published on November 18th, 2024 - Example: [eslint-plugin-unicorn#2496](https://github.com/sindresorhus/eslint-plugin-unicorn/issues/2496) was reported on November 15th, 2024 and a fix published on November 19th, 2024 -In the case of a breakage being discovered on the `main` branch, this RFC proposes the following process: +If a breakage occurs on the `main` branch of ESLint, it will be assumed a plugin has introduced a compatibility bug and should be fixed. +This RFC proposes the following process: -1. An ESLint team member should file a bug report on the plugin's repository -if it doesn't yet exist-, as well as an issue on `eslint/eslint` linking to that bug report -2. If the issue isn't resolved within two weeks: - 1. An ESLint team member should send a PR to resolve the issue that removes the plugin from ESLint's ecosystem CI job - 2. An ESLint team member should file a followup issue to re-add it once the breakage is fixed +1. An ESLint team member should file issues tracking fixing the breakage: + - A bug report on the plugin's repository if it doesn't yet exist + - An issue on `eslint/eslint` linking to that bug report +2. If the issue isn't resolved by the next day, an ESLint team member should: + 1. Send a PR to the ESLint repository to remove the plugin from ESLint's ecosystem CI job + 2. File a followup issue to re-add it once the breakage is fixed -In the case of a breaking being discovered on a PR branch, this RFC proposes the following process: +In the case of a breakage being discovered on a PR branch, this RFC proposes the following process: 1. If the failure is an indication of an issue in the PR, the PR should be updated as usual -2. Otherwise, if the failure is an indication the plugin needs to be updated, the PR's author should file a bug report on the plugin's repository - if it doesn't yet exist -3. If the issue isn't resolved within two weeks: - 1. The PR's author should remove the plugin from ESLint's ecosystem CI job in the PR - 2. The PR's author should file a followup issue to re-add it once the breakage is fixed +2. Otherwise, if the failure is an indication the plugin needs to be updated, the PR's author should drive filing issues to update the plugin: + 1. The PR author should file a bug report on the plugin's repository - if it doesn't yet exist + 2. If the issue isn't resolved within two weeks: + 1. The PR's author should remove the plugin from ESLint's ecosystem CI job in the PR + 2. The PR's author should file a followup issue on ESLint, initially labeled as `blocked`, to re-add it once the breakage is fixed + 3. Once the breakage is fixed, a team member should remove replace the issue's `blocked` label should be replaced with `accepted` ### Major Releases @@ -115,6 +118,7 @@ Third-party plugins will be selectively added if they meet all of the following - Adding a notable new API usage not yet covered: to avoid duplicate equivalent plugins - Has had a breakage reported on ESLint: to be cautious in adding to the list - Is under active maintenance and has taken a week or less to fix any ESLint breakages within the last year: to avoid packages that won't be updated quickly on failures +- Add a `test:eslint-compat` script that exclusively runs lint rule tests The number of third-party plugins should remain small. Each added plugin adds a risk of breakage, so plugins will only be added after filing a new issue and gaining team consensus. @@ -142,8 +146,9 @@ That does not seem worth the time expenditure given how rarely plugins are expec This RFC's discussion settled on it not being worth it. Plugins using internal/private ESLint APIs are one of the canonical examples of what this process is meant to flag. -However, this process intentionally does not include remediations for using those APIs beyond filing an issue on the downstream repository. -The intent is that consumers will own correcting uses of ESLint, and can ask for help in the standard ESLint channels as needed. +However, this process intentionally does not include processes for making code changes in those plugins. +For downstream repositories, this process only proposes how the ESLint team or PR contributors may file issues. +This RFC's intent is that those repositories will drive changing their uses of ESLint APIs. ## Open Questions @@ -162,7 +167,7 @@ Even when all packages in an ecosystem are well-tested the way ESLint and its ma > [Venerable xkcd "Workflow" comic](https://xkcd.com/1172) -### What if a breakage causes rules to report incorrectly, but doesn't cause `npm lint` to crash? +### What if a breakage causes rules to report incorrectly, but doesn't cause `npm test:eslint-compat` to crash? Checking for incorrect rule reports is not handled by this RFC. All recent significant downstream breakages caused rules to fully crash. From a26b9eec499d6dfa7466e5d0bfb80a7b07f0acae Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 6 Feb 2025 16:40:31 -0500 Subject: [PATCH 22/23] typo: Plugin Selection --- designs/2024-repo-ecosystem-plugin-tests/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index 25d083bf..b88a919e 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -26,7 +26,7 @@ When the root cause is a bug in the downstream plugins, an "early warning" syste This RFC proposes creating a small list of popular third-party plugins that will be tested as part of ESLint's CI. Each plugin will have a `test:eslint-compat` script in their `package.json` that runs lint rule tests. -See [Selection Criteria](#plugin-selection) below for specifics on which plugins will be included. +See [Plugin Selection](#plugin-selection) below for specifics on which plugins will be included. > ⚠️ Plugins are currently being asked for feedback on the `test:eslint-compat` script. From fb5f7bd47342229fff5fb14e16eb4174045ad72f Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 6 Feb 2025 16:45:58 -0500 Subject: [PATCH 23/23] Note on build script name --- designs/2024-repo-ecosystem-plugin-tests/README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/designs/2024-repo-ecosystem-plugin-tests/README.md b/designs/2024-repo-ecosystem-plugin-tests/README.md index b88a919e..e27dadf7 100644 --- a/designs/2024-repo-ecosystem-plugin-tests/README.md +++ b/designs/2024-repo-ecosystem-plugin-tests/README.md @@ -63,6 +63,9 @@ test_ecosystem: run: npm run test:ecosystem --plugin ${{ matrix.plugin }} ``` +For now, it is assumed each plugin that needs to be built before testing does so with a script named `build`. +The CI job could be given overrides in the `matrix.plugin` to override the name of the builder script(s) as needed. + ### Failure Handling It is theoretically possible that the ecosystem CI job will occasionally be broken by updates to plugins.