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

POC: chore: use Nx workspace lint rules #3163

Merged
merged 5 commits into from
Jan 3, 2024

Conversation

thaisguigon
Copy link
Contributor

@thaisguigon thaisguigon commented Dec 27, 2023

Closes #3162

Context

This PR leverages Nx's workspace lint rules feature (see https://nx.dev/nx-api/eslint/generators/workspace-rule) to avoid having to build eslint rules written with Typescript before they can be used by Eslint and VSCode.

How it works

Nx loads dynamically TS eslint rules from the tools/eslint-rules package, which then become available under the @nx eslint plugin in the eslint config.

For instance, a rule named my-custom-rule becomes available in the eslint config as @nx/workspace-my-custom-rule.

Use Nx's generator to easily generate a basic rule file: nx g @nx/eslint:workspace-rule my-custom-rule

Included in this PR

  • Using Nx's workspace lint rule generator, those changes were automatically applied by Nx:
    • Generated tools/eslint-rules package with Typescript and Jest configuration
    • Added firsttris.vscode-jest-runner to recommended VSCode extensions
    • Added root extendable eslint and TS configs
  • Moved eslint rules from packages/eslint-plugin-twenty to tools/eslint-rules.
  • Reworked some of the custom eslint rules to simplify the code/fix TS errors.
  • Moved:
    • common eslint config to the root eslint config
    • some devDependencies to the root package.json

Note that Nx suggests moving all dependencies and devDependencies to the root package.json: https://nx.dev/concepts/more-concepts/dependency-management

  • Reworked packages/twenty-front eslint config to use Nx's eslint plugin configs. This adds some useful default eslint rules.
  • Added nrwl.angular-console (Nx's official VSCode extension) to recommended VSCode extensions.
  • Other changes in TS and Eslint configs were inspired by https://github.com/nrwl/nx-examples/tree/master.

TODO (not included in this PR)

  • Rework other packages configs to extend the root TS and Eslint configs.

Caveats

  • Nx does not allow to change the rules package's name: they must be located in tools/eslint-rules. A sub-directory can eventually be used but that's it.

Note that Nx monorepo packages are usually organized by purpose under three folders: apps, libs and tools.

  • Eslint rules that were previously available as twenty/... are now renamed to @nx/workspace-.... If we want in the future to publish an eslint-plugin-twenty package to NPM, we can still build the TS eslint rules to a dist directory and publish it under this name. Nx provides some tools to help with creating publishable libs (see https://nx.dev/concepts/more-concepts/buildable-and-publishable-libraries). However, this might be YAGNI for now.
  • I managed to rename the rules to use the twenty/... prefix in the workspace by re-creating the eslint-plugin-twenty package and re-exporting the rules from require('@nx/eslint-plugin/src/resolve-workspace-rules').workspaceRules, but in the end this might be YAGNI so I didn't include it in this PR.

@thaisguigon thaisguigon force-pushed the chore/resolve-workspace-eslint-rules branch from 375fb77 to bdbd2d2 Compare December 27, 2023 11:12
@thaisguigon thaisguigon marked this pull request as ready for review December 27, 2023 11:13
@charlesBochet charlesBochet self-assigned this Jan 2, 2024
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.

Wow! Love it!
I have tested it locally. I've merged main and fixed a few conflicts.

We should double check how it runs on the CI and if docker install is not broken

@charlesBochet charlesBochet merged commit 8483cf0 into main Jan 3, 2024
13 checks passed
@charlesBochet charlesBochet deleted the chore/resolve-workspace-eslint-rules branch January 3, 2024 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Timebox] POC: use Nx Workspace Lint rules feature
2 participants