Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

How should I use no-relative-imports #435

Closed
AndreasCag opened this issue Jun 9, 2018 · 18 comments
Closed

How should I use no-relative-imports #435

AndreasCag opened this issue Jun 9, 2018 · 18 comments
Labels
Difficulty: Easy Someone with little to no experience in TSLint should be able to send a pull request for this issue. Status: Awaiting More Feedback Type: Breaking Change Type: Question

Comments

@AndreasCag
Copy link

AndreasCag commented Jun 9, 2018

Hello!
I am amazed how productive became my work with vscode, typescript and this config.
Thank you for your great work with it.

I'm new with typescript and tslint, so I'm a bit confused how to use rule 'no-relative-imports'.

Suppose I have small frontend project with Webpack + Vue + ts-loader.
Main core written with typescript, other files with javascript.

In folder 'core' I have two files:

ElData.ts

...

export class AnimationElement {
  ...
}

...

SportData.ts

import * as ElData from './ElData';

...

The thing is, 'no-relative-imports' throws error:
[tslint] Imported module is being loaded from a relative path. Please use an absolute path: import * as ElData from './ElData'; (no-relative-imports)

Hmm, disable rule is not my way to resolve a problem!

  1. First thing I try is just remove './' from my import like this:

SportData.ts

import * as ElData from 'ElData';

...

[tslint] Module 'ElData' is not listed as dependency in package.json (no-implicit-dependencies)

Well, lets try another way

  1. Improve tsconfig.json
{
  ...
  "compilerOptions": {
    ...
    "baseUrl": "./src"
  }
}

And then

SportData.ts

import * as ElData from 'core/ElData';

...

[tslint] Module 'core' is not listed as dependency in package.json (no-implicit-dependencies)
[tslint] Submodule import paths from this package are disallowed; import from the root instead (no-submodule-imports)

Arrgh

  1. Last hope

My webpack config a bit supercharged with aliases

{
  ....
  resolve: {
    alias: {
      '@': resolve('src'),
    }
  },
  ...
}

SportData.ts

import * as ElData from '@/core/ElData';

...

[tslint] Module '@/core' is not listed as dependency in package.json (no-implicit-dependencies)
[tslint] Submodule import paths from this package are disallowed; import from the root instead (no-submodule-imports)
[ts] Cannot find module '@/core/ElData'.

Jesus, all I achieved is just increase of errors.

How should I cook my imports?

@MatSFT
Copy link

MatSFT commented Jun 22, 2018

I have the same question. I cannot figure out how to get no-relative-imports to work with the no-submodule-includes and the no-implicit-dependencies issues.

Just seems like too much hassle so I disabled the no-relative-imports rule.

@JoshuaKGoldberg
Copy link

Hi there - welcome to TS/TSLint/VSCode/etc.! Glad you're enjoying the new area 😊 it's great to have you!

I personally don't use this rule in my projects. The idea as I understand it is to encourage setting a baseUrl in your tsconfig.json, but that seems uncomfortable to me. Should we deprecate this rule @HamletDRC ? Do you have more context around it?

@JoshuaKGoldberg JoshuaKGoldberg added Type: Question Status: In Discussion Please continue discussing the proposed change before sending a pull request. and removed Status: In Discussion Please continue discussing the proposed change before sending a pull request. labels Jul 4, 2018
@MatSFT
Copy link

MatSFT commented Jul 9, 2018

I've tried using the baseUrl method to get rid of the no relative paths issues, however when I do that I start violating two other rules: no-submodule-includes and no-implicit-dependencies.

no-submodule-includes is hit because I include components from subfolders in my solution.
no-implicit-dependencies is hit because the rule thinks that if I am not doing a relative include, then the code must be coming from a NPM package and wants me to include the package in my package.json file.

Can we drop the no-relative-imports rule from the microsoft tslint rule set? It might make sense for tiny projects or for npm packages but definitely does not work for website projects using tools such as webpack.

@JoshuaKGoldberg
Copy link

violating two other rules

Amusing. Yes, I'm in support of this. Let's leave this issue open for a few weeks, we can mark it as deprecated, then remove in a future major version.

@JoshuaKGoldberg JoshuaKGoldberg added Difficulty: Easy Someone with little to no experience in TSLint should be able to send a pull request for this issue. Type: Breaking Change Status: Awaiting More Feedback labels Jul 9, 2018
vitaly-redkin added a commit to vitaly-redkin/CHLELA_TEST_TASK that referenced this issue Jul 18, 2018
tslint:latest and tslint-microsoft-contrib rule sets applied.

Exceptions are set to:

"no-relative-imports": false,
"no-import-side-effect": [true, {"ignore-module": "(\\.css)$"}]

The first rule allow relative imports. It seems MS itself is going to deprecate them (microsoft/tslint-microsoft-contrib#435).

The second rule allows CSS files to be imported without an "import with side effects" warning.
@LukeSheard
Copy link

@JoshuaKGoldberg Please don't remove this, I really like this rule. We don't use the no-submodule-includes and just wrapped the no-implicit-dependencies rule to ignore the files in the baseUrl path and it works really well!

Maybe we can just publish this as a standalone rule?

@JoshuaKGoldberg
Copy link

Interesting! @LukeSheard - could you please give a little more info on why you chose that way?

@LukeSheard
Copy link

@JoshuaKGoldberg While I probably can't give amazingly detailed specifics.. Essentially we run a very large monorepo + single package.json at the root and so we run with the idea that our "top level" is a set of packages itself, meaning that imports look like

import { myThing } from "my/other/thing/over/here";

Since we other languages side by side with TypeScript we find that it gives nice consistency and also means if you move a file you can easily update imports by just searching for the module path.

@JoshuaKGoldberg
Copy link

JoshuaKGoldberg commented Jul 24, 2018

That's a really interesting approach, thanks for posting it! Have you tried good-fences?

Also, would you mind also posting your tslint.json config here (or just the relevant rules around paths, importing, and/or modules)?

Instead of deprecating this rule, since this is a very legitimate use case, let's say we remove it from the recommended ruleset and mention the monorepo use case in its documentation?

@LukeSheard
Copy link

@JoshuaKGoldberg I haven't come across good-fences before, but it looks very useful for our code base!

Our tslint.json is a little bit up in the air right now as we begin to let the dust settle, but I'll get follow up in a couple of days.

I think that removing it from the default recommended configuration is the best solution, leaving it for users to bring it if it works for their use case.

@LukeSheard
Copy link

Just to follow up here this is a the rough selection of our rules around imports and exports. To make things slightly more opaque we actually wrap this up with a script to check the no-implicit-dependencies outputs from tslint so that we ignore modules we know we have in the codebase.

{
    "extends": ["tslint:recommended", "tslint-microsoft-contrib", ... ],
    "defaultSeverity": "warning",
    "rules": {
        "no-default-export": true,
        "no-implicit-dependencies": [true],
        "no-submodule-imports": false,
        "no-relative-imports": true,
        "export-name": false,
        "no-import-side-effect": [true, { "ignore-module": "(\\.css)$" }],
    }
}

@zhouzi
Copy link

zhouzi commented Sep 9, 2018

While I can get behind the original reasons, I believe this rule should be removed for a few reasons:

  1. It's unconventional in the sense that it differs from traditional module resolution and needs a specific configuration.
  2. As said earlier in this thread, it conflicts with other rules.
  3. Typescript doesn't replace the aliases with the actual file path, e.g if ~/* is configured to be the root, ~/actions/todo will remain as is when compiled. Whatever is consuming the bundle would need to resolve the aliases as well. From what I read, Typescript is unlikely to change this behavior. See TypeScript#15479.
  4. Since it's unconventional, tools are likely to break. I'm using AVA and it fails to resolve the aliases. Perhaps the problem comes from AVA but still, relative paths (aka "traditional paths") are properly resolved.

I'm still pretty new to Typescript and getting my head around this rule is what took me the most time so far.

@IllusionMH
Copy link
Contributor

IllusionMH commented Nov 4, 2018

In v6.0 allow-siblings option will be available which allows imports from same or nested folders so ./ElData from original post will be allowed.

Does this option solve other problems (if any) in this thread?

@ghost
Copy link

ghost commented Jan 7, 2019

Also same experience here with conflicting "no-submodule-imports", "no-implicit-dependencies" and "no-relative-imports" rules.

I had defined an alias @app for absolute imports of project modules.
E.g. import xy from "@app/foo/bar"

What helped me, was to add this alias to the whitelist of the rules like this:

"no-implicit-dependencies": [true, ["@app"]],
"no-submodule-imports": [true, "@app"],
"no-relative-imports": [true, "allow-siblings"]

@alejohann
Copy link

This is not possible for Angular 7. Already tried every answer on the web :(

@ghost
Copy link

ghost commented Jan 18, 2019

Sorry to hear that. I can only speak from a react-ts app perspective. Solution above worked for me.

@WillSquire
Copy link

Tweaked @Ford04's solution and it seems to work well with a default React project structure:

{
  "no-implicit-dependencies": [true, "dev", ["src"]],
  "no-submodule-imports": [true, "src"],
  "no-relative-imports": [true, "allow-siblings"]
}

@abierbaum
Copy link

Just ran across this project that may be relevant to some people in this discussion.

https://github.com/vladimiry/tslint-rules-bunch

@JoshuaKGoldberg
Copy link

☠️ It's time! ☠️

Per #876, this repository is no longer accepting feature pull requests. TSLint is being deprecated and we recommend you switch to https://typescript-eslint.io.

Thanks for open sourcing with us, everyone!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Difficulty: Easy Someone with little to no experience in TSLint should be able to send a pull request for this issue. Status: Awaiting More Feedback Type: Breaking Change Type: Question
Projects
None yet
Development

No branches or pull requests

9 participants