-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
fix: search for tsconfig relative to eslintrc #1263
Conversation
🦋 Changeset detectedLatest commit: 875ae63 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Running Lighthouse audit... |
i was just browsing through the recent PRs, and I'll chime in to confirm that i do this pattern in my monorepos as well. |
don't y'all put this stuff in the root config? i usually do this: https://github.com/t3-oss/create-t3-turbo/blob/bb46ce25a43fb6379f69988ea1f7e14f6e032ebd/.eslintrc.js#L8-L13 |
I don't think using the Furthermore, I don't quite understand your config. Why is the root ESLint extending from each project? That seems backwards. The child ESLint configs should be inheriting from the parent, like how normal TypeScript code would work. |
Also, as a side note, the file should have a cjs extension, not a js extension. |
I agree with @Zamiell @juliusmarminge Using a single eslint config that spans your whole monorepo is a bad practice, especially if it grows in complexity. If you only have two similar projects in your repo (e.g. all of them are using React) it may be fine. For monorepos that contain different projects in them (some React, Svelte and NodeJS) where every project needs a dedicated eslint config, your approach is not viable. In addition, it also defeats the whole project structure if configuration files are outside the project scope in which they are used. |
This should not be a problem as long as |
Or just consider the common use-case of a monorepo with a back-end and a front-end, both written in TypeScript. The front-end needs to have a tsconfig that specifies browser-related environment (e.g. DOM types), and the back-end needs to have a tsconfig that specifies node-related environment (e.g. Node types). You need to lint each project separately, so it does not make sense to combine both browser+node node together in the monorepo ESLint config. |
It's not a problem insofar that it causes any lint errors, but I think it is best practice in 2023 to name your ESLint config file |
The typescript parser options doesnt need to be separate, so I like just specifying the tsconfig paths in root. As for the rules etc those can extend some common config package and also add new rules on their own (I think I left this as a todo for our monorepo). Not quite sure what the benefits are of duplicating the tsconfig path in each config? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyway, this way it atleast works in monorepos so that's a big win - even though I wouldn't do this myself, no need to provide monorepo config opinions here 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyway, this way it atleast works in monorepos so that's a big win - even though I wouldn't do this myself, no need to provide monorepo config opinions here 😅
They need to be separate, because each package has a separate tsconfig. You wouldn't want to be linting |
If that's the case then you must have misconfigured your tsconfig.include fields. Let's say I have:
Then the packageB files will be parsed by the packageB/tsconfig settings and the same for A. This doesn't change if you specify project as an array in the root .eslintrc. If you get ts settings from B/tsconfig leaking into pacjageA/src then that's not a caused by a bad lint config - but a bad tsconfig setup |
It isn´t about tsconfig´s here, it´s about eslintrc´s. Your root |
Thanks for merging. 🚀 |
* fix: search for tsconfig relative to eslintrc * chore: add changeset
✅ Checklist
Changelog
I created a t3-app in an existing monorepo and I expect eslint to use the config file that is in the project and not in the turborepo root. The current
.eslintrc.cjs
always searches for thetsconfig.json
in the root of the project. I resolved the path to do a relative seach, which is what most people expect if there lies atsconfig.json
next to theeslintrc.cjs
.💯