-
Notifications
You must be signed in to change notification settings - Fork 53
refactor: Update ESLint configurations for ESLint 9 compatibility #3788
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
base: eslint9
Are you sure you want to change the base?
Changes from all commits
1fac68e
c1477c8
bd2f3ac
c66e62c
7fac182
f35cfb5
55df0ae
8199633
3480b33
d362fc1
bbb5465
aa99a67
04e46c9
ea05943
68ac224
23ab068
0aacfbc
1e78d55
275b471
f61d11a
b0319b0
c4d60e8
4a4c24c
140edee
76c7869
7761931
928b1bb
2583868
fea5e75
3d745cc
471fdb7
d2eddac
c32dd5d
af68a06
719dbb3
ca09977
b2a4831
28e9eae
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,8 +7,9 @@ module.exports = defineConfig([ | |
| languageOptions: { | ||
| globals: { | ||
| ...globals.browser, | ||
| ...globals.node | ||
| ...globals.node, | ||
| 'sap': 'readonly' | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of curiosity: why is this needed?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addresses error |
||
| } | ||
| } | ||
| } | ||
| ]); | ||
| ]); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| const { defineConfig } = require('eslint/config'); | ||
| const babelParser = require('@babel/eslint-parser'); | ||
| const js = require('@eslint/js'); | ||
|
|
||
| module.exports = defineConfig([ | ||
| { | ||
| files: ['./webapp/**/*.js', './webapp/**/*.ts'], | ||
|
|
||
| ignores: [ | ||
| 'target/**', | ||
| 'webapp/test/**', | ||
| 'webapp/localservice/**', | ||
| 'backup/**', | ||
| '**/Gruntfile.js', | ||
| '**/changes_preview.js', | ||
| '**/changes_preview.ts', | ||
| '**/gulpfile.js', | ||
| '**/*.d.ts', | ||
| 'test/**' | ||
| ], | ||
|
|
||
| languageOptions: { | ||
| parser: babelParser, | ||
| parserOptions: { | ||
| requireConfigFile: false | ||
| } | ||
| }, | ||
|
|
||
| plugins: { | ||
| '@sap-ux/fiori-tools': require('../lib/index.js') | ||
| }, | ||
|
|
||
| rules: { | ||
| ...js.configs.recommended.rules, | ||
| '@sap-ux/fiori-tools/sap-no-global-variable': 'error', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It may make sense to sort them. AI can help here :) rules: {
...js.configs.recommended.rules,
// Error rules
'@sap-ux/fiori-tools/sap-no-global-variable': 'error',
'@sap-ux/fiori-tools/sap-no-hardcoded-color': 'error',
'@sap-ux/fiori-tools/sap-no-hardcoded-url': 'error',
'@sap-ux/fiori-tools/sap-no-localstorage': 'error',
'@sap-ux/fiori-tools/sap-no-override-rendering': 'error',
'@sap-ux/fiori-tools/sap-no-override-storage-prototype': 'error',
'@sap-ux/fiori-tools/sap-no-sessionstorage': 'error',
'@sap-ux/fiori-tools/sap-no-ui5base-prop': 'error',
'@sap-ux/fiori-tools/sap-no-absolute-component-path': 'error',
'@sap-ux/fiori-tools/sap-no-location-reload': 'error',
'@sap-ux/fiori-tools/sap-no-global-event': 'error',
'@sap-ux/fiori-tools/sap-no-exec-command': 'error',
'@sap-ux/fiori-tools/sap-no-br-on-return': 'error',
'@sap-ux/fiori-tools/sap-no-dynamic-style-insertion': 'error',
'@sap-ux/fiori-tools/sap-no-element-creation': 'error',
'@sap-ux/fiori-tools/sap-no-global-define': 'error',
'@sap-ux/fiori-tools/sap-no-navigator': 'error',
'@sap-ux/fiori-tools/sap-no-inner-html-write': 'error',
'@sap-ux/fiori-tools/sap-no-commons-usage': 'error',
// Warning rules
'@sap-ux/fiori-tools/sap-no-jquery-device-api': 'warn',
'@sap-ux/fiori-tools/sap-message-toast': 'warn',
'@sap-ux/fiori-tools/sap-no-ui5-prop-warning': 'warn',
'@sap-ux/fiori-tools/sap-no-localhost': 'warn',
'@sap-ux/fiori-tools/sap-usage-basemastercontroller': 'warn',
'@sap-ux/fiori-tools/sap-no-encode-file-service': 'warn',
'@sap-ux/fiori-tools/sap-no-dom-insertion': 'warn',
'@sap-ux/fiori-tools/sap-cross-application-navigation': 'warn',
'@sap-ux/fiori-tools/sap-no-location-usage': 'warn',
'@sap-ux/fiori-tools/sap-timeout-usage': 'warn',
'@sap-ux/fiori-tools/sap-no-proprietary-browser-api': 'warn',
'@sap-ux/fiori-tools/sap-no-dom-access': 'warn',
'@sap-ux/fiori-tools/sap-no-history-manipulation': 'warn',
'@sap-ux/fiori-tools/sap-no-global-selection': 'warn',
'@sap-ux/fiori-tools/sap-forbidden-window-property': 'warn',
'@sap-ux/fiori-tools/sap-no-inner-html-access': 'warn',
'@sap-ux/fiori-tools/sap-bookmark-performance': 'warn',
'@sap-ux/fiori-tools/sap-browser-api-warning': 'warn',
'@sap-ux/fiori-tools/sap-ui5-legacy-jquerysap-usage': 'warn',
'@sap-ux/fiori-tools/sap-ui5-global-eval': 'warn',
'@sap-ux/fiori-tools/sap-ui5-legacy-factories': 'warn',
'@sap-ux/fiori-tools/sap-ui5-forms': 'warn',
// Off rules
'@sap-ux/fiori-tools/sap-ui5-no-private-prop': 'off',
'@sap-ux/fiori-tools/sap-browser-api-error': 'off',
'@sap-ux/fiori-tools/sap-no-window-alert': 'off'
}
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Group by error, warn and off and sort within by rule name?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Group by error, warn and off.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the benefit of grouping them by severity because that is something that can change? I can see a scenario where you introduce a new rule and start as warning and later on change it to error, but then you have to also reorder them and that seems a bit strange.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Benefit for me is structure. With current implementation, yes correct if adding new rule, changing existing one does not matter because we have no structure. My taste: it is difficult to get an overview of which rules has which severity. With structure in place, adding or changing severity should be adapted in respective group. That is the purpose. |
||
| '@sap-ux/fiori-tools/sap-no-jquery-device-api': 'warn', | ||
| '@sap-ux/fiori-tools/sap-no-hardcoded-color': 'error', | ||
| '@sap-ux/fiori-tools/sap-no-hardcoded-url': 'error', | ||
| '@sap-ux/fiori-tools/sap-no-localstorage': 'error', | ||
| '@sap-ux/fiori-tools/sap-no-override-rendering': 'error', | ||
| '@sap-ux/fiori-tools/sap-no-override-storage-prototype': 'error', | ||
| '@sap-ux/fiori-tools/sap-no-sessionstorage': 'error', | ||
| '@sap-ux/fiori-tools/sap-no-ui5base-prop': 'error', | ||
| '@sap-ux/fiori-tools/sap-message-toast': 'warn', | ||
| '@sap-ux/fiori-tools/sap-no-ui5-prop-warning': 'warn', | ||
| '@sap-ux/fiori-tools/sap-no-localhost': 'warn', | ||
| '@sap-ux/fiori-tools/sap-usage-basemastercontroller': 'warn', | ||
| '@sap-ux/fiori-tools/sap-no-absolute-component-path': 'error', | ||
| '@sap-ux/fiori-tools/sap-no-encode-file-service': 'warn', | ||
| '@sap-ux/fiori-tools/sap-no-dom-insertion': 'warn', | ||
| '@sap-ux/fiori-tools/sap-cross-application-navigation': 'warn', | ||
| '@sap-ux/fiori-tools/sap-no-location-usage': 'warn', | ||
| '@sap-ux/fiori-tools/sap-timeout-usage': 'warn', | ||
| '@sap-ux/fiori-tools/sap-no-proprietary-browser-api': 'warn', | ||
| '@sap-ux/fiori-tools/sap-no-dom-access': 'warn', | ||
| '@sap-ux/fiori-tools/sap-no-history-manipulation': 'warn', | ||
| '@sap-ux/fiori-tools/sap-no-global-selection': 'warn', | ||
| '@sap-ux/fiori-tools/sap-no-location-reload': 'error', | ||
| '@sap-ux/fiori-tools/sap-no-global-event': 'error', | ||
| '@sap-ux/fiori-tools/sap-no-exec-command': 'error', | ||
| '@sap-ux/fiori-tools/sap-no-br-on-return': 'error', | ||
| '@sap-ux/fiori-tools/sap-no-dynamic-style-insertion': 'error', | ||
| '@sap-ux/fiori-tools/sap-no-element-creation': 'error', | ||
| '@sap-ux/fiori-tools/sap-no-global-define': 'error', | ||
| '@sap-ux/fiori-tools/sap-forbidden-window-property': 'warn', | ||
| '@sap-ux/fiori-tools/sap-no-navigator': 'error', | ||
| '@sap-ux/fiori-tools/sap-no-inner-html-write': 'error', | ||
| '@sap-ux/fiori-tools/sap-no-inner-html-access': 'warn', | ||
| '@sap-ux/fiori-tools/sap-bookmark-performance': 'warn', | ||
| '@sap-ux/fiori-tools/sap-no-commons-usage': 'error', | ||
| '@sap-ux/fiori-tools/sap-ui5-no-private-prop': 'off', | ||
| '@sap-ux/fiori-tools/sap-browser-api-error': 'off', | ||
| '@sap-ux/fiori-tools/sap-browser-api-warning': 'warn', | ||
| '@sap-ux/fiori-tools/sap-no-window-alert': 'off', | ||
| '@sap-ux/fiori-tools/sap-ui5-legacy-jquerysap-usage': 'warn', | ||
| '@sap-ux/fiori-tools/sap-ui5-global-eval': 'warn', | ||
| '@sap-ux/fiori-tools/sap-ui5-legacy-factories': 'warn', | ||
| '@sap-ux/fiori-tools/sap-ui5-forms': 'warn' | ||
| } | ||
| } | ||
| ]); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| const { defineConfig } = require('eslint/config'); | ||
| const babelParser = require('@babel/eslint-parser'); | ||
|
|
||
| module.exports = defineConfig([{ | ||
| files: ['webapp/test/**/*.js', 'webapp/test/**/*.ts'], | ||
| ignores: ['**/*.d.ts'], | ||
|
|
||
| languageOptions: { | ||
| parser: babelParser, | ||
| parserOptions: { | ||
| requireConfigFile: false | ||
| } | ||
| }, | ||
|
|
||
| plugins: { | ||
| '@sap-ux/fiori-tools': require('../lib/index.js') | ||
| }, | ||
|
|
||
| rules: { | ||
| '@sap-ux/fiori-tools/sap-opa5-autowait-true': 'warn' | ||
| } | ||
| }]); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| const { | ||
| defineConfig, | ||
| } = require("eslint/config"); | ||
|
|
||
| const typescriptEslint = require("@typescript-eslint/eslint-plugin"); | ||
| const tsParser = require("@typescript-eslint/parser"); | ||
| const js = require("@eslint/js"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| const tseslint = require('typescript-eslint'); | ||
|
|
||
|
|
||
|
|
||
| module.exports = defineConfig([ | ||
| { | ||
| files: ["./webapp/*.ts", "./webapp/**/*.ts"], | ||
|
|
||
| ignores: [ | ||
| "target/**", | ||
| "webapp/test/changes_loader.ts", | ||
| "webapp/test/changes_preview.ts", | ||
devinea marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| "webapp/localservice/**", | ||
| "webapp/localService/**", | ||
| "undefined/**/Example.qunit.ts", | ||
| "backup/**", | ||
| "**/*.d.ts", | ||
| ], | ||
|
|
||
| plugins: { | ||
| "@typescript-eslint": typescriptEslint, | ||
| }, | ||
|
|
||
| languageOptions: { | ||
| parser: tsParser, | ||
| ecmaVersion: 5, | ||
| sourceType: "script", | ||
|
|
||
| parserOptions: { | ||
| projectService: true, | ||
| }, | ||
| }, | ||
|
|
||
| rules: { | ||
| ...typescriptEslint.configs.recommended.rules, | ||
| ...typescriptEslint.configs['recommended-type-checked'].rules, | ||
| "@typescript-eslint/no-unsafe-call": "warn", | ||
| "@typescript-eslint/no-unsafe-member-access": "warn", | ||
| "@typescript-eslint/no-unsafe-return": "warn", | ||
| "@typescript-eslint/no-unsafe-argument": "warn", | ||
| "@typescript-eslint/no-unsafe-assignment": "warn", | ||
| }, | ||
| } | ||
| ]); | ||
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
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.
this is meant for only current PR or we plane to have another branch?
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 also have this PR #3827
It can be removed later.