Skip to content

Added - Eslint, Prettier, Husky, Fixed bugs#89

Open
WesafEnginer wants to merge 2 commits into
mate-academy:mainfrom
WesafEnginer:task_solution
Open

Added - Eslint, Prettier, Husky, Fixed bugs#89
WesafEnginer wants to merge 2 commits into
mate-academy:mainfrom
WesafEnginer:task_solution

Conversation

@WesafEnginer

Copy link
Copy Markdown

No description provided.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am requesting changes because some critical checklist requirements around tooling setup are not yet met. The ESLint flat config has structural issues: ignores is currently mixed into the same object that spreads pluginJs.configs.recommended and playwright.configs['flat/recommended'] in eslint.config.mjs, but with flat config it must be on its own top-level config entry (in the exported array) so that ESLint reliably ignores node_modules, the Playwright report directory, and playwright.config.js [CHECKLIST ITEM #1]. Also, eslint-config-prettier is being used as a standalone config object in the array instead of being incorporated via extends/spread into your main config; this can lead to unexpected behavior and does not match the recommended flat-config usage for integrating Prettier with ESLint [CHECKLIST ITEM #1]. In addition, the task requires a working Prettier, Husky, and lint-staged setup; there is no visible lint-staged configuration and the Husky pre-commit hook is not clearly wired to run npx lint-staged, so staged files may not actually be checked before commit, which breaks the “tools working as expected” requirement [CHECKLIST ITEM #1].

To fix this, you should: 1) move ignores into its own top-level config object in the eslint.config.mjs export array, 2) integrate eslint-config-prettier inside your main ESLint config via extends or a spread so that it just disables conflicting rules, and 3) add a proper lint-staged configuration (for example in package.json) and ensure .husky/pre-commit runs npx lint-staged so ESLint/Prettier are enforced on staged files. As a polish/medium improvement, consider explicitly applying Playwright’s languageOptions.globals in your flat config so test, expect, etc. are always recognized without extra globals warnings. Your Playwright page objects and tests themselves look clean and appropriate for the configured rules, so once you fix the configuration wiring and pre-commit integration, your solution will align well with the task requirements.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread eslint.config.mjs Outdated
Comment on lines +11 to +31
{
...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/**',
],

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This config object mixes in pluginJs.configs.recommended and playwright.configs['flat/recommended'] and then adds an ignores property; with ESLint flat config, ignores should be specified on a top‑level config entry and not potentially overridden or misplaced inside merged configs. To satisfy the checklist focus on ESLint configuration correctness, consider moving ignores to its own top‑level config object or ensuring it’s applied in a way ESLint will actually respect.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am approving your solution because all visible critical requirements around the ESLint flat configuration and the Playwright page objects/tests are correctly implemented and there are no confirmed blockers. The eslint.config.mjs now uses a proper flat-config structure with ignores in its own top-level object, and there is no longer any misuse of eslint-config-prettier, which satisfies the earlier high-priority ESLint issues. The page objects (HomePage, SignInPage, SignUpPage) and the Playwright tests are clean, use valid imports, and don’t introduce configuration or runtime problems related to the task.

The only remaining concern is that I cannot see your Prettier, Husky, and lint-staged configuration files here, so I can’t directly confirm that git commit runs npx lint-staged and blocks commits on ESLint errors as required, but there is no evidence of a misconfiguration in the files you shared. Please double-check that:

  • lint-staged is configured (usually in package.json) to run ESLint/Prettier on staged JS/test files, and
  • .husky/pre-commit invokes npx lint-staged so ESLint errors prevent commits as described in the checklist.
    Within the scope of the provided code, you’ve addressed the previously reported issues well and the tooling-related setup that is visible looks solid—nice work bringing the ESLint configuration into compliance and keeping the tests straightforward and consistent.

✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Comment thread src/pages/SignUpPage.js
this.usernameField = page.getByPlaceholder('Username');
this.emailField = page.getByPlaceholder('Email');
this.passwordField = page.getByPlaceholder('Password');
this.signUpButton = page.getByRole('button', { name: 'Sign up' });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ESLint flat config now places ignores in its own top-level object, which satisfies the requirement that global ignores be a standalone entry; however, per checklist item about tooling setup, make sure you’ve also integrated Prettier and lint-staged/Husky correctly elsewhere (e.g., package.json and .husky/pre-commit), since those configurations are not visible in this file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants