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: cannot find tsconfig.json near... in sheriff.config.ts #161

Open
tomwhite007 opened this issue Oct 29, 2024 · 18 comments
Open
Assignees
Labels
bug Something isn't working

Comments

@tomwhite007
Copy link

I've created a demo repo to try out your new Barrel-less Module Boundaries in Nrwl Nx. But, Sheriff cannot find tsconfig.json because there is only tsconfig.base.json in the root of a Nrwl Nx repo.

This may relate to Issue 68

Error reported from inside sheriff.config.ts:

Dependency Rule (internal error): cannot find tsconfig.json near /Users/tom/Development/enterprise-angular-mono-repo-patterns-example-with-sheriff/sheriff.config.tseslint(@softarc/sheriff/dependency-rule)

Screenshot:

image

Example Repo:

https://github.com/tomwhite007/enterprise-angular-mono-repo-patterns-example-with-sheriff

@rainerhahnekamp
Copy link
Collaborator

rainerhahnekamp commented Oct 29, 2024

I've created also a PR (in your repo). The issue was that you cannot use TS aliases in your sheriff.config.ts.

And then run npx sheriff verify apps/booking/booking-mobile/src/main.ts

@tomwhite007
Copy link
Author

Thanks @rainerhahnekamp !

Your response was so quick I can't keep up. You've set me on the right path, and I'll report back here about the error shortly.

@rainerhahnekamp
Copy link
Collaborator

Hi @tomwhite007, no worries. Just let me know once you had the chance to take a look at it.

@tomwhite007
Copy link
Author

tomwhite007 commented Nov 4, 2024

Hi @rainerhahnekamp

Thank you for your help. Since then, I've added more modules and libs to my demo repo. The Sheriff lint rules are working perfectly - I'm impressed! I have noticed that when things are configured incorrectly (i.e. non-existent module paths in sheriff.config.ts), the lint errors that appear are incorrectly describing different errors; I can raise an issue on that if it would help? It would be helpful in the config file if incorrect module paths had a red underline - if that was possible.

I should also mention that the original issue described in this thread is still present - though I realise now that it is only cosmetic. The red lint underline appears on the @softarc/sheriff-core import line inside sheriff.config.ts

image

I presume this lint error is because some part of the code doesn't recognise tsconfig.base.json in an Nx repo replaces tsconfig.json?

@rainerhahnekamp
Copy link
Collaborator

Hi @tomwhite007, so you mean having eslint for the sheriff.config.ts itself? That could work, but we still have to find a way on how to communicate errors to users, when there is no sheriff.config.ts at all.

What kind of error messages do you get, where you say they are describing the behavior incorrectly? Please raise a separate issue for that.

We'll keep this issue open. In our tests, we've never encountered the issue with tsconfig.json. Could it be that you don't have an entryFile property?

@tomwhite007
Copy link
Author

tomwhite007 commented Nov 4, 2024

Hello @rainerhahnekamp

so you mean having eslint for the sheriff.config.ts itself?

Yes. It would improve the DX where modules need to be defined. Users that didn't create a sheriff.config.ts would be unaffected; their modules are inferred so the module path would always be a real path and not need to be reported.

What kind of error messages do you get... ? Please raise a separate issue for that.

Yes. Will raise shortly.

Could it be that you don't have an entryFile property?

No. I've now added an entryFile property to the config (which has little meaning in a monorepo, but I picked one app to test against). The import lint error is still there at the top of the sheriff.config.ts file. I've also tried and confirmed the error also appears in a new Nx Angular Workspace with a fresh Sheriff install and sheriff.config.ts added.

Hope that all helps.

@tomwhite007
Copy link
Author

Thanks for your help so far @rainerhahnekamp

My demo repo is working fine now.

I have created an issue to contain the incorrect configuration errors: #165

So now this thread can be only about the error at the top of sheriff.config.ts in Nx Workspaces:

Dependency Rule (internal error): cannot find tsconfig.json near ... (@softarc/sheriff/dependency-rule)

@rainerhahnekamp rainerhahnekamp self-assigned this Nov 8, 2024
@rainerhahnekamp rainerhahnekamp added the bug Something isn't working label Nov 8, 2024
@rainerhahnekamp
Copy link
Collaborator

@tomwhite007 I have a hard time to reproduce the error. I've setup https://github.com/rainerhahnekamp/sheriff-nx. It is a fresh Nx repo with sheriff installed. If I run npx sheriff verify or npx nx lint it works without any issues.

@tomwhite007
Copy link
Author

Thanks for trying @rainerhahnekamp . I'll dig into this and report back next week to give you all my findings at once.

@tomwhite007
Copy link
Author

tomwhite007 commented Nov 13, 2024

Hi @rainerhahnekamp

I've raised a PR against your test repo. I found that by adding the Sheriff plugin to eslint.config.js, I could then see the red underline lint error in sheriff.config.ts from my VS Code editor.

image

Can you see that now?

@rainerhahnekamp
Copy link
Collaborator

Thanks Tom, it didn't show up in my IDE but once I ran eslint sheriff.config.ts, it was there.

The problem is that eslint usually runs against files which are not located in the root of an nx workspace. For example, when you run it against any main.ts or whatever, there is always a parent folder which has the tsconfig.json.

So I see the problem, it shouldn't impact the lining of your projects, but we should fix it somehow. Do you have ideas?

@tomwhite007
Copy link
Author

I'd need to spin your code up, and I'm not sure how to do that yet. But I see that the init function in packages/core/src/lib/main/init.ts does this:

fs.findNearestParentFile(entryFile, 'tsconfig.json'),

So, I suspect that when the process runs in the context of an Nx repo, it needs to do this:

fs.findNearestParentFile(entryFile, 'tsconfig.base.json'),

(assuming that doesn't impact how the process behaves in an Nx Workspace currently)

If you have an opinion on how to check for the Nx context, say, looking for the presence of nx.json in the root, I'd like to hear it. But I'd enjoy submitting a first draft PR for discussion if you're interested and you feel patient enough for me to do it over the coming weeks.

PS. I'm aware of how elegant your code is in this project. I'd do my best to match that.

@rainerhahnekamp
Copy link
Collaborator

I'm thinking in a slightly different direction. I don't want that Sheriff needs to know that it is running on nx and that nx is using tsconfig.base.json. If possible, it should just work.

There are two things. First, strictly speaking, sheriff.config.ts is not part of any application/lib, and therefore, Sheriff should not deal with it. Maybe we need to rethink the recommended setting in eslint.config.js for Nx projects.

I am all in for showing error messages in sheriff.config.ts, but this is a completely different process and therefore might not have the same initialization process. We might even say, the check of the configuration file is an own ESLint rule. See #165

@tomwhite007
Copy link
Author

Ah, I get what you mean.

How about this approach? rainerhahnekamp/sheriff-nx#2

@tomwhite007
Copy link
Author

Thinking about it, I think the eslint plugin syntax should allow the ignores behind-the-scenes within the plugin itself. You might be thinking that way already. I'm pretty sure I can work that out. I'll have a read-up on it.

@rainerhahnekamp
Copy link
Collaborator

@tomwhite007, we can do the quick fix or provide already the initial setup for a special configuration rule.

  • The quick fix would be to add the ignores property to the docs.
  • Another option would be to implement a separate initialization process that activates when it is applied to sheriff.config.ts. This process would return a different type, ensuring that linting rules do not trigger any error messages.

I am open to both. Do you want to contribute a PR?

@tomwhite007
Copy link
Author

Yes. Can do. Thank you.

@tomwhite007
Copy link
Author

Hi @rainerhahnekamp

It's been a while, but I've been starting a new contract. Hope this is worth the wait.

I'm dropping 2 PRs that give the same result so you can choose. I'll comment on both about what I've done. I've basically tried both of your bullet options above while learning how your code and tests work.

If you prefer an alternative to them, I could look at inserting a check into Core (packages/core/src/lib/main/init.ts) instead. But that just feels more heavy-handed to me at the minute.

Observation: the repo's own eslint rules seem to need a bit of housekeeping; html and json files are checked with TS file rules at the moment (which leads to a lot of red underlines in package.json). Would you like me to address in another PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants