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

Bug: when using flat config, the ignored files are still parsed #17400

Closed
1 task
Comandeer opened this issue Jul 21, 2023 · 7 comments
Closed
1 task

Bug: when using flat config, the ignored files are still parsed #17400

Comandeer opened this issue Jul 21, 2023 · 7 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion works as intended The behavior described in this issue is working correctly

Comments

@Comandeer
Copy link

Environment

Node version: v20.5.0
npm version: v9.8.0
Local ESLint version: v8.45.0 (Currently used)
Global ESLint version: Not found
Operating System: darwin 22.5.0

What parser are you using?

Default (Espree)

What did you do?

Configuration
import js from '@eslint/js';

export default [
	{
		files: [
			'**/*.js'
		],

		ignores: [
			'ignored.js'
		],

		...js.configs.recommended
	}
];
// ignored.js
( function() {
	await 1;
}() );

Command for linting: eslint ..

What did you expect to happen?

As the file is listed in the ignores section, I would assume it is completely ignored, including parsing it.

What actually happened?

The file was parsed and the error about invalid JS syntax was raised

Link to Minimal Reproducible Example

https://github.com/Comandeer/bugs-reports/tree/eslint-parsing-ignored

Participation

  • I am willing to submit a pull request for this issue.

Additional comments

A quick debugging shows that the issue is connected with the

if (!config) {
return void 0;
}
fragment.

When running the eslint . command, the file is matched against several configs, including default ones. They do not contain any rules but contain default settings for the parser – and due to that the condition is almost always false. The config for the ignored file contains an empty rules property. That doesn't cause any issues for files containing valid JS syntax (as they are parsed and checked against zero rules) but fails for files with invalid JS syntax.

Probably the simplest solution would be to also check if the config.rules property has at least one key.

@Comandeer Comandeer added bug ESLint is working incorrectly repro:needed labels Jul 21, 2023
@mdjermanovic mdjermanovic added works as intended The behavior described in this issue is working correctly and removed bug ESLint is working incorrectly repro:needed labels Jul 22, 2023
@mdjermanovic
Copy link
Member

Hi @Comandeer, thanks for the issue!

This works as intended.

.js files are linted by default.

ignores in a configuration object that has other keys specifies files for which that configuration object doesn't apply:

https://eslint.org/docs/latest/use/configure/configuration-files-new#specifying-files-and-ignores

ignores in a configuration object that has no other keys specifies files that should be ignored:

https://eslint.org/docs/latest/use/configure/configuration-files-new#globally-ignoring-files-with-ignores

If you want to ignore ignored.js file, configuration should be:

import js from '@eslint/js';

export default [
    {
        files: [
            '**/*.js'
        ],
        ...js.configs.recommended
    },
    {
        ignores: [
            'ignored.js'
        ]
    }
];

@mdjermanovic mdjermanovic closed this as not planned Won't fix, can't repro, duplicate, stale Jul 22, 2023
@ivodolenc
Copy link

Hi @mdjermanovic I'm also bump into this.

So ignores in the same object have no effect if I understand correctly?

Doesn't work:

const jsConfig = {
  files: ['**/*.{js,mjs,cjs}'],

  // ignore.js is not ignored as expected
  ignores: ['ignore.js']
}

export default [jsConfig]

Works:

const jsConfig = {
  files: ['**/*.{js,mjs,cjs}']
}

export default [
  jsConfig,
  {
    // ignore.js is ignored as expected
    ignores: ['ignore.js']
  }
]

This is a bit confusing, so how can we use this with shared configurations without always explicitly setting ignores?

Let's say we have a shared configuration with ignores included. According to the above, ignores won't work if we set them explicitly, right? Do we even need ignores in shared configs?

import { jsConfig } from 'npm-package'

export default [
  jsConfig,
  {
    ignores: ['ignore.js']
  }
]

@mdjermanovic
Copy link
Member

@ivodolenc ignores in the same object with any other keys has the effect of not applying that configuration object to files matching those ignores: https://eslint.org/docs/latest/use/configure/configuration-files-new#excluding-files-with-ignores

ignores as the only key in a configuration object has the effect of ESLint completely ignoring files matching those ignores: https://eslint.org/docs/latest/use/configure/configuration-files-new#globally-ignoring-files-with-ignores

Shared configuration could export an array of configuration objects:

// npm-package

export const jsConfig = [
    {
        files: ['**/*.{js,mjs,cjs}'],
        rules: {
            // ...
        }
    },
    {
        ignores: ['ignore.js']
    }    
];
// eslint.config.js

import { jsConfig } from 'npm-package';

export default [
  ...jsConfig
];

@ivodolenc
Copy link

@mdjermanovic tnx for fast reply.

Hmm strange, because I have this config:

// eslint.config.js

import jsPlugin from '@eslint/js'

const ignores = ['**/examples/**/*']

const jsConfig = {
  files: ['**/*.{js,mjs,cjs}'],
  ignores: [...ignores, '**/*.{ts,mts,cts,tsx}'],
  rules: {
    ...jsPlugin.configs['recommended'].rules
  }
}

export default [jsConfig]

According to that configuration, I expect the examples directory to be ignored, but I still get notifications from eslint.

This config works fine:

// eslint.config.js

import jsPlugin from '@eslint/js'

const ignores = ['**/examples/**/*']

const jsConfig = {
  files: ['**/*.{js,mjs,cjs}'],
  rules: {
    ...jsPlugin.configs['recommended'].rules
  }
}

export default [
  jsConfig,
  {
    ignores: [...ignores, '**/*.{ts,mts,cts,tsx}']
  }
]

Shared configuration could export an array of configuration objects:

Yep, I figured that as soon as I posted it 😀

So configuration within shared packages should be rearranged to follow that form if we use ignores.

I haven't tried it yet, but I assume that when the configuration is imported from the package, the user can always overwrite the options later?

import { jsConfig } from 'npm-package'

export default [
  jsConfig,
  {
    // this will extend ignores from config above, right?
    ignores: ['!ignore.js', 'ignore-2.js', ...]
  }
]

@ivodolenc
Copy link

ivodolenc commented Jul 24, 2023

Just feedback from a user perspective because it's confusing, maybe because this is a new configuration system we have to get used to.

From the user's perspective, the ignore option means that ESLint will completely ignore all entries from the specified array and will not lint them.

My understanding to current eslint behaviour is that if the ignores option is specified within the same config object, then all specified options within config object will not be applied to all entries from the ignores array, but eslint will still lint those files/dirs from the ignore option? And if we want to completely ignore files/dirs, we need to specify a standalone object with an ignores option.

@passcod
Copy link

passcod commented Dec 13, 2023

This is the least intuitive thing you could possibly have done.

@grandsong
Copy link

I also encountered this issue.

I was following the readme of antfu/eslint-config.

// eslint.config.js
import antfu from '@antfu/eslint-config'

export default antfu({
  // Enable stylistic formatting rules
  // stylistic: true,

  // Or customize the stylistic rules
  stylistic: {
    indent: 2, // 4, or 'tab'
    quotes: 'single', // or 'double'
  },

  // TypeScript and Vue are auto-detected, you can also explicitly enable them:
  typescript: true,
  vue: true,

  // Disable jsonc and yaml support
  jsonc: false,
  yaml: false,

  // `.eslintignore` is no longer supported in Flat config, use `ignores` instead
  ignores: [
    './fixtures',
    // ...globs
  ]
})

Somehow the author believed ignores can be there, together with all other options in one big object. In my project, however, ignores never worked.

After reading this page, I finally made a workaround by moving ignores into a standalone object as an element of the array.

This is the insanely intuitive!

@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Jan 19, 2024
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Jan 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion works as intended The behavior described in this issue is working correctly
Projects
Archived in project
Development

No branches or pull requests

5 participants