-
Notifications
You must be signed in to change notification settings - Fork 507
Updates eslint + prettier and fixes warnings #77
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: master
Are you sure you want to change the base?
Updates eslint + prettier and fixes warnings #77
Conversation
Updates packages: - eslint : ^8.57.0 => ^9.35.0 - prettier : ^3.2.5 => 3.6.2 (pinned) - @vue/eslint-config-prettier : ^9.0.0 => ^10.2.0 Note: Uses '--save-exact' for prettier as per best practices Removes packages: - @rushstack/eslint-patch - eslint-plugin-vue Adds packages: - @vue/eslint-config-standard - eslint-config-flat-gitignore - eslint-config-prettier - eslint-plugin-prettier - eslint-plugin-tailwindcss - eslint-plugin-vue Regenerates package-lock.json: - 'npm cache clean --force' - 'rm -rf node_modules' - 'rm package-lock.json' - 'npm install' Migrates '.eslintrc.js' to 'eslint.config.mjs': - Exposes 'globals.browser' (leaves 'globals.node' commented) - Adds '.vscode' folder to GlobalIgnore rules - Adds GitIgnore rules - Adds TailwindCSS rules - Adds custom rules to align with project norms Addresses all remaining eslint and prettier warnings: - .prettierrc.json - CHANGELOG.md - index.html - jsconfig.json - vercel.json - src/assets/layout/\_preloading.scss - src/views/pages/Crud.vue - src/views/pages/Landing.vue - src/views/uikit/TableDoc.vue Adds/Updates 'package.json' scripts: - Removes deprecated '--ignore-path' from 'lint' script - Removes '--fix' from 'lint' script, leaving as a 'check' action - Adds 'lint-fix' script as a 'fix' action - Adds 'prettier' script as a 'check' action - Adds 'prettier-fix' script as a 'fix' action New script details: - lint : 'eslint . --ext .vue,.js,.jsx,.cjs,.mjs' - lint-fix : 'eslint . --ext .vue,.js,.jsx,.cjs,.mjs --fix' - prettier : 'prettier --check .' - prettier-fix : 'prettier --write .' Adds entries to '.gitignore': - 'dist-ssr' - 'components.d.ts' Adds '.prettierignore' file: - '.vscode/' - 'public/demo/data/' - 'src/assets/demo/flags/flags.css' Removes 'lang="ts"' from Landing.vue
|
Greetings! I maintain TekWizely/wails-template-primevue-sakai which integrates Sakai Vue as a GoLang Wails Starter Template, making it possible to write desktop apps with Sakai as the UI framework. In preparing to create a TypeScript conversion of the project, it became beneficial to upgrade to ESLint V9. I decided to see if I could route those changes back up through this project. That way my project doesn't have to stray away from its roots. Thanks in for creating/sharing Sakai Vue and please let me know if you have any questions about my PR. -DF ps: Hiding Whitespace will make the changes easier to review. The largest changes are to Almost all others are the result of fixing lint/prettier warnings. All non-whitespace changes were manually applied/reviewed, having prettier fix one a time then comparing to original. |
| "prettier": "^3.2.5", | ||
| "prettier": "3.6.2", |
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.
Version pinned according to official documentation.
| .env | ||
| dist | ||
| dist-ssr | ||
| components.d.ts |
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.
The official stance on whether to add components.d.ts to source control is mixed, with pros and cons on both sides, but since this project did not already have it, I erred on the don't-add-it side.
| @@ -1,17 +1,15 @@ | |||
| <!DOCTYPE html> | |||
| <!doctype html> | |||
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.
Prettier changed this to doctype and since the value is case-insensitive and there is no community consensus on UPPER vs lower, I left it.
#DontTauntHappyFunBall
| @@ -1,4 +1,4 @@ | |||
| <script setup lang="ts"> | |||
| <script setup> | |||
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 looked like a bug so I removed it ?
| expandedRows.value = products.value.reduce((acc, p) => (acc[p.id] = true) && acc, {}); | ||
| expandedRows.value = products.value.reduce((acc, p) => { | ||
| acc[p.id] = true; | ||
| return acc; | ||
| }, {}); |
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.
ESLint was NOT impressed by this clever one-liner - I manually coded the fix.
| <InputText v-model="filters['global'].value" placeholder="Search..." /> | ||
| <InputText v-model="filters.global.value" placeholder="Search..." /> |
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 changed this vs leaving the warning or disabling the warning, but happy to revert it and disable the warning as a 'project norm' override if you like?
| <InputText v-model="filters1['global'].value" placeholder="Keyword Search" /> | ||
| <InputText v-model="filters1.global.value" placeholder="Keyword Search" /> |
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 changed this vs leaving the warning or disabling the warning, but happy to revert it and disable the warning as a 'project norm' override if you like?
| **Implemented New Features and Enhancements** | ||
|
|
||
| - Update PrimeVue version | ||
| - Update PrimeVue version |
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.
As this file was small, and the fixes were only whitespace, I let prettier fix the file, but I'm happy to revert this and add it to the .prettierignore if you like?
Already covered in .gitignore, which prettier will inspect for ignore patterns.
| .vscode/ | ||
| public/demo/data/ | ||
| src/assets/demo/flags/flags.css |
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.
Just confirmed via prettier docs on ignoring that it picks up .gitignore when present, so we only need to add files here that are in the repository (since they likely won't be in .gitignore).
|
@tugcekucukoglu , @cagataycivici , @bahadirsofuoglu , @sezisfurkan : Any chance of getting some eyes on this? |
Updates packages:
Note: Uses '--save-exact' for prettier as per best practices
Removes packages:
Adds packages:
Regenerates package-lock.json:
Migrates '.eslintrc.js' to 'eslint.config.mjs':
Addresses all remaining eslint and prettier warnings:
Adds/Updates 'package.json' scripts:
New script details:
Adds entries to '.gitignore':
Adds '.prettierignore' file:
Removes 'lang="ts"' from Landing.vue