-
Notifications
You must be signed in to change notification settings - Fork 12
Introduce the use of path aliases in @dendreth/relay
#365
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
b375012 to
ee234b4
Compare
@dendreth/relay
kkirkov
requested changes
Jul 9, 2024
smanilov
previously approved these changes
Jul 9, 2024
There was a problem hiding this 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
previously approved these changes
Jul 9, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work
ee234b4 to
0496145
Compare
9db398d to
ca56d92
Compare
ca56d92 to
e38ccbc
Compare
kkirkov
approved these changes
Jul 29, 2024
f6e4a68 to
88093b8
Compare
Dimo99
approved these changes
Jul 31, 2024
…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
```
88093b8 to
7152557
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:buildandyarn 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-pathsconfig(@dendreth/solidity): Update
hardhat.config.tsto usetsconfig-pathsHardhat does not natively support TypeScript path aliases out of the box.
However, we can use
tsconfig-pathspackage to achieve this.With this we fix errors like this:
build(check:build): Refactor script to run
tscwith--buildflagThe
--buildoption can be seen as a build orchestrator that find referencedprojects, 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
compilerOptionsupdates:Add
referencesto therelayworkspaceMake sure all ts files are included
config(tsconfig.json): Make sure all
jsonfiles are includedThis change resolves problems like this:
build(yarn): Unplug
@lodestar/types@1.17.0config(relay): Add
@lodestar/typesin paths.This change fixes following problems:
config(tsconfig.json): Exclude
distdirThis fixes problems like:
build(build-plonky-2): Refactor script to use
--buildflagconfig(@dendreth/balance-verification): Add project references
The
@dendreth/balance-verificationworkspace 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-2ends in:build(@dendreth/balance-verification): Add
@types/noderefactor(relay): Remove unused imports