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

new "flat" configs contain invalid parser key #2061

Closed
Techn1x opened this issue Jan 15, 2024 · 19 comments · Fixed by #2092
Closed

new "flat" configs contain invalid parser key #2061

Techn1x opened this issue Jan 15, 2024 · 19 comments · Fixed by #2092
Labels

Comments

@Techn1x
Copy link

Techn1x commented Jan 15, 2024

This makes them not usable for anyone trying to switch to flat eslint config

Each of the configs import the "base" config

module.exports = [...base, { rules: gtsRules }];

But that "base" config contains "parser" key, which is invalid in a flat config - needs to be inside languageOptions, and import the parser rather than providing a string value

parser: 'ember-eslint-parser',

@bmish
Copy link
Member

bmish commented Jan 15, 2024

@NullVoxPopuli @patricklx can you look into this? I've confirmed it only breaks when gjs/gts files are present.

This partially fixes it:

const emberEslintParser = require('ember-eslint-parser');
//...
{ 
  languageOptions: {
    parser: emberEslintParser,
  },
}

But then we get:

TypeError: Expected string in the form "pluginName/objectName" but found "ember/<noop>".

Possible because <noop> may not be considered a valid processor name? (should it be?)

https://github.com/eslint/eslint/blob/1f3744278433006042b8d5f4e9e1e488b2bbb011/lib/config/flat-config-schema.js#L217

@bmish bmish added the Bug label Jan 15, 2024
@Techn1x
Copy link
Author

Techn1x commented Jan 19, 2024

TypeError: Expected string in the form "pluginName/objectName" but found "ember/<noop>".

Possible because <noop> may not be considered a valid processor name? (should it be?)

Seems likely the reason.

If it's difficult to rename that processor for some reason, or we want to remove it as an issue entirely - under a flat config you can also import and use an object as the processor here, instead of the string name

https://eslint.org/blog/2022/08/new-config-system-part-2/#processors-works-in-a-similar-way-to-eslintrc

The one addition in flat config is that processor can now also be an object containing both a preprocess() and a postprocess() method.

@Techn1x
Copy link
Author

Techn1x commented Jan 19, 2024

Possible because may not be considered a valid processor name? (should it be?)

some background
eslint/eslint#14321 (comment)
eslint/eslint#14321 (comment)

@NullVoxPopuli
Copy link
Contributor

i wonder if both preprocessors should just be references, rather than strings

@bmish
Copy link
Member

bmish commented Jan 19, 2024

@patricklx can you comment about the <noop> parser name and intention with that? Seems like it shouldn't be too difficult to fix this for flat config.

@NullVoxPopuli
Copy link
Contributor

@Techn1x can you provide the config you're using?

@Techn1x
Copy link
Author

Techn1x commented Jan 30, 2024

Unfortunately I couldn't fully switch because typescript/eslint also do not have a flat config yet (and also this issue here in eslint-plugin-ember)

So I settled for just using overrides key in the meantime, in order to be easier to switch when the day comes. If it helps, here it is

// .eslintrc.js
const commonEmberRules = {
  'ember/route-path-style': 'error',
  // ember rules to work on
  'ember/no-get': 'off', // remaining usage is limited & mostly necessary. Too dangerous as warn due to autofixing.
  'ember/no-computed-properties-in-native-classes': 'warn',
  'ember/no-classic-components': 'warn',
  'ember/no-runloop': 'warn',
  'ember/no-array-prototype-extensions': 'warn',
  // off to avoid some lint noise for a while..
  'ember/require-tagless-components': 'off',
  'ember/no-classic-classes': 'off',
}

const commonTypescriptRules = {
  'no-unused-vars': 'off',
  '@typescript-eslint/no-unused-vars': [
    'error',
    {
      vars: 'all',
      args: 'after-used',
      argsIgnorePattern: '^_',
      destructuredArrayIgnorePattern: '^_',
      ignoreRestSiblings: true,
    },
  ],
  '@typescript-eslint/no-empty-function': 'off',
  '@typescript-eslint/consistent-type-imports': 'error',
  '@typescript-eslint/no-explicit-any': 'warn',
}

module.exports = {
  root: true,
  overrides: [
    // JS / TS
    {
      files: ['**/*.{js,ts}'],
      plugins: ['ember', '@typescript-eslint'],
      parser: '@typescript-eslint/parser',
      extends: [
        'eslint:recommended',
        'plugin:ember/recommended',
        'plugin:@typescript-eslint/recommended',
        // From eslint-config-prettier - this disables stylistic rules that prettier handles. should be last.
        'prettier',
      ],
      env: {
        browser: true,
      },
      rules: {
        ...commonTypescriptRules,
        ...commonEmberRules,
      },
    },
    // GTS
    {
      files: ['**/*.gts'],
      plugins: ['ember'],
      parser: 'ember-eslint-parser',
      extends: [
        'eslint:recommended',
        'plugin:ember/recommended',
        'plugin:ember/recommended-gts',
        // From eslint-config-prettier - this disables stylistic rules that prettier handles. should be last.
        'prettier',
      ],
      env: {
        browser: true,
      },
      rules: {
        ...commonTypescriptRules,
        ...commonEmberRules,
      },
      globals: {
        // give glint/gts files access to global types
        ModelFor: 'readonly',
        Nullable: 'readonly',
        StudentEventsService: 'readonly',
      },
    },
    // GJS
    {
      files: ['**/*.gjs'],
      plugins: ['ember'],
      parser: 'ember-eslint-parser',
      extends: [
        'eslint:recommended',
        'plugin:ember/recommended',
        'plugin:ember/recommended-gjs',
        // From eslint-config-prettier - this disables stylistic rules that prettier handles. should be last.
        'prettier',
      ],
      env: {
        browser: true,
      },
      rules: {
        ...commonEmberRules,
      },
    },
    // co-located tests
    {
      files: ['app/**/*-test.{js,ts,gjs,gts}'],
      rules: {
        // This rule does not support co-located tests
        'ember/no-test-support-import': 'off',
      },
    },
    // mirage uses some functions that look like array prototype extensions
    {
      files: ['mirage/**/*.{js,ts}'],
      rules: {
        'ember/no-array-prototype-extensions': 'off',
      },
    },
    // node files
    {
      files: [
        './.eslintrc.js',
        './.prettierrc.js',
        './.template-lintrc.js',
        './ember-cli-build.js',
        './testem.js',
        './blueprints/*/index.js',
        './config/**/*.js',
        './lib/*/index.js',
        './server/**/*.js',
        'tailwind.config.js',
        './contentful/**/*.js',
      ],
      extends: ['eslint:recommended', 'plugin:n/recommended'],
      parserOptions: {
        sourceType: 'script',
      },
      env: {
        browser: false,
        node: true,
      },
      rules: {
        'n/no-unpublished-require': 'off',
        '@typescript-eslint/no-var-requires': 'off',
      },
    },
  ],
}

@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Jan 30, 2024

I got this far with trying to create a test project:

But ran in to:

And downgrading the parser didn't seem to fix the problem, which is a bit disappointing. :(

@Techn1x
Copy link
Author

Techn1x commented Jan 30, 2024

And downgrading the parser didn't seem to fix the problem, which is a bit disappointing. :(

Sad. They did merge a fix though, hopefully they cut the next patch release shortly

@NullVoxPopuli
Copy link
Contributor

Here is an example flat config: https://github.com/NullVoxPopuli/ember-eslint-parser/blob/main/test-projects/configs/flat-js/eslint.config.mjs

Currently, it only does GJS (I'll PR one that works on GTS later)

@bmish
Copy link
Member

bmish commented Feb 13, 2024

@patricklx can you comment about the <noop> parser name and intention with that? Seems like it shouldn't be too difficult to fix this for flat config.

@patricklx ping on this? Shall we just rename it to fix this issue?

@patricklx
Copy link
Contributor

Hi, sure can be renamed to anything, just need to make sure to keep update the config, legacy as well

@bmish
Copy link
Member

bmish commented Feb 13, 2024

What do you think it should be renamed to? This processor name <noop> is part of the public API but since it's not a valid name, we need to change it.

@NullVoxPopuli
Copy link
Contributor

what's the issue? (am I confused? lol)

This is what a flat config looks like: https://github.com/NullVoxPopuli/ember-eslint-parser/blob/main/test-projects/configs/flat-js/eslint.config.mjs

@patricklx
Copy link
Contributor

patricklx commented Feb 13, 2024

Well, the noop processor is only there to check if the setup is correct. It's not really required

@Techn1x
Copy link
Author

Techn1x commented Feb 14, 2024

Just throwing in my uninformed comments. Sorry if this is too many cooks in the kitchen 😄

Unfortunately I couldn't fully switch because typescript/eslint also do not have a flat config yet

They've since cut a release with a flat config - going to attempt a flat config in my project today.

I got this far with trying to create a test project: ...
But ran in to: [babel legacy decorators bug] babel/babel#16239

Looks like they have merged a fix and cut a release for that now, hopefully no longer a blocker

what's the issue? (am I confused? lol)

Not sure who this is directed at - but to clarify - I could write a flat config for my project as you have done over in ember-eslint-parser where you mostly just imported the recommended rules from this project (eslint-plugin-ember) and defined the rest yourself. But the README for this project (eslint-plugin-ember) suggests that the recommended config from this project should be usable as a flat config, when it currently can't be due to it not actually being flat-config-compatible.

### 2. Update your config
```js
// eslint.config.js (flat config)
const eslintPluginEmberRecommended = require('eslint-plugin-ember/configs/recommended');
module.exports = [
...eslintPluginEmberRecommended,
];
```

@Techn1x
Copy link
Author

Techn1x commented Feb 14, 2024

If it's helpful, this is what I ended up with as flat config for my project - doesn't use the configs from this project, decided to define everything myself

Some extra notes. Until eslint v9 is released, flat config and mjs/cjs files aren't on by default. So when running in CLI need to use a command like;
ESLINT_USE_FLAT_CONFIG=true eslint . --quiet --config eslint.config.mjs
And also need to configure .vscode/settings.json

  "eslint.experimental.useFlatConfig": true,
  "eslint.options": {
    "overrideConfigFile": "eslint.config.mjs"
  },
expand for config
import globals from 'globals'
import js from '@eslint/js'
import tseslint from 'typescript-eslint'

import eslintPluginEmber from 'eslint-plugin-ember'
import eslintPluginNode from 'eslint-plugin-n'

import emberParser from 'ember-eslint-parser'
import babelParser from '@babel/eslint-parser'

import emberConfig from 'eslint-plugin-ember/configs/recommended'
import emberGJSConfig from 'eslint-plugin-ember/configs/recommended-gjs'
import emberGTSConfig from 'eslint-plugin-ember/configs/recommended-gts'
import prettierConfig from 'eslint-config-prettier'

const eslintPluginTypescript = tseslint.plugin
const typescriptParser = tseslint.parser
/**
 * typescript-eslint doesn't seem to export a useful flat set of rules for a recommended set, rather full configs.
 * So we build the rules we need out of those configs.
 */
const typescriptRules = {
  ...tseslint.configs.recommended.reduce((rules, item) => ({ ...rules, ...item.rules }), {}),
}

const commonEmberRules = {
  'ember/route-path-style': 'error',
  // ember rules to work on
  'ember/no-get': 'off', // remaining usage is limited & mostly necessary. Too dangerous as warn due to autofixing.
  'ember/no-computed-properties-in-native-classes': 'warn',
  'ember/no-classic-components': 'warn',
  'ember/no-runloop': 'warn',
  'ember/no-array-prototype-extensions': 'warn',
  // off to avoid some lint noise for a while..
  'ember/require-tagless-components': 'off',
  'ember/no-classic-classes': 'off',
}

const commonJavascriptRules = {
  'no-unused-vars': [
    'error',
    {
      vars: 'all',
      args: 'after-used',
      argsIgnorePattern: '^_',
      destructuredArrayIgnorePattern: '^_',
      ignoreRestSiblings: true,
    },
  ],
}

const commonTypescriptRules = {
  ...typescriptRules,
  // replace the JS rule with a TS one
  'no-unused-vars': 'off',
  '@typescript-eslint/no-unused-vars': [
    'error',
    {
      vars: 'all',
      args: 'after-used',
      argsIgnorePattern: '^_',
      destructuredArrayIgnorePattern: '^_',
      ignoreRestSiblings: true,
    },
  ],
  '@typescript-eslint/no-empty-function': 'off',
  '@typescript-eslint/consistent-type-imports': 'error',
  '@typescript-eslint/no-explicit-any': 'warn',
}

export default [
  js.configs.recommended, // standard eslint recommended config
  // JS
  {
    files: ['**/*.js'],
    plugins: {
      ember: eslintPluginEmber,
    },
    languageOptions: {
      parser: babelParser,
      parserOptions: {
        sourceType: 'module',
        requireConfigFile: false,
        babelOptions: {
          plugins: [['@babel/plugin-proposal-decorators', { decoratorsBeforeExport: true }]],
        },
      },
      globals: {
        ...globals.browser,
      },
    },
    rules: {
      ...emberConfig.rules,
      ...commonEmberRules,
      ...commonJavascriptRules,
    },
  },
  // TS
  {
    files: ['**/*.ts'],
    plugins: {
      ember: eslintPluginEmber,
      '@typescript-eslint': eslintPluginTypescript,
    },
    languageOptions: {
      parser: typescriptParser,
      globals: {
        ...globals.browser,
      },
    },
    rules: {
      ...emberConfig.rules,
      ...commonEmberRules,
      ...commonTypescriptRules,
    },
  },
  // GJS
  {
    files: ['**/*.gjs'],
    plugins: {
      ember: eslintPluginEmber,
    },
    languageOptions: {
      parser: emberParser,
      globals: {
        ...globals.browser,
      },
    },
    rules: {
      ...emberGJSConfig.rules,
      ...commonEmberRules,
      ...commonJavascriptRules,
    },
  },
  // GTS
  {
    files: ['**/*.gts'],
    plugins: {
      ember: eslintPluginEmber,
      '@typescript-eslint': eslintPluginTypescript,
    },
    languageOptions: {
      parser: emberParser,
      globals: {
        ...globals.browser,
      },
    },
    rules: {
      ...emberGTSConfig.rules,
      ...commonEmberRules,
      ...commonTypescriptRules,
    },
  },
  // mirage uses some functions that look like array prototype extensions
  {
    files: ['tests/mirage/**/*.{js,ts}'],
    rules: {
      'ember/no-array-prototype-extensions': 'off',
    },
  },
  // node files
  {
    ...eslintPluginNode.configs['flat/recommended-script'],
    files: [
      '.prettierrc.js',
      '.template-lintrc.js',
      'ember-cli-build.js',
      'testem.js',
      'blueprints/*/index.js',
      'config/**/*.js',
      'lib/*/index.js',
      'server/**/*.js',
      'tailwind.config.js',
      'contentful/**/*.js',
    ],
    rules: {
      ...eslintPluginNode.configs['flat/recommended-script'].rules,
      'n/no-unpublished-require': 'off',
      '@typescript-eslint/no-var-requires': 'off',
    },
  },
  // this disables stylistic rules that prettier handles. should be last.
  prettierConfig,
  {
    // equivalent of old .eslintignore file
    ignores: [
      // compiled output
      'dist',
      'tmp',
      // misc
      '!.*',
      '.*/',
      '.eslintcache',
      'node_modules',
    ],
  },
]

@bmish
Copy link
Member

bmish commented Feb 14, 2024

I think this should fix it, please test if you can. The issue with our flat config is described in this issue and I have reproduced it to confirm it's a problem.

@Techn1x
Copy link
Author

Techn1x commented Feb 15, 2024

If it's helpful, this is what I ended up with as flat config for my project

In case anyone's using that as an exmaple, a small refinement to that config to use tseslint.config() function can be found here;
typescript-eslint/typescript-eslint#8465 (comment)

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

Successfully merging a pull request may close this issue.

4 participants