vechain / vechain-sdk-js

The official JavaScript SDK for VeChain.
24 stars 9 forks source link

1450 thor client fix circular dependencies #1514

Closed lucanicoladebiasi closed 4 days ago

lucanicoladebiasi commented 1 week ago

Description

The code refactors the ThorClient to avoid circular dependencies among the modules composing the Thor client.

Fixes #1450

Type of change

Please delete options that are not relevant.

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Test Configuration:

Checklist:

github-actions[bot] commented 1 week ago

Test Coverage

Summary

Lines Statements Branches Functions
Coverage: 99%
98.99% (4345/4389) 97.53% (1387/1422) 98.9% (901/911)
Title Tests Skipped Failures Errors Time
core 827 0 :zzz: 0 :x: 0 :fire: 2m 19s :stopwatch:
network 719 0 :zzz: 0 :x: 0 :fire: 4m 59s :stopwatch:
errors 42 0 :zzz: 0 :x: 0 :fire: 16.866s :stopwatch:
logging 3 0 :zzz: 0 :x: 0 :fire: 18.588s :stopwatch:
hardhat-plugin 19 0 :zzz: 0 :x: 0 :fire: 1m 0s :stopwatch:
aws-kms-adapter 23 0 :zzz: 0 :x: 0 :fire: 1m 8s :stopwatch:
ethers-adapter 5 0 :zzz: 0 :x: 0 :fire: 1m 11s :stopwatch:
rpc-proxy 37 0 :zzz: 0 :x: 0 :fire: 1m 2s :stopwatch:
freemanzMrojo commented 1 week ago

Hi @lucanicoladebiasi , I think we should modify the script at package.json that counts the number of circular dependencies to validate that indeed they are going down (as of now it counts up to 11).

The script is check:circular-dependencies at network's package.json 👍

lucanicoladebiasi commented 1 week ago

Hi @lucanicoladebiasi , I think we should modify the script at package.json that counts the number of circular dependencies to validate that indeed they are going down (as of now it counts up to 11).

The script is check:circular-dependencies at network's package.json 👍

I modified the script as

 "check:circular-dependencies": "npx madge --json --circular --extensions ts src | jq '. | length' | awk '{if($1 > 0) exit 1}'"

and it doesn't complain anymore.

freemanzMrojo commented 1 week ago

Hi @lucanicoladebiasi , I think we should modify the script at package.json that counts the number of circular dependencies to validate that indeed they are going down (as of now it counts up to 11). The script is check:circular-dependencies at network's package.json 👍

I modified the script as

 "check:circular-dependencies": "npx madge --json --circular --extensions ts src | jq '. | length' | awk '{if($1 > 0) exit 1}'"

and it doesn't complain anymore.

Nice! If we have 0 we could just run npx madge --circular --extensions ts src without the rest, like we do in package.jsonat core 👍