Skip to content
This repository has been archived by the owner on Sep 30, 2021. It is now read-only.

fix: update polkadot deps, and patch decorateConstants #433

Merged
merged 1 commit into from
May 3, 2021

Conversation

TarikGul
Copy link
Member

@TarikGul TarikGul commented May 3, 2021

This updates polkadot/api dependency and fixes a bug that comes with it.

decorateConstants from @polkadot/metadata now requires a version as well.

Copy link
Contributor

@maciejhirsz maciejhirsz left a comment

Choose a reason for hiding this comment

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

LGTM

createMetadata(registry, metadataRpc).asLatest
);
const metadata = createMetadata(registry, metadataRpc);
return decorateConstants(registry, metadata.asLatest, metadata.version);
Copy link
Contributor

Choose a reason for hiding this comment

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

Mildly confused about API here, but I reckon it's upstream. I'd have expected .version to also require .asLatest here, otherwise how do you know which version info are you grabbing? 🤷‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

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

I think under the hood polkadot-js metadata is always the shape of the latest metadata. And assuming stuff is only added to metadata its ok because you can just have place holders in new fields when you are using it to represent old metadata. Not sure if thats exactly how it works but from reading the code thats about my understanding

Copy link
Member Author

Choose a reason for hiding this comment

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

^^ Exactly this. Also I think the .asLatest method is misleading, per the docs it Returns the wrapped values as a latest version object.

@TarikGul TarikGul merged commit fd8689a into master May 3, 2021
@TarikGul TarikGul deleted the tarik-update-polkadot-fix-decorate branch May 3, 2021 16:24
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