no-implicit-dependencies: Support path mapping #3364
Comments
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. |
In our case there is generally no |
In my case, I'm "importing" separate typescript projects I'm working on simultaneously using path mapping:
so that I can use them in the project as if they were modules. |
@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 |
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. |
+1 for adding a whitelist like in no-submodule-imports |
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... |
What he said ^^ |
After upgrading I now have hundreds of these errors for the same reasons outlined above. Kind of a pointless rule. |
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.
Then we can do imports like :
May be we should reopen this issue to solve this case (there is no need for node_modules, just the tsconfig.json file). |
@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. |
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. |
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 So, with paths in
And optional in
It should be possible to get production and development dependencies to install using If you choose to use this hack can it probably be smart to add a 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 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.. |
Also having this issue, resulting in sonar flagging this incorrectly as a code smell. |
I think this is a valid request since tslint is all about typescript and paths are a valid (and important) typescript setting. |
What if I have the
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? |
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 "no-implicit-dependencies": [true, ["@src", "@app", "~"]] |
@ifiokjr I was using I changed my alias to |
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.
Bug Report
TypeScript code being linted
src/a.ts
types/foo.d.ts
tsconfig.json
with
tslint.json
configuration:Actual behavior
Expected behavior
No error. I think if an import isn't resolved to something in
node_modules
this rule should ignore it.The text was updated successfully, but these errors were encountered: