Skip to content

Latest commit

 

History

History
166 lines (115 loc) · 9.4 KB

README.md

File metadata and controls

166 lines (115 loc) · 9.4 KB

Introduce ecosystem tests for popular plugins

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.

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 is an example change in ESLint's that caused downstream breakages in third-party plugins. At least two popular plugins -eslint-plugin-unicorn and typescript-eslint- 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. ESLint core's 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

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.

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:

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 plugins. However, this RFC believes that case will be exceedingly rare and short-lived:

  • Per 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 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. 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. 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:

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 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

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 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

At least one month should be held between those steps to make sure the job is consistently passing.

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. This RFC's discussion settled on it not being worth it.

Open Questions

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

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