feat: Directive to Skip File Parsing

[jeengbe]

Summary

A new /* eslint-ignore-file */ directive that gives files the ability to ignore themselves.

Related Issues

related GitHub Issue

[linux-foundation-easycla]

• ✅login: jeengbe / (2a34dff)
• ✅login: jeengbe / (2a34dff, 765bbf1)

The committers listed above are authorized under a signed CLA.

[ljharb]

this seems like a security attack vector. why would i want to permit someone to add a file to my codebase that, unbeknownst to me, isn't verified by eslint?

[jeengbe]

why would i want to permit someone to add a file to my codebase

The file would be generated code, such as an API client generated by typescript-swagger-api, a routes file generated by tsoa or type definitions generated by graphql-codegen.

unbeknownst to me, isn't verified by eslint?

How is this different from files self-ignoring with /* eslint-disable */, are there certain security-focused rules that override eslint-disable?

[ljharb]

Fair question, and there's https://www.npmjs.com/package/eslint-plugin-eslint-comments for that, but eslint-disable still allows them to be parsed, which i think is important.

I'd expect generated code to output into a single folder, which could easily be ignored with ignorePatterns.

[jeengbe]

I'd expect generated code to output into a single folder

I must respectfully disagree with you on that point, and evidently many others. In most codebases I've seen so far, generated code is not hosted in a single directory.

Instead, consider the following GraphQL/Tsoa backend:

.
└── src/
    ├── lib/
    │   ├── svc-a/
    │   │   ├── generated/
    │   │   │   └── Api.ts <- typescript-swagger-api
    │   │   └── index.ts
    │   └── svc-b/
    │       ├── generated/
    │       │   └── proto <- gRPC client
    │       └── index.ts
    ├── resolvers/
    │   ├── a/
    │   │   └── resolve.ts
    │   ├── b/
    │   │   └── resolve.ts
    │   └── types.ts <- graphql-codegen
    └── build/
        └── tsoa <- tsoa
            └── routes.ts

A collective generated/ folder is unfeasible and doesn't scale well.

[ljharb]

you can ignore **/generated/**, though.

[jeengbe]

You're right, I missed that.

Regardless, this propsal is not about the functional ability to skip certain files from being parsed. Instead, it's about the file skipping itself fully. Files with /* eslint-disable */ at the top are a waste of resources, by how I understand that directive to work.

[ljharb]

I agree with your assessment - however, my experience managing linter settings in large codebases tells me that files overriding eslint themselves is an antipattern (albeit sometimes necessary for some kinds of overrides) and that eslint config is best managed centrally in a root config file. Adding this feature would thus require me to waste further resources banning its use in every project I manage.

[jeengbe]

Could you please elaborate on the difference between /* eslint-disable */ and the proposed directive? Functionally, they are the same, and with this, otherwise already ignored files would no longer slow down the linting process.

I don't have the experience of working in large codebases you do, and in no scenario I can think of, would disable work, but skip-file not.

[ljharb]

The difference is that with this new proposal, parse errors can go undetected.

[jeengbe]

I don't see how that is a bad thing. It's not in the interest of the application developer to experience ESLint crashes over files that are ignored anyway. Other than for reporting issues to the library developer, parsing eslint-disabled files has no value to the application developer. If your experience tells otherwise, that would resolve this discussion, I myself don't see when this would not be the case.

[ljharb]

A syntax error in production that's not caught in CI can break your entire application; just because you don't want eslint rules on a file doesn't mean you want syntax errors in it.

Can you help me understand a use case where you do NOT care that there's syntax errors in a lintable file?

[jeengbe]

Oh, we might be talking about slightly different things. 😅 With "ESLint crashes", I mean that as a consequence a bug in an ESLint rule, such as here. This caused ESLint to crash on files automatically generated by Tsoa, which the application developer practically has no control over.

When you talk about syntax errors, do you regard ESLint as a syntax checker? I did not at all consider that. To me, ESLint is purely about code style, and syntactic checks are done by the TypeScript compiler.

Or concretely, your concern is that ESLint would no longer catch syntax errors in a self-ignoring file, which would cause that file to go to production otherwise unverified?

[ljharb]

Yes, eslint even with no rules enabled is a syntax checker, and this is a very important function it serves. Not everyone uses TypeScript.

Yes, that is my concern.

[nzakas]

The .eslintignore file is deprecated. All ignores are now in the eslint.config.js file.

[nzakas]

I'm curious if you have any data to back up this assertion?

In my experience, I don't believe this is true. ESLint will still validate the syntax in this case.

[nzakas]

Please look at the actual source code to determine where changes should be made. This is part of the RFC process.

[nzakas]

Yes it is. We need a complete understanding of how such a change would function inside of ESLint.

[nzakas]

I thought the point was that we wouldn't parse the file? But here you're saying we will in order to find the comment? Can you clarify?

And what does it mean for a non-fatal error to be raised? Do you mean it would be look like a lint violation? Would the user get any output?

[nzakas]

Please look through our docs and find what would need to be updated.

[nzakas]

Why would we parse at all? Why not just use a regular expression to see if the text exists in the source?

[nzakas]

Per your motivation, shouldn't the directive always be first in a generated file?

[nzakas]

If this is already possible using an ignore file, then why would we add another way to do the same thing?

[nzakas]

@jeengbe are you still working on this? There is feedback to address.

[jeengbe]

@jeengbe are you still working on this? There is feedback to address.

Yes, apologies, no, I completely forgot about this issue.

The comments are valid points and I don't have a strong enough commitment to this proposal to advocate for it further. I would also need some help with the internal effect of this change since I do not know how ESLint functions internally.

I propose we close this PR unless you think otherwise.

[nzakas]

That sounds good. I think using the current approach of ignoring files is probably the best way to go to address this use case. Thanks for your time and thoughtfulness.