Switch from eslintrc.json to es.config.js #2483
Conversation
Our Pull Request Approval ProcessWe have these basic policies to make the approval process smoother for our volunteer team. Testing Your CodePlease make sure your code passes all tests. Our test code coverage system will fail if these conditions occur:
The process helps maintain the overall reliability of the code base and is a prerequisite for getting your PR approved. Assigned reviewers regularly review the PR queue and tend to focus on PRs that are passing. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
|
Warning Rate limit exceeded@Suyash878 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 54 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes primarily involve transitioning from a deprecated ESLint configuration file format ( Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
Files selected for processing (12)
- .husky/pre-commit (1 hunks)
- CONTRIBUTING.md (2 hunks)
- README.md (2 hunks)
- config/vitestSetup.ts (1 hunks)
- eslint.config.mjs (1 hunks)
- package.json (2 hunks)
- setup.ts (2 hunks)
- src/resolvers/Mutation/updateAgendaCategory.ts (1 hunks)
- tests/helpers/userAndOrg.ts (1 hunks)
- tests/resolvers/UserTag/childTags.spec.ts (1 hunks)
- tests/resolvers/UserTag/usersAssignedTo.spec.ts (1 hunks)
- tests/resolvers/middleware/currentUserExists.spec.ts (1 hunks)
Files skipped from review due to trivial changes (8)
- CONTRIBUTING.md
- README.md
- config/vitestSetup.ts
- setup.ts
- src/resolvers/Mutation/updateAgendaCategory.ts
- tests/helpers/userAndOrg.ts
- tests/resolvers/UserTag/usersAssignedTo.spec.ts
- tests/resolvers/middleware/currentUserExists.spec.ts
Additional context used
Learnings (1)
tests/resolvers/UserTag/childTags.spec.ts (2)
Learnt from: meetulr PR: PalisadoesFoundation/talawa-api#2460 File: tests/resolvers/Mutation/assignUserTag.spec.ts:35-44 Timestamp: 2024-08-15T13:37:37.956Z Learning: The variable `testSubTag1` is used in the `assignUserTag` test suite to verify that ancestor tags are correctly assigned.Learnt from: meetulr PR: PalisadoesFoundation/talawa-api#2460 File: tests/resolvers/Mutation/unassignUserTag.spec.ts:32-37 Timestamp: 2024-08-15T13:37:28.099Z Learning: The variable `testSubTag1` is used in the `unassignUserTag` tests to verify the unassignment of child tags.
Additional comments not posted (6)
.husky/pre-commit (1)
1-1: Potential execution issues due to script modification.The removal of the shebang line and the line that sources the Husky script could potentially break the execution of this pre-commit hook. Ensure that the script is executable and functions correctly in all intended environments.
Run the following script to verify the script's functionality:
Also applies to: 7-7
tests/resolvers/UserTag/childTags.spec.ts (1)
1-1: Consider the implications of removing the ESLint non-null assertion directive.The removal of the
/* eslint-disable @typescript-eslint/no-non-null-assertion */directive could lead to stricter TypeScript checks and potentially more robust code. However, ensure that this change does not introduce errors in parts of the codebase that rely on non-null assertions.Run the following script to verify the impact of this change on the codebase:
Verification successful
Review the use of non-null assertions in test files.
The non-null assertions are primarily used in test files, which suggests that their removal might not significantly impact production code. However, ensure that these assertions are used safely and that tests do not make unsafe assumptions about data presence.
- Review the following test files for safe use of non-null assertions:
tests/resolvers/Post/creator.spec.tstests/resolvers/Post/comments.spec.tstests/resolvers/Query/getEventAttendee.spec.tstests/resolvers/Mutation/registerForEvent.spec.tstests/resolvers/Mutation/createFundCampaignPledge.spec.tstests/resolvers/Mutation/updateUserTag.spec.tstests/resolvers/Mutation/addEventAttendee.spec.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of removing the ESLint non-null assertion directive. # Test: Search for non-null assertions in the TypeScript files. Expect: No errors related to non-null assertions. rg --type ts '!'Length of output: 117561
Script:
#!/bin/bash # Description: Search for non-null assertions in TypeScript files. # Test: Search for non-null assertions specifically. Expect: Instances where `!` is used for non-null assertions. rg --type ts '\w+!\.'Length of output: 2660
eslint.config.mjs (1)
1-135: Comprehensive and well-structured ESLint configuration.The new
eslint.config.mjsfile is well-structured and appears to comprehensively address various aspects of linting and code quality, especially for TypeScript projects. Ensure that this configuration is thoroughly tested to confirm its compatibility with the existing codebase and does not introduce any unintended side effects.package.json (3)
102-104: Dependency additions for ESLint are appropriate.The addition of
@eslint/compat,@eslint/eslintrc, and@eslint/jsaligns with the PR's objective to enhance ESLint configuration and compatibility. These packages are essential for the new ESLint setup.
132-132: Update ofeslintversion is justified.Upgrading
eslintfrom^8.56.0to^9.9.0is necessary to support the new configuration format and potentially includes improvements and new features that are beneficial for the project.
137-137: Addition ofglobalspackage.The introduction of the
globalspackage at version^15.9.0is likely intended to enhance the global variables recognition by ESLint, which can be crucial for linting in complex projects. Ensure that this aligns with the project's linting requirements.Verification successful
Globals package is used in ESLint configuration.
The
globalspackage is imported and utilized ineslint.config.mjsto configure global variables for Node.js, aligning with the project's linting requirements. No further action is needed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the necessity of the `globals` package in the new ESLint configuration. # Test: Search for the usage of `globals` in the new ESLint configuration. Expect: References to global variable configurations. rg --type json -A 10 $'globals' ./eslint.config.jsLength of output: 108
Script:
#!/bin/bash # Search for ESLint configuration files in the repository. fd --type f --name ".eslintrc*" # Check for references to the `globals` package in any found ESLint configuration files. rg 'globals' $(fd --type f --name ".eslintrc*")Length of output: 5360
|
After successful merging of this PR, I will go ahead and create a similar issue in the Talawa-Admin repository as well. |
|
Please fix the conflicting file so we may merge |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- .husky/pre-commit (1 hunks)
Additional comments not posted (1)
.husky/pre-commit (1)
23-23: Clarification needed on the removal ofgit add .The removal of
git add .from the pre-commit hook script could lead to issues where changes made by commands earlier in the script are not staged for commit. This change might cause developers to commit incomplete or incorrect code states if they are not manually adding changes to the staging area.It's important to understand the rationale behind this removal:
- If the intention is to make developers manually stage files, this should be clearly communicated to avoid confusion.
- If this is an oversight, consider adding the command back or implementing another mechanism to ensure all changes are staged.
…-lock.json to resolve version related conflicts
…into issue#2472
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (1)
delete2.sh (1)
2-2: Use double quotes around the variable in theforloop.The
forloop is not using double quotes around the variableCHANGED_UNAUTH_FILES, which can cause word splitting and globbing issues.Add double quotes around the variable:
-for file in ${CHANGED_UNAUTH_FILES}; do +for file in "${CHANGED_UNAUTH_FILES}"; do
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
Files selected for processing (5)
- .husky/pre-commit (2 hunks)
- delete.sh (1 hunks)
- delete2.sh (1 hunks)
- eslint.config.mjs (1 hunks)
- package.json (2 hunks)
Files skipped from review due to trivial changes (1)
- package.json
Files skipped from review as they are similar to previous changes (1)
- eslint.config.mjs
Additional context used
Shellcheck
delete2.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
delete.sh
[warning] 3-3: CODECOV_UNIQUE_NAME appears unused. Verify use (or export if used externally).
(SC2034)
Additional comments not posted (2)
delete.sh (1)
7-7: LGTM!Running ESLint on changed files is a good practice to ensure code quality.
.husky/pre-commit (1)
20-20: LGTM!The change to stage all files using
git add .is approved.However, please verify the impact of removing the line sourcing the Husky script on the execution of the pre-commit hook. The AI-generated summary indicates that this removal may disrupt the execution of subsequent commands.
Run the following script to verify the execution of the pre-commit hook:
Verification successful
Pre-commit hook executes successfully
The pre-commit hook executed without any errors, indicating that the removal of the line sourcing the Husky script did not disrupt its functionality. The configuration is working as expected.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the execution of the pre-commit hook. # Test: Simulate a commit and check if the pre-commit hook is executed successfully. # Expect: The pre-commit hook should be executed without any errors. git commit --allow-empty -m "Test commit"Length of output: 81
.husky/pre-commit
Outdated
| @@ -3,9 +3,6 @@ | |||
| # Disable the hooks in CI | |||
| [ -n "$CI" ] && exit 0 | |||
|
|
|||
| # Change to the current directory | |||
| . "$(dirname -- "$0")/_/husky.sh" | |||
| @@ -132,6 +135,7 @@ | |||
| "eslint-plugin-import": "^2.29.1", | |||
| "eslint-plugin-tsdoc": "^0.3.0", | |||
| "get-graphql-schema": "^2.1.2", | |||
| "globals": "^15.9.0", | |||
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
Files selected for processing (1)
- package.json (2 hunks)
Files skipped from review due to trivial changes (1)
- package.json
|
I will open a new PR, since this one has a lot of unnecessary file changes and conflicts. |
|
I was working on a newer branch for opening a new PR regarding this issue. Now in order to migrate and properly use |
|
Please use your initiative to achieve the goal |
What kind of change does this PR introduce?
Bug fix
Issue Number:
Fixes #2472
Did you add tests for your changes?
Not relevant
Snapshots/Videos:
If relevant, did you update the documentation?
Not relevant here.
Summary
This PR migrates the deprecated version of
eslintrc.jsontoeslint.config.js.Now eslint will correctly fix all the linting issues which was not the case earlier as mentioned in the screenshot of the issue raised.
Does this PR introduce a breaking change?
Not sure if relevant here.
Other information
None
Have you read the contributing guide?
Yes
Summary by CodeRabbit
Summary by CodeRabbit
New Features
CONTRIBUTING.mdfile to enhance resource accessibility for contributors.Bug Fixes
Documentation
README.mdto enhance visual structure.CONTRIBUTING.mddocument.Refactor
Chores
package.jsonto improve linting capabilities and ensure compatibility with modern JavaScript features.