Repo: add end-to-end/integration tests for popular 3rd party plugins

[JoshuaKGoldberg]

#19134 is an example of an issue where a change was merged into ESLint's main branch that caused downstream breakages in third-party plugins. Two popular examples: unicorn (sindresorhus/eslint-plugin-unicorn#2496) and typescript-eslint (typescript-eslint/typescript-eslint#10338). The specific breaking PR was #17656 (by me, sorry folks 🙂).

Proposal: how about we add some kind of ecosystem test to CI that runs some basic linting with these popular plugins? That way #17656's breakage would have been caught before hitting users.

To start, maybe targeting popular plugins with a lot of rules would be helpful for finding edge cases:

• eslint-plugin-unicorn: for miscellaneous rules
• typescript-eslint: for the TypeScript side
• eslint-plugin-vue: for the framework side

If this is too heavyweight to put into individual PRs, it could alternately put only on the main branch. That way breakages would be discovered before a release.

For reference, we have some tests in typescript-eslint for community plugins that alert us when there's a breakage: e.g. typescript-eslint/typescript-eslint#10322 -> typescript-eslint/typescript-eslint#10328.

Co-authored-by: @kirkwaiblinger

[absidue]

I think it is worth defining what should happen if something in that "ecosystem CI" breaks. Here are my two suggested policies:

Breakages caused by changes to internal/private ESLint APIs:

As ESLint explicitly discourages plugins from using them, these breakages should be considered non-critical. ESLint needs to be able to make changes to internal code, otherwise every single release would have to be a major one, which is unreasonable.

For these breakages the changes need to happen on the side of the 3rd party packages, so I would suggest setting a 2 week time limit for how long one of those ESLint changes could be pushed back, that gives the 3rd party packages time to release fixes. If the time limit expires and the affected packages haven't released updates yet, ESLint should be allowed to proceed anyway, the ESLint maintainers can then point out to complaining users that the 3rd party packages are using internal APIs and that they were given a reasonable amount of time to fix it. If packages release updates sooner than the time limit the changes can also be released sooner.

Packages that are not in the "ecosystem CI" should be disregarded and should not cause any delays to ESLint changes.

Breakages caused by changes to public APIs

These kinda of changes will likely require discussion, so it likely will be different on a case by case basis but the general rule for breaking changes to public APIs is to release them in major releases.

[SukkaW]

Breakages caused by changes to public APIs

These kinda of changes will likely require discussion, so it likely will be different on a case by case basis but the general rule for breaking changes to public APIs is to release them in major releases.

The recent defaultOptions breaking is a perfect example of a breaking change that failed to be caught during the review.

[absidue]

@SukkaW I presume you haven't actually read any of the threads on the topic, as mentioned by multiple ESLint and TypeScript ESLint maintainers the changes were to ESLint internals, exported via the use-at-your-own-risk export which ESLint highly discourages people using. It belongs in the first category.

[mdjermanovic]

I support this proposal.

If this is too heavyweight to put into individual PRs, it could alternately put only on the main branch. That way breakages would be discovered before a release.

I think the checks should run on PRs too. We have scheduled releases from the main branch every 2nd Friday, so catching a problem when the PR is already merged would be too late.

For these breakages the changes need to happen on the side of the 3rd party packages, so I would suggest setting a 2 week time limit for how long one of those ESLint changes could be pushed back, that gives the 3rd party packages time to release fixes. If the time limit expires and the affected packages haven't released updates yet, ESLint should be allowed to proceed anyway,

This sounds reasonable to me, for the benefit of end users.

@eslint/eslint-tsc thoughts?

[nzakas]

I'm not opposed to this idea, but I am concerned over the logistics.

Assuming we do this on PRs and not just on main, we could end up with a stack of PRs that are blocked on third-party packages. Even with a 2 week SLA for fixing those, what would practically happen when that breakage isn't addressed? Would we then remove the test? And would we then have to re-enable it later?

Also, who gets to decide what "popular" plugins are?

And what is the process for major releases, where we know that packages will be broken?

[JoshuaKGoldberg]

Even with a 2 week SLA for fixing those, what would practically happen when that breakage isn't addressed? Would we then remove the test? And would we then have to re-enable it later?

👍 from me on that as the process. I hope and think that most breakages would be resolved very quickly. Today, they already are typically resolved within a week even without the proposed early warning tests. If something takes longer, then I'd imagine an initial process like:

1. Have a team member file a tracking issue (followup issue: automate it)
2. If it isn't resolved in, say, 2 weeks:
   1. Remove the test
   2. File a followup issue to re-add it

...where filed issues should include a text copy of the failure and link to tracking issue in the downstream repository.

Also, who gets to decide what "popular" plugins are?

Qualifying criteria would be nice. Straw man proposal:

• 1 million npm downloads a week: arbitrary large size threshold to avoid a gluttony of small packages

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

And what is the process for major releases, where we know that packages will be broken?

That is tricky... Maybe to start, the tests should be disabled for any plugin that doesn't explicitly support the version of ESLint being tested? If they don't have a maximum major version in their (peer)dependencies then this could be hardcoded in the ESLint repo and updated after the release.

I'm proposing that only to unblock getting this in soon. Figuring out a better system for major releases would be very good.

[fasttime]

I don't feel strongly for this proposal. If the team's preference is for having integration tests, that's fine for me. I imagine we'll want to keep separate tests for each plugin so it's easier to disable and re-enable each test individually when the compatibility changes.

If I understand correctly, the cause for incident #19134 is that typescript-eslint must rely on implementation details of built-in rules to function correctly, which is an intrinsically fragile solution since those details can change with any release. I wonder if it would make sense to work with typescript-eslint to figure out how to improve this situation in the future.

[nzakas]

I think this is involved enough that we should do an RFC for the proposed process to see all of the details together. I think that's the only way we can make a solid decision regarding this.

[JoshuaKGoldberg]

I wonder if it would make sense to work with typescript-eslint to figure out how to improve this situation in the future.

We would love that ❤! I filed #19173.

RFC

👍 I can write that up. As breaker of defaultOptions, I feel a sense of responsibility for fixing the situation for the future.