-
-
Notifications
You must be signed in to change notification settings - Fork 51
chore: upgrade ESLint to v9 #203
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #203 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 3 3
Lines 18 18
Branches 2 2
=========================================
Hits 18 18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
eslint.config.js
Outdated
| import configWebpack from "eslint-config-webpack"; | ||
|
|
||
| export default defineConfig([ | ||
| globalIgnores(["node_modules", "dist", "coverage", "test/fixtures"]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need these ignores, they are default in our eslint configuration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, right.
| import { compile } from "coffeescript"; | ||
|
|
||
| import schema from "./options.json"; | ||
| import CoffeeScriptError from "./CoffeeScriptError"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weird, ideally dirty should warn on this and ask about extension, dirty it is like preparing for ES modules, so no warnings for require, but yes for import, I will look today
Anyway good job, it is first dirty usage, so we will fix everything today and will start to migrate in other repos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like import/extensions doesn't have ability to control such things, i.e. you can't enable/disable it for es modules or commonjs, the rules work for ES modules and CommonJS together, we need to report it to them, so let's postpone it, when we will migrate to ES modules we will change config and catch all cases
|
@snitin315 Feel free to update other repos too |
This PR contains a:
Motivation / Use-Case
Update ESLint v9
Breaking Changes
No
Additional Info
No