no-relative-parent-imports over-reporting when import/resolver = typescript

[michaelfarrell76]

I have been using the following tslint rule:

"no-relative-imports": [true, "allow-siblings"],

to forbid any relative imports such as import constants from '../constants'

I am trying to migrate my tslint rules to eslint, and it seems like the rule import/no-relative-parent-imports should be what I am looking for. However, when I turn the rule on, all of my imports that are path rewrites in my tsconfig file report errors. See image below.

I have the following tsconfig.json

  ...
  "baseUrl": ".",
  "paths": {
     "@commons/*": ["./commons/src/*"]
  },
  ...

And the following eslintrc.js

 ...
 "import/no-relative-parent-imports": ["warn"],
 ...
 "settings": {
    "import/resolver": {
      "typescript": {},
    }
  }

...

When I log out the input before the error is reported here, I see the following paths getting reported:

../../types.ts
../../types.ts { id: 'import/no-relative-parent-imports',

It seems as though the typescript import/resolver is translating the paths before they are applied to this eslint check. Is there a way that I can configure my eslint to work with this use case? or is this not yet possible

[ljharb]

the point of the rule is about resolved parent imports, not aliased ones - if all you want is to block ../ you can use no-restricted-syntax and an AST selector.

I think it would work the same with a webpack alias.

[benmosher]

What TS parser are you using? I don't think it's the resolver, I think it's the TS parser rewriting the path before the lint rule runs.

[michaelfarrell76]

@benmosher "parser": "@typescript-eslint/parser", the parser is rewriting, I can allow for the parser to rewrite but have the rule be applied before then

[benmosher]

I'm not sure what you mean. I think the parser always runs before the lint rule.

The parser would need to choose not to process the paths first. I am not 100% sure it's doing this but when I looked at the code for no-relative-parent-imports IIRC it's using the path straight from the AST.

[benmosher]

Sorry, I just double-checked and I must have looked in the wrong place before. It is resolving the path inside the rule which means it is the plugin's problem.

[benmosher]

wait, @ljharb, by this

the point of the rule is about resolved parent imports, not aliased ones

you mean that you think this rule shouldn't only fire on import paths that start with ..?

[benmosher]

I think the fix for #1123 was an overcorrection and it should still check for a leading . in the path. if you agree, @ljharb?

[ljharb]

@benmosher no, what i mean is, the path you type in your code shouldn't matter - only what it resolves to should matter.

[smably]

if all you want is to block ../ you can use no-restricted-syntax and an AST selector

@ljharb This is not a great option when extending a config such as eslint-config-airbnb that already enables no-restricted-syntax with its own config, since any options I set on the rule would clobber the ones from the shared config. I could write my own rule, but that seems like overkill.

And while some might be using this rule to prevent resolution of imports higher in the tree, I just want to prevent the use of .. in imports since it makes them inherently fragile if files are moved to a different level in the tree, and because paths starting with chains of ../.. are hard to read. I do specifically care about the way that import paths are specified.

[ljharb]

@smably you're correct about the difficulty there; you could write your own rule using eslint-rule-composer and no-restricted-syntax, though.

.. doesn't make things any more fragile than literally any other path. Any file rename or move potentially means you have to update all consumers. The "hard to read" part, I disagree with but recognize is subjective.

[michaelfarrell76]

.. doesn't make things any more fragile than literally any other path.

It may be true that the path is equally "fragile" on a rename or move but if the paths are absolute (or pseudo-absolute by using an alias), when you move a file you have a much stronger understanding of where that path moved to.

Take for example the two pieces of code:

Relative

import addTwoNumbers from '../addToNumbers';

Aliased

import addTwoNumbers from '@backend/utils/addToNumbers';

Say i now move the function addToNumbers to my common utility function module because I want to share it with my frontend project.

the new code will be

Aliased Updated

import addTwoNumbers from '@commons/utils/addToNumbers';

Both of the relative and aliased paths will require an update of course but I can now do a find and replace in my editor for @backend/utils/addToNumbers -> @commons/utils/addToNumbers which is incredibly easy in the case of aliased paths, but I will have to manually go through all relative paths because a find and replace on ../ is not as simple.

However, I do understand if this is the wrong rule for this desired outcome.

[ljharb]

Yes - I think you want a custom rule that forces every specifier to be bare - ie, to forbid relative paths entirely.

[mosanger]

just ran into the same issue. an easy solution maybe:
#669 (comment)

[bduffany]

The unfortunate thing about the workaround in #669 is that it doesn't allow adding custom error messages, because no-restricted-import only allows custom error messages for exact path matches (but not patterns). So the developer will see "this import is restricted by a pattern" but no explanation of why.

[bduffany]

I'm running into an issue that ../../foo/bar.ts is correctly reported as an error but ../../foo/bar is not. Is that within the scope of this issue or should I start a new one?

[ljharb]

I would expect both to resolve to the same thing if the typescript resolver is set up properly.

[bduffany]

@ljharb they do both resolve to the same thing and import correctly at runtime. But one is reported as an error by ESLint when no-restricted-import is enabled and one is not. I would expect both to be reported as errors by ESLint because both are relative imports.

[ljharb]

Yes, I agree with you.

[adarnon]

Any news for this problem? It is prohibiting the use of the no-relative-parent-imports rule in TS packages with path aliases.

[josephdpurcell]

Here is how I implemented this on a NestJS project with Typescript:

module.exports = {
  rules: {
    'no-restricted-imports': ['error', {'patterns': ['..*']}],
  },
};

It prevents any import statement that references the parent dir as relative, forcing the use of an alias. I didn't figure out how to add a custom message.

[johnjensenish]

I didn't figure out how to add a custom message.

Here's an example of a custom message (docs):

module.exports = {
  rules: {
    'no-restricted-imports': [
      'error',
      {
        patterns: [
          {
            group: ['../*'],
            message: 'Usage of relative parent imports is not allowed.',
          },
        ],
      },
    ],
  },
};

[JounQin]

close in favor of https://github.com/import-js/eslint-import-resolver-typescript

Feel free to comment if it still occurs.

[ljharb]

@JounQin this one seems to already be using the TS resolver

[JounQin]

@JounQin this one seems to already be using the TS resolver

Hah, sorry to misread the thread.

But there is no reproduction was provided, so it's hard to tell what was the cause then.