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

Added support for ESLint flat config files (eslint.config.js) #338

Closed
wants to merge 1 commit into from

Conversation

Prinzhorn
Copy link
Contributor

@Prinzhorn Prinzhorn commented Nov 28, 2024

Refs #335

@silverwind could you update your minimal test to verify this works for you as well?

I've inherited a project that uses nx monorepos and just migrated to the new flat file format (with multiple eslint.config.js that extend each other). Apparently the resolution of configurations works different now. For some reason nx lint successfully linted but SublimeLinter couldn't parse the files. Running eslint directly worked only when pointing --config at the closest config.

This code was mostly written by Claude inspired by the code that nx uses:

https://github.com/nrwl/nx/blob/8eb615969633ca4d2e5147828d725d7df5bbca2a/packages/eslint/src/executors/lint/lint.impl.ts#L40-L49

https://github.com/nrwl/nx/blob/8eb615969633ca4d2e5147828d725d7df5bbca2a/packages/eslint/src/utils/config-file.ts#L31-L52

If I edit linter.py locally this PR fixes the problem for me. I honestly don't fully understand the entire eslint setup and what exactly broke.

@Prinzhorn
Copy link
Contributor Author

Prinzhorn commented Nov 28, 2024

Turns out there's also an experimental unstable_config_lookup_from_file flag, so this will be coming to eslint eventually I guess

https://eslint.org/docs/latest/use/configure/configuration-files#configuration-file-resolution

I wonder if my PR would break any use-cases? It's definitely missing mjs/cjs extensions.

@silverwind
Copy link

silverwind commented Nov 28, 2024

I think the eslint cli should be fully able to find its config files on its own so I would say it's a very bad idea to pass the --config option.

@Prinzhorn
Copy link
Contributor Author

Prinzhorn commented Nov 28, 2024

You are absolutely right and this should be the job of eslint. I'm using:

"linters": {
      "eslint": {
         "args": ["--flag", "unstable_config_lookup_from_file"]
      }
   }

for now. Otherwise the integrated eslint algorithm is just backwards to what monorepos require and how the algorithm works for legacy configs.

@Prinzhorn Prinzhorn closed this Nov 28, 2024
@kaste
Copy link
Contributor

kaste commented Nov 28, 2024

Maybe you can take a look at SublimeLinter's debug output. Isn't the most important part here which cwd SublimeLinter uses here because eslint will traverse from there upwards. That being said, manually setting working_dir: "$file_path" in the settings then should do the trick as well. (Usually the problem then would be: how do we find and choose a sane starting path for unsaved/unnamed buffers.)

@Prinzhorn
Copy link
Contributor Author

Maybe you can take a look at SublimeLinter's debug output.

Yes, that's how I was able to debug this and ended up here. I had to figure out why the command that SublimeLinter runs did not work.

So nx can successfully lint. That means the project and dependencies seem to be set up correctly. SublimeLinter runs this for one of the components in the project:

cat accounting-offer.component.ts | /src/Client/node_modules/.bin/eslint \
    --format=json \
    --stdin \
    --stdin-filename \
    /src/Client/libs/customer/accounting-onboarding/src/lib/accounting-offer/accounting-offer.component.ts

which will result in

Parsing error: Unexpected token enum

Because it does not pick up all the configs correctly to support TypeScript.

If I set "working_dir": "$file_path" I get:

error  Parsing error: /src/Client/libs/customer/accounting-onboarding/src/lib/accounting-offer/accounting-offer.component.ts was not found by the project service. Consider either including it in the tsconfig.json or including it in allowDefaultProject

which is also exactly what I get when I manually run eslint in the /accounting-offer directory. Which makes sense, because there is no eslint.config.js in that directory and eslint will not walk up the tree. It won't walk further than cwd it seems. That's why unstable_config_lookup_from_file exists to restore the old behavior of the legacy configs.

If I enable "args": ["--flag", "unstable_config_lookup_from_file"] (or apply this PR) it correctly lints, but only when I'm in /src/Client/libs/customer/accounting-onboarding or higher up because that's where the shared config ist. Otherwise it won't find it on its way from accounting-offer.component.ts to the cwd.


I re-read everything again.

Isn't the most important part here which cwd SublimeLinter uses here because eslint will traverse from there upwards

From what I've described above it appears eslint won't walk further up than the cwd? Otherwise why would it only work when I'm in /src/Client/libs/customer/accounting-onboarding but not in /src/Client/libs/customer/accounting-onboarding/src/lib/accounting-offer/

@kaste
Copy link
Contributor

kaste commented Nov 29, 2024

From what I've described above it appears eslint won't walk further up than the cwd?

That's not what I expect when I read their docs (https://eslint.org/docs/latest/use/configure/configuration-files#configuration-file-resolution):

When ESLint is run on the command line, it first checks the current working directory for eslint.config.js. 
If that file is found, then the search stops, otherwise it checks for eslint.config.mjs. If that file is found, then
the search stops, otherwise it checks for eslint.config.cjs. If none of the files are found, it checks the parent 
directory for each file. This search continues until either a config file is found or the root directory is reached.

The flag unstable_config_lookup_from_file should tell eslint where to start with its traversal. By default it starts with cwd, and with this flag it starts with file_path. To my understanding it comes down to how a nx project is layed out.

As it seems that nx lint does something different than running eslint raw and direct, I wonder why you don't execute nx lint within Sublime as well. (E.g. SublimeLinter uses yarn run if it finds a yarn package.)

Regardless of that, you did not say what SublimeLinter computes as the best default cwd. (It probably selects the nearest path with a package.json file going upwards.)

@Prinzhorn
Copy link
Contributor Author

Thanks a lot for the recommendations. It looks like something I did last week fixed the issue, I likely repaired the nx setup as I was upgrading dependencies.

I wonder why you don't execute nx lint within Sublime as well

Had no idea this was possible but also no idea how I'd do that. Looks like nx can't lint a single file and every nx command requires a project to be specified, which SublimeLinter has no understanding of.

For now I'd rather not touch anything and hope it keeps working.

@kaste
Copy link
Contributor

kaste commented Dec 2, 2024

In theory, you set executable: ["nx", "lint"] (or: nx exec etc) but you already point out that this wouldn't work as nx runs these "tasks" on a project basis. I thought that nx exec behaves more like e.g. yarn run, basically just passing the args through, just resolving the node/executable bits.

Better don't touch it for now. 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants