Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
chore: switch to ESLint flat config#6836
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Thanks for the PR,@nzakas! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently onhttps://opencollective.com/typescript-eslint. |
netlifybot commentedApr 3, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
✅ Deploy Preview fortypescript-eslint ready!
To edit notification comments on pull requests, go to yourNetlify site settings. |
thanks heaps for doing this! Even went and fixed up the nx config for us! quick question from a glance: |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Yes. So basically my thinking was first, get the repo's config files swapped over to |
nx-cloudbot commentedApr 5, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
So this is where I need help. I just don't know enough about Nx to understand what the problem is with the lint task. |
JoshuaKGoldberg commentedApr 5, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I'll defer to@JamesHenry as the Nx expert in the org, but Isuspect that Nx doesn't yet recognize
|
Hi@nzakas, thanks again for this! @JoshuaKGoldberg is exactly right, for now in all project.json files with a "lint" target you will need to add the Now that you have confirmed that flat config is feature complete we will add automatic detection for this in an upcoming Nx version but for now you'll need to configure it, e.g for the website project: There seem to be some remaining issues, however, and I wanted to hand it back to you before I spend too long spinning my wheels on stuff I'm not familiar with. 2 issues that seem like they have easy solutions:
At least one remaining issue that I can't easily understand from the schema validation error message: I wonder if anything can be done to build on top of ajv and improve that error message? |
@JoshuaKGoldberg@JamesHenry great, I'll make those changes and see how far I can get. |
Okay, so the root level problem I'm dealing with right now is that because Nx uses |
@nzakas Nx doesn't use the CLI |
The exact class usage is here: |
@JamesHenry ah! Yes, that makes a big difference. It's the CLI that recognizes the environment variable and then switches between |
JamesHenry commentedApr 6, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@nzakas cool, I'll submit a PR to Nx next week when I'm back from vacation to allow it to: A) automatically detect B) Conditionally change the implementation class depending on the config used/the presence of the environment variable Until then I've mark this one as blocked, I'll circle back as soon as there is an Nx version for us to apply here |
Awesome, thanks so much. Enjoy your vacation! |
Just a follow up on this, now that we (Nx) are a larger company with more resources to work on product, we have a more robust planning cycle. I have added supporting flat config as an item for our next iteration which starts in a couple of weeks. I should hopefully be working on it personally. I will update this PR once there is a version to try it out with. Thanks again@nzakas! |
Sure thing. I'll stay tuned in to this PR. |
Hi@nzakas I'm sorry it took me a while to loop back to this. I added the support for Flat Config to Nx in the last cycle and have updated your branch here to contain those changes. However, when I run the linting for a particular project, it seems that the root config is currently invalid based on the new structure? |
Sorry about that. I think the config file was so long that I just missed that section. Indeed, there are no overrides in flat config, so I just flattened that section out and pushed the new config file. (I'm on my way out so didn't have time to verify it 100% works but at least it's the correct format.) |
@nzakas it did fail to validate again just FYI |
This means more than one config is specifying For now I've just commented out the offending lines in the configs. For some reason, I can't get the lint in the website package to run correctly locally, so relying on CI to help figure that part out. |
GitMurf commentedJun 15, 2023
Hi team! Nice work so far. I have a couple projects coming up that I am proposing and they are requiring some clarity on the transition to the new flat configs. Is this the PR to follow for the transition of typescript-eslint to using the new flat config file format |
Just a heads up, we've also got#7273 in-progress. Might be relevant here. |
@nzakas now thateslint/eslint#17640 is merged, are there any changes you want to make here? |
That PR doesn't really apply here. This PR's purpose was to switch the repo to use flat config. Updating the plugin for flat config would be separate. |
Got it - what I really mean is, is there anything on your end you plan on doing to this PR? Or is it ready for us to take over? |
JamesHenry commentedOct 18, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I think a win for everybody here would be if we abandon this PR and instead run the Nx automated migration for migrating from .eslintrc to flat config:
WDYT? |
Automation FTW! My feelings are not hurt if a robot can do this better than me. 😄 |
karlhorky commentedOct 20, 2023
@JamesHenry I guess you mean this Nx automated migration to flat config? Looks pretty cool 👀 didn't know there was a tool to automate this! I guess it could work for other configs elsewhere too... likeour |
JamesHenry commentedOct 20, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@karlhorky to be clear the automation is not for plugin authors to convert their plugin implementations to flat config, it is for users to convert their setups. This PR relates to typescript-eslint’s own usage of eslint |
karlhorky commentedOct 20, 2023
Yeah, maybe it still applies to shared configs like the one I shared above? (it's not a plugin, although we also have one of those) |
PR Checklist
:)
Overview
In testing the new flat config, I thought a good way to test typescript-eslint would be to convert the project itself to use flat config. I've got most of the way there, but I'm afraid my knowledge of how to configure Nx is pretty basic and so I can't quite get the linting to run. Happy to continue iterating on this if someone can point me in the right direction.