Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Configurable no-implicit-dependencies Ignore to Support Absolute Path Imports #3483

Closed
bmarcaur opened this issue Nov 13, 2017 · 23 comments
Closed

Comments

@bmarcaur
Copy link
Member

bmarcaur commented Nov 13, 2017

Bug Report

  • TSLint version: 5.8.0
  • TypeScript version: 2.5.3
  • Running TSLint via: CLI (yarn)

TypeScript code being linted

# webpack config
{
  #
  # omitted
  #
  resolve: {
    alias: {
      "react/lib/ReactMount": "react-dom/lib/ReactMount",
      react: __dirname + "/node_modules/react",
      "#": SRC_DIR,
    },
    extensions: [".json", ".js", ".ts", ".tsx", ".less", ".css"],
    modules: [SRC_DIR, "node_modules"],
    mainFields: ["module", "jsnext:main", "browser", "main"],
  },
  #
  # omitted
  #
}

# Some later file
import {CollectorState} from "#/store/collectorState";

with tslint.json configuration:

{
  "extends": [
    "tslint:latest",
    "tslint-config-prettier"
  ],
  "rules": {
    "interface-name": false,
    "member-access": false,
    "object-literal-sort-keys": false,
    "arrow-parens": false,
    "no-empty-interface": false,
    "no-console": false,
    "no-submodule-imports": [true, "#"],
    "max-classes-per-file": [true, 3]
  }
}

Actual behavior

src/services/SearchService.ts
ERROR: 5:43   no-implicit-dependencies  Module '#' is not listed as dependency in package.json

Expected behavior

I use webpack resolve to support absolute path imports and tslint, on two different linters (no-submodule-imports and no-implicit-dependencies), picks this up as a package import and throws an error. My ask is the ability to configure an ignore for certain import prefixes (or packages in this scenario). Similar to:

    "no-submodule-imports": [true, "#"],

I would like to keep the rule enabled as it did help me catch an implicit import but it breaks the above workflow.

@ajafff
Copy link
Contributor

ajafff commented Nov 14, 2017

There's just one question: how to add the whitelisted modules to the existing config?

Currently this rule has two config options, so you can write "no-submodule-imports": [true, "dev", "optional"] without whitelisting dev or optional.
Simply adding the whitelisted dependencies after that (or in between) makes the configuration rather confusing.
In addition there is the possiblity of conflict between config options and actual package names.

I think the best approach would be a config object:

"whitespace": [
  true,
  {
    "dev": true,
    "optional": true,
    "whitelist": ["#", "~"] // property could be named "ignore" as well
]

@happycollision
Copy link

happycollision commented Dec 15, 2017

An aside:
I think @bmarcaur is looking to change the no-implicit-dependencies rule, not the no-submodule-imports rule, or the whitespace rule. You may already know this, but it is unclear to me if we are all on the same page.

My question/suggestion:
Is it possible to allow a config object OR two strings following true so that it does not break any current configs? So that either of the following would be acceptable:

"no-implicit-dependencies": [
  true,
  {
    "dev": true,
    "optional": true,
    "allow": ["#", "~", "/app"] // I like "allow" as the exception name, but ditch it if it conflicts with established conventions
  }
]

"no-implicit-dependencies": [ true, "dev", "optional" ]

@bmarcaur
Copy link
Member Author

bmarcaur commented Dec 15, 2017

Yeah sorry my example above uses another tslint rule to show an example of what I would want not that I want to change no-submodule-imports. Sorry that is a bit confusing. Also I imagine that you want to support a heterogeneous API here where it supports both the old and the new format @ajafff to avoid the API break. For example:

"no-implicit-dependencies": [
 true,
 {
   "dev": true,
   "optional": true,
   "allow": ["#", "~", "/app"] // I like "allow" as the exception name, but ditch it if it conflicts with established conventions
 },
 "dev",
 "optional"
]

edit: I started working on this but then I realized that I broke the API. So unfortunately no real progress to show. Ill take a look at this again when i have time over the holidays.

@viridia
Copy link

viridia commented Dec 22, 2017

I just ran into this as well - I have no choice but to disable these rules in my tslint.json.

@happycollision
Copy link

@viridia: You might consider doing some inline tslint disabling. That way, you'd still get feedback if you accidentally import something that is not in your package file.

That's what I settled on in the meantime. Of course, for my project, I didn't have very many places where I was importing in a way that caused an issue. If you do, it might be a bigger headache.

@viridia
Copy link

viridia commented Dec 23, 2017

@happycollision I used absolute imports pervasively throughout the code - based on advice from this medium post: https://spin.atomicobject.com/2017/10/07/absolute-paths-javascript/

@Legym
Copy link

Legym commented Feb 26, 2018

I'm also having an issue. This would be so great if we had a fix for this. I ended up disabling the rule in my tslint.json

@tdolsen
Copy link

tdolsen commented Mar 28, 2018

For those interested have I figured out a very hacky workaround, while we are waiting for a real fix.

@fchiumeo
Copy link

Any forecast if that will be implemented?

@7sDream
Copy link

7sDream commented Apr 15, 2018

Need this enhancement too, please consider implement it. 🌹

@vladnicula
Copy link

I would also benefit from this.

@benjaminbillet
Copy link

I had to disable the no-implicit-dependencies rule because I use absolute imports in my projects. This feature could be very useful.

@tbsvttr
Copy link

tbsvttr commented Jun 4, 2018

Need this feature here, too.

@formatlos
Copy link

this feature would be very much appreciated

@fchiumeo
Copy link

https://github.com/palantir/tslint/releases/tag/5.11.0

v5.11.0
[new-rule-option] Add whitelist for no-implicit-dependencies (#3979)

@jeznag
Copy link

jeznag commented Jul 30, 2018

Can documentation be added for this? I don't understand how to use it.

If I have
a. devDependencies
b. paths listed in tsconfig.json, e.g.

    "paths": {
      "@App/*": [
        "src/*"
      ],
      "@Shared/*": [
        "src/Shared/*"
      ]
    },

How would I use this?

I tried:

    "no-implicit-dependencies": [true, "dev", ["@App", "@Shared"]],

But I get this error:

/Users/jeremynagel/dev/energylink-project/react-components/data-exporter2/src/DataJunction/DataJunctionTable.tsx
(22,8): Module '@Shared/BootstrapTableFormatters' is not listed as dependency in package.json

@ilearnio
Copy link

ilearnio commented Aug 2, 2018

Got it working. Here is what I did:

  1. Upgrade to tslint@^5.11.0

  2. In tslint.json add an array of path's you want to whitelist to no-implicit-dependencies. In my case I only need to whitelist ~, so it looks like:

"no-implicit-dependencies": [true, ["~"]]
  1. In tsconfig.json I have the following paths
"paths": {
   "~/*": ["./*"]
},

@jeznag
Copy link

jeznag commented Aug 3, 2018

What about dev dependencies @ilearnio ?

@ilearnio
Copy link

ilearnio commented Aug 3, 2018

@jeznag Hm, not sure if I'll ever need them in my app's code. So far all works good

@chris5287
Copy link

@jeznag FYI if your paths start with @ then you need to specify the whole name (not just the package name) due to the way the package name is calculated (it seems to not split the name of if it’s starts with an @ symbol): see

function getPackageName(name: string): string {

@arsnl
Copy link

arsnl commented Aug 17, 2018

Damn! That's a good catch @chris5287 ! I was using the @ to define my local path. I guess i'll start using the ~ from now.

My tslint.json

{
  "defaultSeverity": "error",
  "extends": ["tslint:latest", "tslint-config-prettier"],
  "rules": {
    "no-submodule-imports": [true, "~root", "~src"],
    "no-implicit-dependencies": [true, ["~root", "~src"]]
  },
  "linterOptions": {
    "exclude": ["node_modules/**/*", "dist/**/*", "test/**/*"]
  }
}

My tsconfig.json

{
  "compilerOptions": {
    "strict": true,
    "module": "ESNext",
    "moduleResolution": "node",
    "target": "ESNext",
    "allowSyntheticDefaultImports": true,
    "outDir": "dist",
    "sourceMap": true,
    "baseUrl": ".",
    "paths": {
      "~root/*": ["./*"],
      "~src/*": ["src/*"]
    }
  }
}

So I can now import local modules without tslint nor ts error.

Eg:

import requestIdMiddleware from '~src/middlewares/request-id';

@didaktio
Copy link

Don't forget the wildcard (*) character! Easily missed but critical.

Incorrect:

"paths": {
      "path1/": ["dir/"]
    }

Correct:

"paths": {
      "path1/*": ["dir/*"]
    }

@palantir palantir locked and limited conversation to collaborators Sep 15, 2020
@JoshuaKGoldberg
Copy link
Contributor

🤖 Beep boop! 👉 TSLint is deprecated 👈 and you should switch to typescript-eslint! 🤖

🔒 This issue is being locked to prevent further unnecessary discussions. Thank you! 👋

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

No branches or pull requests