eclipse-theia / theia

Eclipse Theia is a cloud & desktop IDE framework implemented in TypeScript.
http://theia-ide.org
Eclipse Public License 2.0
20.11k stars 2.5k forks source link

eslint: bring back no-implicit-dependencies rule #7079

Open paul-marechal opened 4 years ago

paul-marechal commented 4 years ago

Description

When I migrated the rules from tslint to eslint, I did not find a no-implicit-dependency rule that could be configured the same way we used to have it on tslint.

Current rules I found to prevent implicit dependencies:

Old rule used to whitelist the following packages:

@phosphor/algorithm @phosphor/commands @phosphor/coreutils @phosphor/domutils @phosphor/dragdrop @phosphor/messaging @phosphor/properties @phosphor/signaling @phosphor/virtualdom @phosphor/widgets chai chai-string electron express fs-extra inversify jsdom lodash.debounce lodash.throttle monaco-languageclient native-keymap nsfw react react-dom react-virtualized sinon temp typescript uuid vscode-jsonrpc vscode-languageserver vscode-languageserver-protocol vscode-languageserver-types vscode-uri vscode-ws-jsonrpc yargs

paul-marechal commented 4 years ago

cc @vince-fugnitto

paul-marechal commented 4 years ago

Worst case scenario we could try to implement our own rule within the repo.

akosyakov commented 4 years ago

If we remove whitelisting how much duplication it causes, can we somehow catch duplicate versions?

paul-marechal commented 4 years ago

We could use peer dependencies, it should mitigate duplication.

akosyakov commented 4 years ago

We could use peer dependencies, it should mitigate duplication.

But it will break the end product if a version does not match with core? I meant to check that we have exactly the same versions across the repo.

Actually @kittaakos built something like that https://github.com/eclipse-theia/theia/blob/d54c2f0d7307ef504e9c46e490f4ebd0ac06ef51/package.json#L48. So maybe we just check it more regularly or fail a build if there are different versions in some packages.

paul-marechal commented 4 years ago

Fixing this issue will also fix bundling issues for electron we are having on theia-apps.

@akosyakov I will do like you mentioned and make all implicit dependencies explicit, with a script to make sure the same range is used across the repo.