Skip to content
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

Swap to fork of eslint-plugin-cypress, add missing recommended rule #106

Merged
merged 5 commits into from
May 25, 2023

Conversation

askoufis
Copy link
Contributor

@askoufis askoufis commented May 24, 2023

eslint-plugin-cypress doesn't support ESLint v8. There is an issue for this on the repo with no activity. There is a plan to uplift and move the package into cypress' monorepo, however there has been no movement on that issue for some months.

Since most users of cypress will likely be using cypress alongside a sku app, this has forced sku to stay on ESLint v7. We would like to update sku to use ESLint v8, and luckily for us there is a fork of eslint-plugin-cypress that supports it. Unfortunately, that package too hasn't been updated for a few months, and it is missing one of the recommended rules provided by the original eslint-plugin-cypress. I have made a PR to add this missing rule.

In the meantime (either until the original package is updated/moved, or the fork is updated), in order to enable sku to update to ESLint v8, without dropping support for an existing lint rule, I've done two things:

  • Swapped to the eslint-plugin-cypress fork @finsit/eslint-plugin-cypress
  • Copied the missing rule from the original eslint-plugin-cypress and exposed it from this package via eslint-plugin-local-rules. This was the simplest way I found to expose a rule from a plugin without having to create and publish a separate package (e.g. eslint-plugin-seek). Since this rule hopefully won't stick around for too long, I opted for this solution rather than creating a package for the rule.

I've added a note to the changeset for consumers that are overriding cypress rules mentioning that they need to update the references to those rules. I'm not considering this a breaking change since this config is usually not consumed directly but rather via a framework tool as sku or skuba.

EDIT: Another option could have been to make our own fork or eslint-plugin-cypress. However, since the use of a fork is hopefully going to be temporary, it didn't feel worth the effort to fork the repo, set up CI, deploy a package, and add the new rule, when it was easy enough to get the rule working in this repo.

@askoufis askoufis requested a review from a team as a code owner May 24, 2023 07:00
@changeset-bot
Copy link

changeset-bot bot commented May 24, 2023

🦋 Changeset detected

Latest commit: f866067

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
eslint-config-seek Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@askoufis askoufis requested a review from a team May 24, 2023 07:00
base.js Show resolved Hide resolved
.changeset/funny-garlics-double.md Outdated Show resolved Hide resolved
Co-authored-by: Remus Mate <rmate@seek.com.au>
@askoufis askoufis added this pull request to the merge queue May 25, 2023
@askoufis askoufis removed this pull request from the merge queue due to a manual request May 25, 2023
@askoufis askoufis added this pull request to the merge queue May 25, 2023
Merged via the queue into master with commit 8ef81c1 May 25, 2023
@askoufis askoufis deleted the cypress-fork branch May 25, 2023 03:46
@seek-oss-ci seek-oss-ci mentioned this pull request May 25, 2023
72636c added a commit that referenced this pull request Apr 22, 2024
I noticed that they cut a new release, is this sufficient for us to
revert back?

https://github.com/cypress-io/eslint-plugin-cypress/releases/tag/v3.0.0

See #106 and #111 for historical context.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants