Automate Linting with Husky and lint-staged to Prevent CI Failures
#893
Conversation
- Ensured all sub-projects follow the unified ESLint rules from the root configuration. - Moved .eslintignore to the project root for centralized management. - Updated scripts to reference the root .eslintignore file directly. - Simplified path handling logic by removing unnecessary relative path calculations.
WalkthroughThis update introduces several changes to the project's linting and configuration setup. A new ESLint configuration file and ignore file have been added, along with a pre-commit hook using Husky to automate linting checks before commits. Additionally, various TypeScript dependencies have been removed across multiple example projects, indicating a shift away from TypeScript usage. The CI workflow has also been simplified to streamline the linting process. 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
CodeRabbit Configuration File (
|
![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
Outside diff range, codebase verification and nitpick comments (3)
examples/nextjs-scheduler/.eslintrc.js (2)
10-10: Ensure proper handling of links and consider the impact on the user experience and performance.Disabling the
@next/next/no-html-link-for-pagesrule allows developers to use HTML links without triggering linting errors. While this change may provide more flexibility in how links are implemented, it's important to consider the following:
- Ensure that the links are properly handled and navigate to the correct pages.
- Assess the impact on the user experience, such as page load times and navigation behavior.
- Evaluate the performance implications of using HTML links instead of Next.js's
Linkcomponent.Consider conducting thorough testing and monitoring to ensure that the change does not introduce any unexpected issues or degrade the overall user experience and performance.
2-2: Missing ESLint configuration file at specified pathThe ESLint configuration file does not exist at the specified relative path
'../../.eslintrc.js'. Please verify the path or ensure that the configuration file is added at the specified location to maintain consistent linting rules across the project.
- Verify the path
'../../.eslintrc.js'or add the missing ESLint configuration file.Analysis chain
Verify the existence of the ESLint configuration file.
Ensure that the ESLint configuration file exists at the specified relative path
'../../.eslintrc.js'.Run the following script to verify the existence of the file:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the ESLint configuration file. # Test: Check if the file exists. Expect: The file should exist. if [ -f "../../.eslintrc.js" ]; then echo "The ESLint configuration file exists at the specified relative path." else echo "The ESLint configuration file does not exist at the specified relative path." fiLength of output: 194
packages/sdk/.eslintrc.js (1)
4-4: Consider the implications of removing TypeScript-related plugins.The removal of TypeScript-related plugins may affect the code quality and consistency. Please consider the following:
- Discuss with the team if the removal aligns with the project's goals and coding standards.
- Evaluate if the remaining rules in the external configuration file (
../../.eslintrc.js) are sufficient to maintain code quality.- Consider gradually introducing TypeScript-related plugins and rules to improve type safety and catch potential issues.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (21)
- .eslintignore (1 hunks)
- .eslintrc.js (1 hunks)
- .github/workflows/ci.yml (1 hunks)
- .husky/pre-commit (1 hunks)
- examples/nextjs-scheduler/.eslintrc.js (1 hunks)
- examples/nextjs-scheduler/package.json (1 hunks)
- examples/react-tldraw/package.json (1 hunks)
- examples/react-todomvc/package.json (1 hunks)
- examples/vanilla-codemirror6/package.json (1 hunks)
- examples/vanilla-quill/package.json (1 hunks)
- lint-staged.config.js (1 hunks)
- package.json (2 hunks)
- packages/devtools/package.json (1 hunks)
- packages/sdk/.eslintrc.js (1 hunks)
- packages/sdk/package.json (3 hunks)
- packages/sdk/src/document/crdt/tree.ts (1 hunks)
- packages/sdk/src/document/document.ts (3 hunks)
- packages/sdk/src/document/json/array.ts (4 hunks)
- packages/sdk/src/util/error.ts (1 hunks)
- packages/sdk/test/helper/helper.ts (1 hunks)
- packages/sdk/test/unit/document/crdt/counter_test.ts (1 hunks)
Files skipped from review due to trivial changes (14)
- .eslintignore
- .github/workflows/ci.yml
- .husky/pre-commit
- examples/react-tldraw/package.json
- examples/react-todomvc/package.json
- examples/vanilla-codemirror6/package.json
- examples/vanilla-quill/package.json
- packages/sdk/package.json
- packages/sdk/src/document/crdt/tree.ts
- packages/sdk/src/document/document.ts
- packages/sdk/src/document/json/array.ts
- packages/sdk/src/util/error.ts
- packages/sdk/test/helper/helper.ts
- packages/sdk/test/unit/document/crdt/counter_test.ts
Additional comments not posted (19)
packages/sdk/.eslintrc.js (1)
3-3: Verify the existence and content of the external ESLint configuration file.Ensure that the file
../../.eslintrc.jsexists and contains the necessary rules for this project.Run the following script to verify the existence and content of the external configuration file:
examples/nextjs-scheduler/package.json (2)
24-24: LGTM!The code changes are approved.
Line range hint
1-27: Verify the removal ofeslint-plugin-prettierandtypescriptdependencies.The AI-generated summary and the list of alterations indicate the removal of
eslint-plugin-prettierandtypescriptdependencies. Since the provided code is the final state after the changes, it does not display the removed dependencies.To verify the removal of the dependencies, run the following script:
Verification successful
Dependencies
eslint-plugin-prettierandtypescripthave been removed.The verification confirms that the dependencies
eslint-plugin-prettierandtypescriptare not present in theexamples/nextjs-scheduler/package.jsonfile, indicating their successful removal.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the removal of `eslint-plugin-prettier` and `typescript` dependencies. # Test: Search for the removed dependencies. Expect: No occurrences. rg --type json $'eslint-plugin-prettier|typescript' examples/nextjs-scheduler/package.jsonLength of output: 91
packages/devtools/package.json (1)
30-30: Verify the impact of removing thetypescriptdependency.The removal of the
typescriptdependency could impact the development workflow if TypeScript is essential for type-checking or compiling the code.Please ensure that the removal of the
typescriptdependency does not break the build or development process. If TypeScript is still required, consider the following alternative solutions:
- Move the
typescriptdependency to the rootpackage.jsonfile to manage it centrally for the entire monorepo.- If TypeScript is only required for development purposes, move the
typescriptdependency to thedevDependenciessection of the rootpackage.jsonfile.Run the following script to verify the impact of removing the
typescriptdependency:lint-staged.config.js (4)
1-3: LGTM!The code changes are approved.
5-18: LGTM!The code changes are approved.
20-39: LGTM!The code changes are approved.
24-37: LGTM!The code changes are approved.
package.json (10)
19-20: LGTM!The code changes are approved.
20-20: LGTM!The code changes are approved.
21-21: LGTM!The code changes are approved.
30-30: LGTM!The code changes are approved.
31-31: LGTM!The code changes are approved.
32-32: LGTM!The code changes are approved.
34-35: LGTM!The code changes are approved.
36-37: LGTM!The code changes are approved.
38-38: LGTM!The code changes are approved.
33-33: LGTM!The code changes are approved.
.eslintrc.js (1)
1-61: LGTM!The ESLint configuration looks good:
- It enables several plugins and extends recommended configurations, providing a solid foundation for linting.
- The custom rules enforce consistent coding style and best practices.
- The TypeScript override ensures that TypeScript-specific rules are applied.
The configuration is well-structured and follows best practices.
|
Unfortunately, running |
|
I've relocated TypeScript to the root directory to better manage the version in monorepo. However, it seems tsdoc is referencing the wrong TypeScript version not the root, which is causing
|
What this PR does / why we need it?
This PR addresses the issue where linting was not being automatically executed before commits, despite
Huskybeing installed. The previous setup hadHuskyinstalled at v8, but properpre-commithooks were not in place to ensure that linting occurred consistently. SinceHuskyv5, the configuration and usage have changed. For more details, refer to the the documentation of Husky.Key changes include:
pre-commitconfiguration in the.huskyfolder, and ensureHuskyworks as expected to automate linting at thepre-commitstage.lint-stagedto only lint the files that arestagedfor commit, improving efficiency.With these updates, developers will no longer need to manually run lint checks, as the process will be automated and enforced during the commit phase.
Any background context you want to provide?
This PR brings several improvements to the linting setup, along with updates to the
Huskyandlint-stagedconfigurations.1. Relocate
lintCommand to RootAs part of the monorepo migration (issue #648), the
lintcommand was previously located in thesdkpackage, requiring developers to runpnpm sdk lint. However, this command is intended to lint the entire project, not just thesdk.To address this, I’ve moved the lint command to the root
package.json. Now, you can simply runpnpm lintto lint the whole project.2. Ignore files in
.eslintignoreinlint-stagedTo prevent unnecessary files from being linted, we use an
.eslintignorefile to specify which paths should be excluded. Sincelint-stageddoesn’t automatically respect.eslintignoreby default, I’ve added thelint-staged.config.jsfile to ensure that the ignore rules are applied. This allows us to avoid linting files specified in.eslintignoreduring thepre-commitstage.Alternatively, if we want to eliminate this step, we would need to upgrade
ESLintto version8.5.0or later (we are currently using8.19.0). For more information, refer to the lint-staged documentation.Additionally, I’ve moved the
.eslintignorefile from thesdkpackage to the root, so it now applies to the entire monorepo. If you need to exclude specific files in other sub-projects, you can add them to this file.3. Centralize ESLint Configuration in the Root
Since we are now working in a monorepo, it makes sense to centralize
ESLintrules in a single configuration file at the root. To achieve this, I’ve created a root.eslintrc.jsfile, and sub-projects likesdkcan extend from it. This ensures consistency across the entire codebase while allowing for project-specific overrides.Here’s an example of how the sdk package extends the root ESLint configuration:
What are the relevant tickets?
Fixes #888
Checklist
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
package.jsonfiles to reflect changes in dependencies and scripts.