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

new rule 'no-implicit-dependencies' #3343

Merged
merged 3 commits into from
Oct 20, 2017
Merged

new rule 'no-implicit-dependencies' #3343

merged 3 commits into from
Oct 20, 2017

Conversation

ajafff
Copy link
Contributor

@ajafff ajafff commented Oct 18, 2017

PR checklist

Overview of change:

[new-rule] no-implicit-dependencies
Fixes: #3235

Is there anything you'd like reviewers to focus on?

CHANGELOG.md entry:

@adidahiya
Copy link
Contributor

adidahiya commented Oct 19, 2017

@ajafff is it possible to decouple this from the TS 2.6 upgrade? Is ts. isExternalModuleNameRelative available in TS 2.5 as a private util we can cast? I would really love to get this rule released in TSLint 5.8 without waiting for the TS release.

@ajafff
Copy link
Contributor Author

ajafff commented Oct 19, 2017

@adidahiya sure, I just need to add an assertion. We already use it in no-submodule-imports.

@ajafff
Copy link
Contributor Author

ajafff commented Oct 19, 2017

@adidahiya it now works with ts@2.5, PTAL

@adidahiya
Copy link
Contributor

thanks!

@adidahiya adidahiya merged commit 79165d5 into palantir:master Oct 20, 2017
@balassy
Copy link

balassy commented Oct 21, 2017

I'm afraid this new rule triggers a false alarm when someone imports an interface from a type information package, for example in package.json:

"devDependencies": {
    "@types/aws-lambda": "0.0.17"
}

In a .ts code file:

import { APIGatewayEvent } from 'aws-lambda'; 

Thank you for creating and maintaining this package!

@adidahiya
Copy link
Contributor

@balassy are you not also using the package aws-lambda in that project? Seems strange to only declare its type definitions as a dependency. If this use case is prevalent then maybe we can add a whitelist of package names as part of the rule config.

@alokito
Copy link

alokito commented Oct 22, 2017

@adidahiya I am facing a similar but not directly relevant problem. When writing a AWS lambda function, the aws-sdk dependency is provided by the lambda container. It is not necessary to add it as an explicit dependency of the function. http://docs.aws.amazon.com/lambda/latest/dg/current-supported-versions.html. Ideally we could disable the implicit package check just for this case.

Nevermind, I see the solution is to include devDependencies, since I have aws-sdk listed there.

@ajafff ajafff deleted the no-implicit-dependencies branch October 22, 2017 13:20
@balassy
Copy link

balassy commented Oct 22, 2017

I have to admit this is a very special situation: my project contains AWS Lambda functions implemented in TypeScript, which are basically callback functions, that will be called by AWS. They don't have any dependencies, so I don't have any aws-lambda package in my project. However, I have the @types/aws-lambda imported as a devDependency, because it provides type information for the callback function signature as it was defined by AWS.

So basically I am creating a plugin, and what I import is only the input-output definition of the interface defined by the hosting environment.

(Actually the @types/aws-lambda package has no relation to the aws-lambda package, they are completely independent, but that's another story.)

@ajafff
Copy link
Contributor Author

ajafff commented Oct 22, 2017

@balassy I would expect the @types/aws-lambda package to contain global types. That's how other declaration packages like @types/jquery work for example. They assume that the thing you are referencing is just there, no import required.

Can you to please report that in the DefinitelyTyped repo (and send a PR)?


It should be possible to detect type-only imports without the type checker. They will be elided during compilation and can be excluded from the check.
That's quite a difficult task and I wonder if it's worth it. I'll open a separate issue to track this and await more feedback. -> #3375

@balassy
Copy link

balassy commented Oct 22, 2017

I've checked the @types/aws-lambda package, and it contains global types:
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/aws-lambda/index.d.ts

Unfortunately I was not able to figure out how to let the TypeScript compiler aware of the type definitions in that package instead of using import.

Thanks, Klaus!

@timocov
Copy link
Contributor

timocov commented Oct 25, 2017

We have the project structure like this:

root/
    package.json (project's root package.json with all deps)
    component1/
        package.json
        component1.ts
        ...
    component2/
        package.json
        component2.ts
        ...

(deep of tree and tree structure maybe different - it is example to show that the component may has self package.json)
package.json of a component just has a private: true, main and typings section to have short import like this import { Component1Class } from 'component1'; (not from 'component1/component1') and seems that this rule does not handle this case and trigger errors on each component.

HyphnKnight pushed a commit to HyphnKnight/tslint that referenced this pull request Apr 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New rule: no-implicit-package-dependencies
5 participants