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

Remove framework plugins from dependencies #923

Merged
merged 7 commits into from
Jan 23, 2024

Conversation

connor-baer
Copy link
Member

@connor-baer connor-baer commented Jan 23, 2024

Purpose

Over the years, Foundry has included more and more ESLint plugins for popular libraries. This has significantly increased its install size and led to several other issues:

  • Dependency resolution: Occasionally, Foundry/ESLint wouldn't be able to resolve plugins. I was never able to determine why, but it usually required explicitly installing the plugin in the host project.
  • Version lock-in: ESLint plugins usually follow the version of the corresponding library. This could block developers from upgrading the library if Foundry hadn't been upgraded to the latest version of the ESLint plugin. For example, the latest version of Next.js and its ESLint plugin is 14.x; however, Foundry still ships with v13.4 of the plugin.
  • Framework detection: Foundry would only enable a plugin if it detected the corresponding framework in the package.json file. This doesn't work well in monorepos.

Plugins:

Approach and changes

  • Remove the ESLint plugins for frameworks (Cypress, Emotion, Jest, Next, Playwright, Storybook, Testing Library) from the dependencies and ask developers to install them explicitly
  • Detect the installed ESLint plugins instead of frameworks to enable the shared configs
  • Log warnings if a recommended ESLint plugin is not installed or if an untested version is installed

Definition of done

  • Development completed
  • Reviewers assigned
  • Unit and integration tests

Copy link

changeset-bot bot commented Jan 23, 2024

🦋 Changeset detected

Latest commit: 26d390a

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

This PR includes changesets to release 1 package
Name Type
@sumup/foundry Major

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

Copy link

codecov bot commented Jan 23, 2024

Codecov Report

Attention: 53 lines in your changes are missing coverage. Please review.

Comparison is base (c721ad6) 63.50% compared to head (26d390a) 64.30%.

Files Patch % Lines
src/cli/debug.ts 0.00% 26 Missing and 1 partial ⚠️
src/cli/index.ts 0.00% 15 Missing ⚠️
src/configs/eslint/config.ts 83.33% 7 Missing ⚠️
src/lib/options.ts 96.49% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             next     #923      +/-   ##
==========================================
+ Coverage   63.50%   64.30%   +0.79%     
==========================================
  Files          26       27       +1     
  Lines        1973     2132     +159     
  Branches       95      113      +18     
==========================================
+ Hits         1253     1371     +118     
- Misses        712      752      +40     
- Partials        8        9       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/lib/options.ts Outdated Show resolved Hide resolved
src/lib/options.spec.ts Outdated Show resolved Hide resolved
@connor-baer connor-baer marked this pull request as ready for review January 23, 2024 14:09
@connor-baer connor-baer requested a review from a team as a code owner January 23, 2024 14:09
@connor-baer connor-baer requested review from tareqlol and larimaza and removed request for a team January 23, 2024 14:09
@connor-baer connor-baer merged commit ea7c264 into next Jan 23, 2024
5 checks passed
@connor-baer connor-baer deleted the feature/framework-plugins branch January 23, 2024 14:21
This was referenced Mar 4, 2024
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.

2 participants