Skip to content
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

feat: add eslint rule for enforcing WorkspaceService naming convention #6308

Merged
merged 2 commits into from
Jul 19, 2024

Conversation

magrinj
Copy link
Member

@magrinj magrinj commented Jul 17, 2024

Description

This PR introduces a custom ESLint rule named inject-workspace-repository. The purpose of this rule is to enforce naming conventions for files and classes that use the @InjectWorkspaceRepository decorator or include services ending with WorkspaceService in their constructors.

Rule Overview

The new ESLint rule checks for the following conditions:

  1. File Naming:

    • Only file ending with .service.ts or .workspace-service.ts are checked.
    • If a file contains a class using the @InjectWorkspaceRepository decorator or a service ending with WorkspaceService in the constructor, the file name must end with .workspace-service.ts.
  2. Class Naming:

    • Classes that use the @InjectWorkspaceRepository decorator or include services ending with WorkspaceService in their constructors must have names that end with WorkspaceService.

How It Works

The rule inspects each TypeScript file to ensure that the naming conventions are adhered to. It specifically looks for:

  • Constructor parameters with the @InjectWorkspaceRepository decorator.
  • Constructor parameters with a type annotation ending with WorkspaceService.

When such parameters are found, it checks the class name and the file name to ensure they conform to the expected patterns.

Example Code

Valid Cases

  1. Correct File and Class Name with Decorator:

    // Filename: my.workspace-service.ts
    class MyWorkspaceService {
      constructor(@InjectWorkspaceRepository() private repository) {}
    }
  2. Service Dependency:

    // Filename: another.workspace-service.ts
    class AnotherWorkspaceService {
      constructor(private myWorkspaceService: MyWorkspaceService) {}
    }

Invalid Cases

  1. Incorrect Class Name:

    // Filename: my.workspace-service.ts
    class MyService {
      constructor(@InjectWorkspaceRepository() private repository) {}
    }
    // Error: Class name should end with 'WorkspaceService'.
  2. Incorrect File Name:

    // Filename: my.service.ts
    class MyWorkspaceService {
      constructor(@InjectWorkspaceRepository() private repository) {}
    }
    // Error: File name should end with '.workspace-service.ts'.
  3. Incorrect File and Class Name:

    // Filename: my.service.ts
    class MyService {
      constructor(@InjectWorkspaceRepository() private repository) {}
    }
    // Error: Class name should end with 'WorkspaceService'.
    // Error: File name should end with '.workspace-service.ts'.
  4. Incorrect File Type:

    // Filename: another.service.ts
    class AnotherService {
      constructor(private myWorkspaceService: MyWorkspaceService) {}
    }
    // Error: Class name should end with 'WorkspaceService'.
    // Error: File name should end with '.workspace-service.ts'.
  5. Incorrect Class Name with Dependency:

    // Filename: another.workspace-service.ts
    class AnotherService {
      constructor(private myWorkspaceService: MyWorkspaceService) {}
    }
    // Error: Class name should end with 'WorkspaceService'.

First step

This rule is only a warning for now, and then we'll migrate all the code that need to be migrated and move from warn to error.

Fix #6309

@magrinj magrinj marked this pull request as ready for review July 17, 2024 15:22
@magrinj magrinj requested review from Weiko and charlesBochet July 17, 2024 15:22
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

  • Added new ESLint rule @nx/workspace-inject-workspace-repository in /packages/twenty-server/.eslintrc.cjs to enforce naming conventions.
  • Introduced inject-workspace-repository rule in /tools/eslint-rules/index.ts.
  • Added /tools/eslint-rules/rules/inject-workspace-repository.ts to define the new rule.
  • Created /tools/eslint-rules/rules/inject-workspace-repository.spec.ts to test the new rule.
  • Added TODO comment in /packages/twenty-server/src/engine/twenty-orm/twenty-orm.providers.ts for future error handling improvements.

5 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link
Member

@charlesBochet charlesBochet left a comment

Choose a reason for hiding this comment

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

Tested, LGTM!

@charlesBochet charlesBochet merged commit de20c56 into main Jul 19, 2024
6 checks passed
@charlesBochet charlesBochet deleted the feat/eslint-rule-workspace-service branch July 19, 2024 17:17
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.

feat: add eslint rule for enforcing WorkspaceService naming convention
2 participants