Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Internal update: clean dependencies #4149

Merged
merged 19 commits into from
Jul 24, 2021
Merged

Internal update: clean dependencies #4149

merged 19 commits into from
Jul 24, 2021

Conversation

cds-amal
Copy link
Member

@cds-amal cds-amal commented Jun 29, 2021

This PR removes unused dependencies from project workspaces .

Workspace Flagged dependencies Action
contract-schema crypto-js removed
source-fetcher @types/node removed
reporters node-emoji removed
hdwallet-provider any-promise removed
bindings removed
preserve-to-filecoin node-fetch removed
require web3 removed
compile-common mocha move to devDep
ts-node move to devDep
debug-utils highlight.js removed
resolver supports-color removed
db pascal-case removed
migrate web3 removed
debugger redux-cli-logger removed
web3-eth-abi removed
core @truffle/contract-sources removed
app-module-path move to devDep
node-ipc removed

@cds-amal cds-amal marked this pull request as draft June 29, 2021 18:12
@cds-amal cds-amal marked this pull request as ready for review June 29, 2021 19:24
@cds-amal cds-amal marked this pull request as draft June 29, 2021 20:57
@cds-amal cds-amal force-pushed the clean-dependency branch 2 times, most recently from ff8c1d9 to 9fdcd75 Compare June 30, 2021 20:36
@cds-amal cds-amal marked this pull request as ready for review June 30, 2021 22:45
@cds-amal cds-amal requested a review from gnidan July 1, 2021 21:23
gnidan
gnidan previously requested changes Jul 1, 2021
Copy link
Contributor

@gnidan gnidan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed some errors here

packages/plugins/package.json Outdated Show resolved Hide resolved
@@ -26,7 +26,6 @@
},
"types": "dist/index.d.ts",
"dependencies": {
"@types/node": "^15.0.1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This package's tsconfig references this. Make sure you can remove it there before saying it's not a dependency.

Although it probably should be a devDep.

Copy link
Member Author

@cds-amal cds-amal Jul 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what the intention was here. According to the docs

If typeRoots is specified, only packages under typeRoots will be included.

This config implies that node is excluded because it wouldn't appear in typeRoots list of paths

    "typeRoots": [
      "./typings",
      "../../**/node_modules/@types/debug"
    ],
    "types": ["node"]

Have you used any tools to verify this behavior? It would be nice to have tests. tsd seems promising ¯_(ツ)_/¯

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just try taking it out and see if it breaks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will people be trying to install this package into their projects and building with TS? :) This is the same kind of thing as what we were talking about earlier with hdwallet-provider, right? I don't know what the right answer is here by the way.

packages/debug-utils/package.json Show resolved Hide resolved
packages/core/package.json Show resolved Hide resolved
packages/core/package.json Outdated Show resolved Hide resolved
@gnidan gnidan dismissed their stale review July 5, 2021 22:00

Dismissing review so as not to impede things while I'm OOO

@eggplantzzz
Copy link
Contributor

I like it!

@eggplantzzz eggplantzzz self-requested a review July 8, 2021 20:55
app-module-path is used for testing, move it to dev dependency
  - [x] app-module-path
  - `yarn remove @types/node` updated yarn.lock
  - `yarn remove any-promise bindings`
  - auto update yarn.lock
  - `yarn remove highlight.js`, updated yarn.lock
  - `yarn remove supports-color`
  - `yarn remove redux-cli-logger web3-eth-abi`, updated yarn.lock
  - `yarn remove @truffle/contracts-sources`
  - `yarn remove @truffle/preserve-fs`
  - `yarn remove @truffle/preserve-to-buckets`
  - `yarn remove @truffle/preserve-to-filecoin`
  - `yarn remove @truffle/preserve-to-ipfs`
  - `yarn remove app-module-path`
  - @truffle/contract-sources
  - node-ipc
Explicitly specify definitions with `types`
@gnidan gnidan merged commit 7b17c78 into develop Jul 24, 2021
@gnidan gnidan deleted the clean-dependency branch July 24, 2021 01:52
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.

None yet

3 participants