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
Update prettier and eslint. #387
Conversation
@@ -0,0 +1,4 @@ | |||
{ | |||
"trailingComma": "none", |
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 default values have changed since Prettier v1. I added a config to keep the previous default values in order to minimize the number of changes resolved the the eslint --fix
command. If we want to always stick with the prettier default values, I can remove this file once the review is complete. Let me know if there is any preference.
@@ -0,0 +1,134 @@ | |||
/* |
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 the new flat config file format will support from v9. Plugins need to change their code in order to work with v9. Right now I use a compatibility layer in order to keep everything still going.
Eslint v9 was released a few days ago. Even with the compatibility layer, some plugins (import or react) have rules that are breaking. I will stick for the moment with v8.57.0. When the plugins will be updated, we can upgrade to Eslint v9, and the migration should go faster since we already use the new config file.
2, | ||
{ namedComponents: "arrow-function" } | ||
], | ||
"import/no-named-as-default-member": "off", |
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 rules from the next two lines are not working even on v8.57.0. Once this Issue will be resolved, I will enable the rules back.
@@ -27,6 +27,7 @@ buildExtensionManifest() | |||
.then(() => { | |||
process.exit(0); | |||
}) | |||
.catch(() => { | |||
.catch(e => { | |||
console.error(e); |
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.
When the build script was failing, the error was not showed inside the console.
"production": { | ||
"plugins": [ | ||
"babel-plugin-jsx-remove-data-test-id", | ||
"@babel/proposal-nullish-coalescing-operator", |
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 file .babelrc was renamed to babel.config.js. The plugins @babel/proposal-nullish-coalescing-operator
and @babel/proposal-optional-chaining
are no longer maintained since the proposal was accepted in ES2020. Now there are 2 new plugins that I added in babel.config.js: @babel/plugin-transform-nullish-coalescing-operator
and @babel/plugin-transform-optional-chaining
.
const result = JSON.stringify(obj); | ||
if (!prettyPrint) { | ||
return result; | ||
} | ||
const prettierConfig = prettier.resolveConfig(); | ||
const prettierConfig = await prettier.resolveConfig(); |
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 API changed to async.
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.
Thank you for doing this. These tech debt tasks seem to fall by the wayside. I'm okay with moving towards the defaults as far as commas and function call parens. We should have a discussion as a team though.
Is there anything we can do to make the integration of Nina's PR easy with all these formatting changes? (i.e. what steps should she follow to merge this into her PR?)
I am ok keeping the Prettier defaults. I can remove the config and run the fix command. I wanted to minimize the number of file changes, so that the PR can be somewhat reviewable.
She told me that her commits contain mainly new files. I can help her with the rebase if this gets merged before her PR. Based on today's discussion it seems more changes are on the horizon. |
Description
This PR is for updating the ESLint to 8.57.0 and Prettier to 3.2.5. A lot of files were changed by the
eslint fix
command.Related Issue
Motivation and Context
We need to update the dependencies because right now the project can be executed only with node v18.
Screenshots (if appropriate):
Types of changes
Checklist: