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

[import/order] Aliases that start with @ get incorrectly grouped #1293

Closed
jeffshaver opened this issue Feb 20, 2019 · 11 comments · Fixed by #1294
Closed

[import/order] Aliases that start with @ get incorrectly grouped #1293

jeffshaver opened this issue Feb 20, 2019 · 11 comments · Fixed by #1294

Comments

@jeffshaver
Copy link
Contributor

Given that Typescript is going to switch over to ESLint as its main linting tool, I wanted to try to get my project at work switched over. In Typescript, you can define paths. Paths allow you to define import paths, i.e. @my-alias/fn could actually point to src/my-folder/fn.ts.

In my project, currently using TSLint, we have external, aliases, parent, sibling as our import sort order. After trying to get a test case working with ESLint, it seems like this plugin incorrectly associates aliases that start with @ to external modules, since it thinks they are "scoped".

I don't know the solution to this, however, I would be open to putting in a PR if someone can provide some insight into solving this?

I have tried to create a minimal repro, but it seems like code-sandbox won't give access to the terminal... But this example does fail, saying that you can't have newlines between groups on the lodash import. https://codesandbox.io/s/7m5zpmxo90

@ljharb
Copy link
Member

ljharb commented Feb 20, 2019

It seems like a typescript resolver (to provide to this linting plugin) would address this, by being able to read the paths.

@jeffshaver
Copy link
Contributor Author

@ljharb i am using the typescript resolver. The paths getting sent to this plugin are correct (did a bunch of logging because i thought i did something wrong).

@jeffshaver
Copy link
Contributor Author

for instance, if i have an alias called @my-alias, the name the plugin gets from my import of @my-alias/fn is @my-alias/fn and the path is @my-alias/fn /Users/jeff/Documents/eslint-import-group-test/src/my-folder/fn.ts

@jeffshaver
Copy link
Contributor Author

this seems like a bug since the documentation states that only folders included in the import/external-module-folders are considered external

@jeffshaver
Copy link
Contributor Author

jeffshaver commented Feb 20, 2019

I am not sure if this is the right way to go about this, but changing the following things in importType.js "fixes" this issue:

// isScoped now gets settings and path and also checks whether the path is external
isScoped (name, settings, path) {
  return scopedRegExp.test(name) && isExternalPath(path, name, settings)
}

// isInternalModule checks to see if the (scoped OR external regexes pass) AND
// it is not an external path
function isInternalModule (name, settings, path) {
  return (
    (scopedRegExp.test(name) || externalModuleRegExp.test(name)) &&
    !isExternalPath(path, name, settings)
  )
}

@jeffshaver
Copy link
Contributor Author

and specifically, I know that this is isScoped causing the issue because if i change my typescript alias to my-alias (remove the @), it works as expected with no changes.

@ljharb
Copy link
Member

ljharb commented Feb 21, 2019

A PR with that change, and test cases would be appreciated :-) I think it'd help make clear what's needed.

jeffshaver added a commit to jeffshaver/eslint-plugin-import that referenced this issue Feb 21, 2019
jeffshaver added a commit to jeffshaver/eslint-plugin-import that referenced this issue Feb 21, 2019
jeffshaver added a commit to jeffshaver/eslint-plugin-import that referenced this issue Feb 21, 2019
jeffshaver added a commit to jeffshaver/eslint-plugin-import that referenced this issue Feb 21, 2019
jeffshaver added a commit to jeffshaver/eslint-plugin-import that referenced this issue Feb 21, 2019
jeffshaver added a commit to jeffshaver/eslint-plugin-import that referenced this issue Feb 23, 2019
jeffshaver added a commit to jeffshaver/eslint-plugin-import that referenced this issue Mar 3, 2019
jeffshaver added a commit to jeffshaver/eslint-plugin-import that referenced this issue Mar 3, 2019
ljharb pushed a commit to jeffshaver/eslint-plugin-import that referenced this issue Mar 3, 2019
@michielmetcake
Copy link

Could anyone tell me why this fix is not working for me?

When importing my modules like example below. I still get the error: 'There should be at least one empty line between import groups eslint(import/order)'

Could anyone tell me what i'm doing wrong here?

import { Icon, PlaceHolder, TransactionType } from '@Components';
import { dateDiff } from '@Utilities';

Using:
"name": "eslint-plugin-import",
"version": "2.18.0"

"name": "eslint",
"version": "5.16.0",

import rule:

     'import/order': [
       WARN,
       {
         groups: ['builtin', 'external', 'internal', [('parent', 'sibling', 'index')]],
         'newlines-between': 'always',
       },
     ],

.eslintrc.js

module.exports = {
  extends: ['react-app', 'prettier', 'prettier/react'],

  // Stop ESLint from looking for a configuration file in parent folders
  root: true,

  plugins: ['react'],

  settings: {
    'import/core-modules': ['redux-saga/effects'],
    'import/resolver': {
      alias: {
        map: [
          ['@ActionTypes', './src/services/store/actionTypes'],
          ['@AuthApi', './src/services/api/auth-api']
          .......
        ],
      },
      extensions: ['.js'],
    },
  },

  parser: 'babel-eslint',
  parserOptions: {
    ecmaVersion: 2017,
    sourceType: 'module',
    ecmaFeatures: {
      experimentalObjectRestSpread: true,
    },
    env: {
      browser: true,
      node: true,
    },
  },

@ljharb
Copy link
Member

ljharb commented Jul 23, 2019

Are you sure that’s the final rule config, and that none of the things you extend is setting it?

@michielmetcake
Copy link

I'm using the CRA. But I'm also extending my CRA with react-app-rewired.

Currently I'm overwriting some CRA config by using the customize-cra.

Is this what you mean by final rule config?

My config-overrides file looks like this:

module.exports = {
  webpack: config => {
    config.resolve = {
      alias: {
        '@ActionTypes': `${root}/services/store/actionTypes.js`,
        '@AuthApi': `${root}/services/api/auth-api.js`,
        ........
      },
    };

    return config;
  }
}

module.exports = override(useEslintRc('./.eslintrc.js'));

@liuxinqiong
Copy link

@michielmetcake excuse me, Had you solved the problem?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants