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

no-implicit-dependencies: Support path mapping #3364

Closed
ghost opened this issue Oct 20, 2017 · 18 comments
Closed

no-implicit-dependencies: Support path mapping #3364

ghost opened this issue Oct 20, 2017 · 18 comments

Comments

@ghost
Copy link

ghost commented Oct 20, 2017

Bug Report

  • TSLint version: 5.8.0
  • TypeScript version: 2.7.0-dev.20171020
  • Running TSLint via: CLI

TypeScript code being linted

src/a.ts

import { x } from "foo";

types/foo.d.ts

export const x = 0;

tsconfig.json

{
    "compilerOptions": {
        "paths": {
            "*": "types/*"
        }
    }
}

with tslint.json configuration:

{
    "rules": {
        "no-implicit-dependencies": true
    }
}

Actual behavior

ERROR: /home/andy/sample/tslint/src/a.ts[1, 19]: Module 'foo' is not listed as dependency in package.json

Expected behavior

No error. I think if an import isn't resolved to something in node_modules this rule should ignore it.

@ajafff
Copy link
Contributor

ajafff commented Oct 20, 2017

I think if an import isn't resolved to something in node_modules this rule should ignore it.

The rule doesn't try to resolve the modules. That would mean you need to have all dependencies installed, which is not really possible with peerDependencies and optionalDependencies. With the current implementation you can lint a fresh clone without installing anything. I guess that's what most code quality tools do.

As I understand path mappings, they only exist at compile time. At runtime you still need the module installed for node / webpack / whatever to correctly pick it up.
That means path mappings are only relevant if you have type only imports that are elided during compilation. In this case the rule is probably not the right choice for you.

@ghost
Copy link
Author

ghost commented Oct 20, 2017

In our case there is generally no package.json at all, so I guess we should just disable this rule then. Thanks!

@ghost ghost closed this as completed Oct 20, 2017
@marcoqu
Copy link

marcoqu commented Oct 24, 2017

In my case, I'm "importing" separate typescript projects I'm working on simultaneously using path mapping:

"compilerOptions": {
    ...
    "paths": {
        "tsbase": ["../tsBaseProject/src"],
        "tslibrary": ["../tsProjectLibrary/src"]
    }
}

so that I can use them in the project as if they were modules.
Is there a way to whitelist them?

@ajafff
Copy link
Contributor

ajafff commented Oct 24, 2017

@marcoqu Path mappings are only relevant at compile time. At runtime these modules need to exist in node_modules. I suggest adding them as either as dependencies or peerDependencies to your package.json

@marcoqu
Copy link

marcoqu commented Oct 24, 2017

When I compile the main source that "imports" the secondary projects, everything is compiled into a single bundle as if they were actual folders in the project. I do not need to have them in the node_modules folder.
To be clear, in the secondary project folders I have actual .ts files, not only the type declarations.

@jmolero
Copy link

jmolero commented Oct 27, 2017

+1 for adding a whitelist like in no-submodule-imports

@mika-fischer
Copy link

We also have the case that we use case that we define a path alias '~' to the base dir in order to avoid relative imports. This alias is later resolved by webpack, fuse-box, etc. Starting with 5.8, tslint spits out lots of spurious errors because of this...

@rkrupinski
Copy link

What he said ^^

@speedy250
Copy link

After upgrading I now have hundreds of these errors for the same reasons outlined above. Kind of a pointless rule.

@fredgate
Copy link

This rule does not work when we use alias for our own source code (not to import from npm packages). It is very useful to use absolute path instead of relative path.
In our tsconfig.json, for an angular application, we have just to add :

"compilerOptions": {
  ...
  "baseUrl": "./src",
  "paths": {
    "~/env": ["environments/environment"],
    "~/*": ["app/*"]
  }
}

Then we can do imports like :

import {FooService} from '~/core';
import {Environment} from '~/env';

May be we should reopen this issue to solve this case (there is no need for node_modules, just the tsconfig.json file).
I appreciate this rule, so it will be unfortunate to disable it.

@chris5287
Copy link

@andy-ms please reconsider supporting paths (we use it extensively with nx workspaces). This is a really useful rule, but for now I am forced to disable it.

@danielkcz
Copy link

I tried looking through the source code and fix would need to go somewhere along these lines. I've also checked how Typescript is handling that and I tracked it down to this function. It's definitely no easy feat to duplicate that logic. I am not sure if that function can be reused, there is a bunch of arguments I am not certain of.

@tdolsen
Copy link

tdolsen commented Mar 28, 2018

I'd very much like to see this getting fixed as well. Path mapping is a highly valued feature. I also tried using linked modules as a workaround, but those aren't supported either.

I did figure out a workaround that solves the issue somewhat for me, but it's not certain it will for everyone, or that it's maintainable at all. Anyway, the solution goes as follows:

Add a fake package to optionalDependencies with the name of the path map from tsconfig.json, and install dependencies using npm install --no-optional. This unfortunately doesn't work with yarn --ignore-optional - it fails trying to fetch the package still.

So, with paths in tsconfig.json looking something like this:

	"paths": {
		"~/*": ["src/*"],
		"some-path/*": ["whatever/*"]
	}

And optional in package.json like this:

	"optionalDependencies": {
		"~": "tslint-hack",
		"some-path": "tslint-hack"
	},

It should be possible to get production and development dependencies to install using npm install --no-optional. This obviously assumes you don't need any optional dependencies installed. Also worth mentioning I didn't get it working with @ as the package name.

If you choose to use this hack can it probably be smart to add a .npmrc file to the project root, with optional=false configured, so you can go back to running npm install without the --no-optional flag.

Another solution that should work is to actually create a package with the desired name and publishing it to a private registry, using verdaccio or similar. I belive it to be possible to configure private registries per module using .npmrc or .yarnrc, and as such should be better in terms of maintainability. None of this is tested though.

Hope this can help out a little for those that want to use this tslint rule and keep their module resolutions in place. But it's no substitute for a proper fix..

@josecolella
Copy link

Also having this issue, resulting in sonar flagging this incorrectly as a code smell.

@a7madgamal
Copy link

I think this is a valid request since tslint is all about typescript and paths are a valid (and important) typescript setting.

@omkar-joshi
Copy link

omkar-joshi commented Sep 14, 2018

What if I have the package.json in a different directory than that of tslint.json?

- web
    - package.json
    - ClientApp
        - tslint.json

I'm working on a similar setup and I'm getting errors in almost all the files due to this rule. Any solution for this?

@ifiokjr
Copy link
Contributor

ifiokjr commented Sep 27, 2018

The way I've found for getting around this issue is using the following configuration option.

"no-implicit-dependencies": [true, ["src", "app", "~"]]

It whitelists the paths provided. Obviously, this means duplication but it's a quick fix if you're looking for one.

For those of us that use @ symbols as prefixes for custom paths I've raised a PR to fix a small bug with the current implementation #4192

"no-implicit-dependencies": [true, ["@src", "@app", "~"]]

@marcosfede
Copy link

@ifiokjr I was using @ as my src alias, so my imports looked like @/components
Couldn't set @ as ignored path since it tried to import @/components as a whole module instead of resolving @ first.

I changed my alias to ~ and used the line above in my tslint, solving the problem

liontwinkle added a commit to liontwinkle/python-demo-ex that referenced this issue Aug 1, 2020
Summary
=======

In preparation to have a Client App in addition to the existing
Stylist App I added the support for having code that is common
for both mobile apps.

The proposed directory structure is this:

\mobile
  \client  <-- Client App (Ionic)
  \stylist <-- Stylist App (Ionic)
  \shared  <-- shared code for both apps

When the Client App is actually created some of the files
that are currently in mobile/stylist directory need to be moved
to mobile/shared directory (e.g. some API services, models, etc.)

Solution
========

To support this structure the following is done:

1. In tsconfig.json add baseUrl and paths to map. This
adds TS compile support.

2. In webpack.config.js add support for @shared alias
to map to the same directory. This adds webpack bundling
support.

3. In watch.config add files "shared" directory to the
the watched list. This enables auto-recompiled in livereload
mode for shared files.

4. Move all files that previously were in "mobile" directory
to "mobile/stylist" directory.

5. Add example files in shared directory and demonstrate
that they can be used in the Stylist App (used in app.component.ts).

6. I had to disable "no-implicit-dependencies" linting rules,
I was unable to make it work with the shared directory properly
(Known issue: palantir/tslint#3364).

7. By popular demand used the same aliasing technique to
make ~ an alias for root source code directory. Now we can import
files like this:
    import { TodayComponent } from '~/today/today.component';

TODO
====

1. Verify that we can also create tests for shared code.

2. Split ios-cert and have separate certificates for Stylist
and Client apps

3. Add separate CI build for Client App when it is created
and split c-checkchanges script to check corresponding directory
for each mobile apps plus the shared directory.
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests