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

chore: move compat dependency to devDependencies #1187

Merged
merged 2 commits into from
Apr 4, 2025

Conversation

43081j
Copy link
Contributor

@43081j 43081j commented Apr 3, 2025

We no longer use it in production code, only in tests. So we can move this to devDependencies now.

We no longer use it in production code, only in tests. So we can move
this to `devDependencies` now.
Copy link

changeset-bot bot commented Apr 3, 2025

⚠️ No Changeset found

Latest commit: 4069ccc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

Copy link
Contributor

github-actions bot commented Apr 3, 2025

Try the Instant Preview in Online Playground

ESLint Online Playground

Install the Instant Preview to Your Local

npm i https://pkg.pr.new/eslint-plugin-svelte@4069ccc

Published Instant Preview Packages:

View Commit

@JounQin
Copy link
Collaborator

JounQin commented Apr 3, 2025

Is that really needed? We requires "eslint": "^8.57.1 || ^9.0.0".

@43081j
Copy link
Contributor Author

43081j commented Apr 3, 2025

from what i remember, in 8.57.1, the flat rule tester is in eslint/use-at-your-own-risk

the index exports the legacy rule tester

in 9.x, the index exports the flat rule tester, and use-at-your-own-risk doesn't export one at all

so we'd need a conditional import of some sort, or continue using the compat utils

@JounQin
Copy link
Collaborator

JounQin commented Apr 3, 2025

I personally think that's easy enough to use eslint.RuleTester || eslintUnsupportedApi.FlatRuleTester other than depending on a dep we don't use much.

@43081j
Copy link
Contributor Author

43081j commented Apr 3, 2025

ok i've pushed a change

it seems to work locally with eslint 8. please take a look

@43081j 43081j force-pushed the move-compat-dev branch from d7760ab to 4069ccc Compare April 3, 2025 11:51
@43081j 43081j requested a review from JounQin April 3, 2025 11:52
Copy link
Member

@baseballyama baseballyama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it’s fine, but let’s have @ota-meshi, the owner of eslint-compat-utils, confirm it as well.

@43081j
Copy link
Contributor Author

43081j commented Apr 3, 2025

when reviewing it, @ota-meshi, keep in mind this is only in tests.

your compat code does some patching up of FlatRuleTester but it doesn't seem we need that here

Copy link
Member

@ota-meshi ota-meshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's great that eslint-compat-utils is no longer needed! Thanks!

@ota-meshi ota-meshi merged commit f4cd4ca into sveltejs:main Apr 4, 2025
17 checks passed
@ota-meshi
Copy link
Member

Oh... We should have included the changeset since this is a dependency removal.

@ota-meshi ota-meshi mentioned this pull request Apr 4, 2025
@43081j 43081j deleted the move-compat-dev branch April 4, 2025 08:29
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.

4 participants