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

Nrwl Nx project: Lint errors for 'root' module are difficult to infer meaning - suggest linting sheriff.config.ts #165

Open
tomwhite007 opened this issue Nov 6, 2024 · 3 comments

Comments

@tomwhite007
Copy link

I was going to entitle this issue 'Unexpected lint errors when sheriff.config.ts is incorrect', but having retested, I can see these errors are intended but they are difficult for a developer to intuit when the problem causing them is configuration errors.

The problem:

When a new app or lib is created in Nx, Sheriff deems its module to be root...

1: If I have no root rule in sheriff.config, I get errors on standard npm imports, e.g.

image

I realise now that the docs say the root rule is required. I had tried to remove the root rule - to enforce strict import rules with no fall-through like @nx/enforce-module-boundaries in this old example here.

2: Assuming I have added a root: 'noTag' rule but then fail to create a module path correctly for my new app or lib, I get a root error as follows

image

Suggested solution

Both points 1 and 2 above are difficult to infer that the problem is created by the developer in the sheriff.config.ts file.

Following a conversation with @rainerhahnekamp in this thread: #161 (comment) , I believe both issues could be solved by adding lint rules to the sheriff.config.ts.

Note:

I've created this issue to separate it from #161 for your consideration so you can choose to close each separately.

@rainerhahnekamp
Copy link
Collaborator

@tomwhite007 we need to think a little bit about the design.

  • The problem is that Sheriff doesn't work when the config file is invalid. So people look at a file, see no error, and think everything is alright.
  • I am not sure, if ng lint also covers sheriff.config.ts. If not, then no error is shown at all.

How do you see it?

@tomwhite007
Copy link
Author

Hi @rainerhahnekamp

  • I'm proposing one or two extra eslint rules that would be included in extends: [sheriff.configs.all],.
  • I believe ng lint would cover it, but I'll test to make sure. And I'll test nx run-many -t lint.
  • Sounds to me like a spike PR might prove the point. Would you mind leaving this thread open so I can find some time to demonstrate?

@rainerhahnekamp
Copy link
Collaborator

Hey @tomwhite007, sure I leave it open and once you find some time you come back to me.

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

No branches or pull requests

2 participants