import-js / eslint-plugin-import

ESLint plugin with rules that help validate proper imports.
MIT License
5.46k stars 1.56k forks source link

Missing documentation on how to setup eslint-plugin-import with flat confiuration and Typescript #3051

Closed valleywood closed 1 week ago

valleywood commented 1 week ago

Readme only contains information about how to setup typescript for this plugin using the old (non-flat) eslint configuration format. https://github.com/import-js/eslint-plugin-import?tab=readme-ov-file#typescript

Upgraded to latest eslint-plugin-import and now the compat functions from @eslint/compat cant be used any longer (fixupConfigRules/fixupPluginRules) and i don't manage to lint my ts files any longer.

I get loads of errors like this:

 error  Parse errors in imported module '@emotion/cache': parserPath or languageOptions.parser is required! (undefined:undefined)  import/default

I have a languageOptions.parser defined in my config:

 const tsParser = require('@typescript-eslint/parser');
 ...

 languageOptions: {
            parser: tsParser,
            ecmaVersion: 6,
            sourceType: 'module',
            parserOptions: {
                ecmaFeatures: {
                    jsx: true,
                },
            },
        },
connorjs commented 1 week ago

The PR has some documentation I was trying to follow, but I have not fully resolved it yet (likely a me issue). See https://github.com/import-js/eslint-plugin-import/pull/3018

Also, until I properly update to latest versions, I pinned eslint-module-utils to ~2.8. (Running npm upgrade led to similar errors as OP, so I figured Iā€™d note this too.)

ljharb commented 1 week ago

cc @michaelfaith it's possible #3018 caused some regressions in eslint-module-utils.

michaelfaith commented 1 week ago

That's interesting, though, I'm not sure how that PR could have caused and issue with eslint-module-utils. The only change to the utils package was expanding the default extensions array to include .mjs and .cjs, which was more of an after thought and not critical for the flat config support. Certainly don't mind reverting that, if that's causing issues; though I don't think that's at play here. Could it have been this change that came in 2.8.2? https://github.com/import-js/eslint-plugin-import/commit/c387276efac8da06e0d8a4eae06989aa0e6631eb
@connorjs does 2.8.2 work for you?

As far as documentation, I can improve on that for sure. @valleywood did you check out the typescript example that I added to the repo with the flat config PR: https://github.com/import-js/eslint-plugin-import/blob/main/examples/flat/eslint.config.mjs

connorjs commented 1 week ago

2.8.2 worked for me. I'll do some more digging into my ESLint config. I was trying to do an "update all" pass and got everything except import plugin updated. I was also running npm upgrade in order to get transitive dependencies also up to date: that's when I found the utils change to be source of my issue. (Which, again, could be a me issue.)

I'll open a separate issue for what I see if needed though. We can keep this focused on documentation if desired? Up to y'all

jordan-erisman commented 1 week ago

That's interesting, though, I'm not sure how that PR could have caused and issue with eslint-module-utils. The only change to the utils package was expanding the default extensions array to include .mjs and .cjs, which was more of an after thought and not critical for the flat config support. Certainly don't mind reverting that, if that's causing issues; though I don't think that's at play here. Could it have been this change that came in 2.8.2? c387276 @connorjs does 2.8.2 work for you?

As far as documentation, I can improve on that for sure. @valleywood did you check out the typescript example that I added to the repo with the flat config PR: https://github.com/import-js/eslint-plugin-import/blob/main/examples/flat/eslint.config.mjs

@michaelfaith, I am also experiencing the same error here: parserPath or languageOptions.parser is required!

I cloned your example repo with the flat config and one way I was able to reproduce was by trying to import a third party library. For example:

yarn add redaxios

imports.ts

//import c from './exports';
import { a, b } from './exports';
import type { ScalarType, ObjType } from './exports';
import redaxios from 'redaxios';

import path from 'path';
import fs from 'node:fs';
import console from 'console';
image

Interesting side issue with that project

If i dont comment out 'import/no-unused-modules': ['warn', { unusedExports: true }], then I get this error:

Oops! Something went wrong! :(

ESLint: 8.57.0

ESLint couldn't find a configuration file
valleywood commented 1 week ago

@michaelfaith Missed that example, thanks! (was looking in the README only) However that example doesn't use make use of the the eslint-import-resolver-typescript as the old example does šŸ¤” Don't know if's relevant/needed any longer though?

Also notably are that the only packages that are complaining about this in my case are the the @emotion prefixed imports (@emotion/styled, @emotion/cache, @emotion/server/create-instance etc.) all other packages seem to work.

Thanks @connorjs for the suggestion to pin eslint-module-utils to version 2.8.2. That made the errors with parserPath or languageOptions.parser is required! go away.

Looking forward to a solution though that doesn't require resolution/overrides šŸ™

SStranks commented 1 week ago

@michaelfaith, I am also experiencing the same error here: parserPath or languageOptions.parser is required!

I'm experiencing this error, under the rule 'import/namespace', for both imported npm packages and also local pnpm workspace packages I have created. Using typescript, with the settings described in the docs for @typescript-eslint/parser and eslint-import-resolver-typescript.

michaelfaith commented 1 week ago

I cloned your example repo with the flat config and one way I was able to reproduce was by trying to import a third party library. For example:

yarn add redaxios

imports.ts

//import c from './exports';
import { a, b } from './exports';
import type { ScalarType, ObjType } from './exports';
import redaxios from 'redaxios';

import path from 'path';
import fs from 'node:fs';
import console from 'console';
image

This was helpful. For the flat config example in the repo, if you explicitly define a parser for the other configs, then the error goes away. e.g.

import importPlugin from 'eslint-plugin-import';
import js from '@eslint/js';
import tsParser from '@typescript-eslint/parser';

export default [
  js.configs.recommended,
  importPlugin.flatConfigs.react,
  {
    files: ['**/*.{js,mjs,cjs,jsx,mjsx,ts,tsx,mtsx}'],
    ...importPlugin.flatConfigs.recommended,
    ...importPlugin.flatConfigs.typescript,
    languageOptions: {
      parser: tsParser,
      ecmaVersion: 'latest',
      sourceType: 'module',
    },
  },
  {
    languageOptions: {
      parser: tsParser,
      ecmaVersion: 'latest',
      sourceType: 'module',
    },
    ignores: ['eslint.config.mjs', '**/exports-unused.ts'],
    rules: {
      'no-unused-vars': 'off',
      'import/no-dynamic-require': 'warn',
      'import/no-nodejs-modules': 'warn',
      'import/no-unused-modules': ['warn', { unusedExports: true }],
    },
  },
];

Which makes me wonder if this is related to what @G-Rath has been experiencing while trying to upgrade the plugin to support v9. They've been getting the same errors while trying to run the unit test suite with v9, and through console logging, found that eslint wasn't putting the default parser (espree) on the context, when a parser wasn't explicitly defined (https://github.com/eslint/eslint/issues/17953#issuecomment-2189846364). So, it could be a bug with eslint, or it could be that the context isn't meant to have the parser attached if the user didn't define a parser, which makes the check that this plugin does in parse invalid and we need to fallback on a default ourselves, rather than relying on there always being one defined on the context, which is the current assumption the code makes.
In any event, if you try explicitly adding a parser, does that overcome the issue for you?

michaelfaith commented 1 week ago

Actually, I think I might have figured it out why the parser isn't making it through on the context. I should be able to get a fix in ~later today~ now.

jordan-erisman commented 1 week ago

Actually, I think I might have figured it out why the parser isn't making it through on the context. I should be able to get a fix in ~later today~ now.

@michaelfaith, this seems to get me passed the no parser error! I can go ahead and approve my from standpoint (not sure who officially needs to do it). Would love if this got released soon!

michaelfaith commented 1 week ago

Thanks for verifying.

jordan-erisman commented 1 week ago

@ljharb, I see this fix was merged in. Is this something that will be released soon? If there's a better spot for me to ask, please let me know

ljharb commented 1 week ago

Yes, but in general asking doesn't do much, so the best thing is to wait :-)