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

New rule: no-implicit-package-dependencies #3235

Closed
adidahiya opened this issue Sep 18, 2017 · 4 comments · Fixed by #3343
Closed

New rule: no-implicit-package-dependencies #3235

adidahiya opened this issue Sep 18, 2017 · 4 comments · Fixed by #3343

Comments

@adidahiya
Copy link
Contributor

adidahiya commented Sep 18, 2017

I'd like a new rule to raise a lint error when I import a module that is not explicitly declared in the dependencies block of package.json. Like this eslint plugin: https://github.com/lennym/eslint-plugin-implicit-dependencies.

Motivation: this problem is an unresolved question when using dependency hoisting in monorepos: https://github.com/yarnpkg/rfcs/blob/master/implemented/0000-workspaces-install-phase-1.md#unresolved-questions

We should consider adding a linter configuration option to specify the location of package.json similar to --project (maybe --manifest). Alternatively, we could walk up the folder tree until we find a package.json for each linted file/folder (potentially expensive).

@ajafff
Copy link
Contributor

ajafff commented Sep 19, 2017

👎 for --manifest
Currently there is no way for rules to access anything but their config. We should not introduce some special behavior just for one rule.

Specifiying the path in tslint.json as rule option won't work, because we cannot resolve relative paths without knowing where the config comes from.

👍 for using the findup approach. Although there might be a performance hit, we only have to do this once for every file that contains imports from node_modules.
Caching comes to mind, but that's not really possible as we don't know when to clear the cache. Ideally the host system caches these recurring file system operations and we don't need to worry.

@adidahiya
Copy link
Contributor Author

Fair points, findup seems fine to me.

@ajafff
Copy link
Contributor

ajafff commented Sep 20, 2017

Do we need to handle path mappings specified in tsconfig.json?
https://www.typescriptlang.org/docs/handbook/module-resolution.html#path-mapping

@ajafff
Copy link
Contributor

ajafff commented Sep 30, 2017

The rule should have a "dev" option to also look at devDependencies.
That option could then be enabled for tests.

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

Successfully merging a pull request may close this issue.

2 participants