fix eslint errors#69
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
This requires changes. While you've successfully set up the configuration files, the core functionality of the tools isn't working as required. There are two critical issues that need to be addressed:
-
Incorrect ESLint Configuration: In
eslint.config.mjs, multiple configurations are merged into a single object, causing rules to overwrite each other. Each configuration should be a separate element in the exported array. Also,eslint-config-prettiermust be the last element to correctly disable conflicting style rules. [CHECKLIST ITEM #1] -
Non-functional Pre-commit Hook: The formatting errors found in
tests/signUp/signUpPositive.spec.jsindicate that the Husky and lint-staged setup is not triggering Prettier before commits. The purpose of this setup is to automate formatting, so this needs to be fixed. [CHECKLIST ITEM #3]
Please correct these configurations to ensure the tools work as intended.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| /** @type {import('eslint').Linter.Config[]} */ | ||
| export default [ | ||
| { languageOptions: { globals: globals.node } }, | ||
| eslintConfigPrettier, |
There was a problem hiding this comment.
The eslint-config-prettier configuration should be placed last in the array. Its purpose is to disable ESLint rules that conflict with Prettier. Placing it before other configurations allows them to re-enable rules that eslint-config-prettier is intended to turn off.
| { | ||
| ...pluginJs.configs.recommended, | ||
| ...playwright.configs['flat/recommended'], | ||
| rules: { | ||
| ...pluginJs.configs.recommended.rules, | ||
| 'no-unused-vars': 'error', | ||
| 'max-len': [ | ||
| 'error', | ||
| { | ||
| code: 80, | ||
| comments: 80, | ||
| }, | ||
| ], | ||
| ...playwright.configs['flat/recommended'].rules, | ||
| 'playwright/expect-expect': 'off', | ||
| }, | ||
| ignores: [ | ||
| '**/node_modules/*', | ||
| 'playwright.config.js', | ||
| '**/playwright-report/**', | ||
| ], | ||
| }, |
There was a problem hiding this comment.
Merging multiple configurations into a single object like this is incorrect. When you spread configurations that both have a rules property (like pluginJs.configs.recommended and playwright.configs['flat/recommended']), their rules will overwrite each other. Then, your explicit rules property on line 14 overwrites them all. To correctly layer these configurations, each one should be a separate element in the exported array.
| test('Successful `Sign up` flow test', async ({ | ||
|
|
||
|
|
||
| }) => { |
There was a problem hiding this comment.
The formatting of this test function's parameters is incorrect. It contains extra newlines and spaces that should be removed.
This kind of formatting issue should be automatically fixed by Prettier. Since it's present in the committed code, it suggests that the Husky and lint-staged setup is not working correctly to format files before they are committed. Please verify your configuration to ensure the tools are working as expected.
2 participants
No description provided.