Skip to content

Conversation

@EmilIvanichkovv
Copy link
Contributor

@EmilIvanichkovv EmilIvanichkovv commented Jul 8, 2024

This PR introduces the use of path aliases and sets up the project to support them.

While this change might seem like a simple refactor, it uncovered numerous issues with the overall configuration of the monorepo. Debugging and resolving these problems involved running our build scripts: yarn check:build and yarn build-plonky-2.

Here is a list of problems encountered and the fixes implemented:

  • refactor(relay): Use @ for paths within /relay

  • build(@dendreth/solidity): Install tsconfig-paths

  • config(@dendreth/solidity): Update hardhat.config.ts to use tsconfig-paths

    Hardhat does not natively support TypeScript path aliases out of the box.
    However, we can use tsconfig-paths package to achieve this.

    With this we fix errors like this:

    An unexpected error occurred:
    
    Error: Your application tried to access @, but it isn't declared in your dependencies;
    this makes the require call ambiguous and unsound.
    
    Required package: @ (via "@/constants/network_config.json")
    Required by: <path>/DendrETH/relay/utils/
    
  • build(check:build): Refactor script to run tsc with --build flag

    The --build option can be seen as a build orchestrator that find referenced
    projects, check if they are up-to-date, and build out-of-date projects in the
    correct order.

    The path aliases changes we want to introduce, forces the need of referenced
    projects.

  • config(tsconfig.json): Streamline the configuration to work with path aliases

    compilerOptions updates:

    • "composite": true
    • "rootDir": "./"
    • "declaration": true
    • "declarationMap": true

    Add references to the relay workspace
    Make sure all ts files are included

  • config(tsconfig.json): Make sure all json files are included

    This change resolves problems like this:

    error TS6307: File '<path>/DendrETH/beacon-light-client/plonky2/input_fetchers/balance_verification/abi/lido_accounting_oracle_abi.json' is not listed within the file list of project '<path>DendrETH/relay/tsconfig.json'. Projects must list all files or use an 'include' pattern.
    
    7 import accountingOracleAbi from '../../abi/lido_accounting_oracle_abi.json';
    
  • build(yarn): Unplug @lodestar/types@1.17.0

  • config(relay): Add @lodestar/types in paths.

    This change fixes following problems:

    ```
    relay/implementations/beacon-api.ts:543:9 - error TS2742: The inferred type of 'getBeaconBlock' cannot be named without a reference to '../../.yarn/unplugged/@lodestar-types-npm-1.17.0-1607a25762/node_modules/@lodestar/types/lib/u tils/executionAddress'. This is likely not portable. A type annotation is necessary.
    
    543   async getBeaconBlock(slot: bigint) {
                ~~~~~~~~~~~~~~
    
    relay/implementations/beacon-api.ts:574:9 - error TS2742: The inferred type of 'getBeaconState' cannot be named without a reference to '../../.yarn/unplugged/@lodestar-types-npm-1.17.0-1607a25762/node_modules/@lodestar/types/lib/utils/executionAddress'. This is likely not portable. A type annotation is necessary.
    
    574   async getBeaconState(slot: bigint) {
                ~~~~~~~~~~~~~~
    
    Found 2 errors.
    ```
    
  • config(tsconfig.json): Exclude dist dir

    This fixes problems like:

    error TS5055: Cannot write file <file-path> because it would overwrite input file.
    
  • build(build-plonky-2): Refactor script to use --build flag

  • config(@dendreth/balance-verification): Add project references

    The @dendreth/balance-verification workspace rely on files from @dendreth/relay.
    That is why we need to add this reference.

    With this change following proble was resolved:
    Running yarn build-plonky-2 ends in:

    ```
    error TS2688: Cannot find type definition file for 'node'.
      The file is in the program because:
        Entry point of type library 'node' specified in compilerOptions
    
    relay/implementations/beacon-api.ts:543:9 - error TS2742: The inferred type of 'getBeaconBlock' cannot be named without a reference to '../../.yarn/unplugged/@lodestar-types-npm-1.17.0-1607a25762/node_modules/@lodestar/types/lib/utils/executionAddress'. This is likely not portable. A type annotation is necessary.
    
    543   async getBeaconBlock(slot: bigint) {
                ~~~~~~~~~~~~~~
    
    relay/implementations/beacon-api.ts:574:9 - error TS2742: The inferred type of 'getBeaconState' cannot be named without a reference to '../../.yarn/unplugged/@lodestar-types-npm-1.17.0-1607a25762/node_modules/@lodestar/types/lib/utils/executionAddress'. This is likely not portable. A type annotation is necessary.
    
    574   async getBeaconState(slot: bigint) {
                ~~~~~~~~~~~~~~
    
    Found 3 errors in the same file, starting at: relay/implementations/beacon-api.ts:543
    ```
    
  • build(@dendreth/balance-verification): Add @types/node

  • refactor(relay): Remove unused imports

@EmilIvanichkovv EmilIvanichkovv force-pushed the fix_paths-emil2 branch 2 times, most recently from b375012 to ee234b4 Compare July 8, 2024 17:08
@EmilIvanichkovv EmilIvanichkovv changed the title Fix paths.. maybe Introduce the use of path aliases in @dendreth/relay Jul 8, 2024
@EmilIvanichkovv EmilIvanichkovv self-assigned this Jul 8, 2024
@EmilIvanichkovv EmilIvanichkovv added the enhancement New feature or request label Jul 8, 2024
smanilov
smanilov previously approved these changes Jul 9, 2024
Copy link
Contributor

@smanilov smanilov left a comment

Choose a reason for hiding this comment

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

Impressive change! Glad to see path aliases working.

kkirkov
kkirkov previously approved these changes Jul 9, 2024
Copy link
Contributor

@kkirkov kkirkov left a comment

Choose a reason for hiding this comment

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

Good work

@EmilIvanichkovv EmilIvanichkovv dismissed stale reviews from kkirkov and smanilov via 0496145 July 9, 2024 14:42
@EmilIvanichkovv EmilIvanichkovv force-pushed the fix_paths-emil2 branch 7 times, most recently from 9db398d to ca56d92 Compare July 11, 2024 14:51
@EmilIvanichkovv EmilIvanichkovv force-pushed the fix_paths-emil2 branch 2 times, most recently from f6e4a68 to 88093b8 Compare July 29, 2024 13:03
kkirkov and others added 13 commits September 5, 2024 13:23
…ig-paths`

Hardhat does not natively support TypeScript path aliases out of the box.
However, we can use `tsconfig-paths` package to achieve this.

With this we fix errors like this:
```
An unexpected error occurred:

Error: Your application tried to access @, but it isn't declared in your dependencies;
this makes the require call ambiguous and unsound.

Required package: @ (via "@/constants/network_config.json")
Required by: <path>/DendrETH/relay/utils/
```
The --build option can be seen as a build orchestrator that finds referenced
projects, checks if they are up-to-date, and builds out-of-date projects in the
correct order.

The path aliases changes we want to introduce, forces the need of
referenced projects.
… aliases

`compilerOptions` updates:
- "composite": true
- "rootDir": "./"
- "declaration": true
- "declarationMap": true

Add `references` to the `relay` workspace
Make sure all ts files are included
This change resolves problems like this:
```
error TS6307: File '<path>/DendrETH/beacon-light-client/plonky2/input_fetchers/balance_verification/abi/lido_accounting_oracle_abi.json' is not listed within the file list of project '<path>DendrETH/relay/tsconfig.json'. Projects must list all files or use an 'include' pattern.

7 import accountingOracleAbi from '../../abi/lido_accounting_oracle_abi.json';
```
This change fixes following problems:
```
relay/implementations/beacon-api.ts:543:9 - error TS2742: The inferred type of 'getBeaconBlock' cannot be named without a reference to '../../.yarn/unplugged/@lodestar-types-npm-1.17.0-1607a25762/node_modules/@lodestar/types/lib/utils/executionAddress'. This is likely not portable. A type annotation is necessary.

543   async getBeaconBlock(slot: bigint) {
            ~~~~~~~~~~~~~~

relay/implementations/beacon-api.ts:574:9 - error TS2742: The inferred type of 'getBeaconState' cannot be named without a reference to '../../.yarn/unplugged/@lodestar-types-npm-1.17.0-1607a25762/node_modules/@lodestar/types/lib/utils/executionAddress'. This is likely not portable. A type annotation is necessary.

574   async getBeaconState(slot: bigint) {
            ~~~~~~~~~~~~~~

Found 2 errors.
```
This fixes problems like:
```
error TS5055: Cannot write file <file-path> because it would overwrite input file.
```
The `@dendreth/balance-verification` workspace rely on files from `@dendreth/relay`.
That is why we need to add this reference.

With this change following problem was resolved:
Running `yarn build-plonky-2` ends in:
```
error TS2688: Cannot find type definition file for 'node'.
  The file is in the program because:
    Entry point of type library 'node' specified in compilerOptions

relay/implementations/beacon-api.ts:543:9 - error TS2742: The inferred type of 'getBeaconBlock' cannot be named without a reference to '../../.yarn/unplugged/@lodestar-types-npm-1.17.0-1607a25762/node_modules/@lodestar/types/lib/utils/executionAddress'. This is likely not portable. A type annotation is necessary.

543   async getBeaconBlock(slot: bigint) {
            ~~~~~~~~~~~~~~

relay/implementations/beacon-api.ts:574:9 - error TS2742: The inferred type of 'getBeaconState' cannot be named without a reference to '../../.yarn/unplugged/@lodestar-types-npm-1.17.0-1607a25762/node_modules/@lodestar/types/lib/utils/executionAddress'. This is likely not portable. A type annotation is necessary.

574   async getBeaconState(slot: bigint) {
            ~~~~~~~~~~~~~~

Found 3 errors in the same file, starting at: relay/implementations/beacon-api.ts:543
```
@Dimo99 Dimo99 enabled auto-merge (rebase) September 5, 2024 10:23
@Dimo99 Dimo99 merged commit a42d25b into main Sep 5, 2024
@Dimo99 Dimo99 deleted the fix_paths-emil2 branch September 5, 2024 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants