Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
paul-marechal opened this issue Feb 4, 2020 · 6 comments
Open

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

paul-marechal opened this issue Feb 4, 2020 · 6 comments
Assignees
Labels
dependencies pull requests that update a dependency file linting issues related to linting

Comments

@paul-marechal
Copy link
Member

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 paul-marechal added linting issues related to linting dependencies pull requests that update a dependency file labels Feb 4, 2020
@paul-marechal
Copy link
Member Author

cc @vince-fugnitto

@paul-marechal
Copy link
Member Author

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

@akosyakov
Copy link
Member

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

@paul-marechal
Copy link
Member Author

paul-marechal commented Feb 6, 2020

We could use peer dependencies, it should mitigate duplication.

@akosyakov
Copy link
Member

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

"prepare:hoisting": "theia check:hoisted -s",
. So maybe we just check it more regularly or fail a build if there are different versions in some packages.

@paul-marechal paul-marechal self-assigned this Feb 11, 2020
@paul-marechal
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies pull requests that update a dependency file linting issues related to linting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants