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

chore(deps): update substrate dev package & types #1500

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

Imod7
Copy link
Contributor

@Imod7 Imod7 commented Oct 1, 2024

Description

Updated substrate/dev package to the latest version, v0.8.0, which introduces some ❗breaking changes❗:

  1. typescript-eslint dependency now requires Node.js engine in versions ^18.18.0 || >=20.0.0
  2. Option suppressImplicitAnyIndexErrors after Typescript 5.5 is disabled

🧑‍💻 Code Updates 🧑‍💻

By removing the option suppressImplicitAnyIndexErrors from the tsconfig files, the types in multiple parts of the code had to be updated. The changes introduced are based on the following logic/tests:

consoleOverride.ts

  • Use of keyof and set the return type based on what it is expected.

AccountsAssetsService.ts

  • Change based on the pattern from similar type PalletAssetsAssetBalance here
  • Same for the change in AccountsPoolAssetsService (similar to PalletPoolAssetsAssetBalance type)

AccountsStakingPayoutsService.ts

Updated prefs type to (ValidatorPrefs,Linkage<AccountId>) since this is the return type when tested it in Kusama with endpoint

http://127.0.0.1:8080/accounts/HJ1dGPxVr13KHGiCTGQfZjZMKGc8J52CsHEjcEXNMDyeGxf/staking-payouts?at=67498&unclaimedOnly=false

NodeTransactionPoolService.ts

Tested in Polkadot with endpoint

http://127.0.0.1:8080/node/transaction-pool?includeFee=true

PalletsDispatchablesService.ts

Tested with endpoint

http://127.0.0.1:8080/pallets/4/dispatchables/claim?metadata=true

in which dispatchableItemMetadata returns ['claim', f]

PalletsErrorsService.ts

Tested with endpoint

http://127.0.0.1:8080/pallets/11/errors/ChangePending

in which errorItemMetadata returns ['ChangePending', {is: ƒ, meta: Type(5)}] and this errorItemMetadata[1].meta returns ErrorMetadataLatest type/interface

PalletsEventsService.ts

Tested with endpoint

http://127.0.0.1:8080/pallets/34/events/CuratorUnassigned

in which eventItemMetadata returns ['CuratorUnassigned', {is: ƒ, meta: Type(5)}]

PalletsForeignAssetsService.ts

Tested with endpoint

http://127.0.0.1:8080/pallets/foreign-assets

in which foreignAssetData returns [{parents: '1', interior: {…}}]

*** The rest of the changes are mostly from the linter.

⚠️ Linter Warning ⚠️

Now when running yarn lint --fix we get this warning :

WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree.

You may find that it works just fine, or you may not.

SUPPORTED TYPESCRIPT VERSIONS: >=4.7.4 <5.6.0

YOUR TYPESCRIPT VERSION: 5.6.2

Please only submit bug reports when using the officially supported version.

but the linter runs correctly ✅
This is because I have not updated @typescript-eslint packages to the latest versions (but updated Typescript) since this introduces more breaking changes. They will be updated in the next substrate/dev release.

❗Changelog❗

In the next release of Sidecar :

  • Take into account/mention the nodejs version requirement
  • Update this part of the README.
  • The removal of the option suppressImplicitAnyIndexErrors does not need to be mentioned since the code was updated accordingly and this should not affect the API users.

Tests

  • yarn test
  • yarn test:latest-e2e-tests
  • yarn test:historical-e2e-tests

@Imod7 Imod7 requested a review from a team as a code owner October 1, 2024 19:11
@filvecchiato filvecchiato self-requested a review October 7, 2024 09:14
Copy link
Contributor

@filvecchiato filvecchiato left a comment

Choose a reason for hiding this comment

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

All looking good to me!

Ran it locally and fixes the build error! Nice work!

LGTM

Copy link
Contributor

@bee344 bee344 left a comment

Choose a reason for hiding this comment

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

LGTM

@Imod7 Imod7 merged commit cf2b58b into master Oct 7, 2024
17 checks passed
@Imod7 Imod7 deleted the domi-update-substrate-dev branch October 7, 2024 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants