fix: type generation for nested tuples

[sander2]

Addresses #5383 .

Without this, setImports fails for the input "[ITuple<[InterbtcPrimitivesCurrencyId, InterbtcPrimitivesCurrencyId]> | [InterbtcPrimitivesCurrencyId | { Token: any } | { ForeignAsset: any } | { LendToken: any } | { LpToken: any } | { StableLpToken: any } | string | Uint8Array, InterbtcPrimitivesCurrencyId | { Token: any } | { ForeignAsset: any } | { LendToken: any } | { LpToken: any } | { StableLpToken: any } | string | Uint8Array], AccountId32 | string | Uint8Array]".

I think this example can be simplified to "[ITuple<[u32, u32]>, u32]". Due to the < of the tuple, the wrong branch of setImports was taken. Swapping the order of the branches causes the right branch to be taken, but the regex-based splitting then fails -- the regex would return ITuple<[u32, u32 due to the nested []. As an aside, the regex also seemed to be incorrect for the [a|b, c|d] type: it would return ['a', 'b,c', 'd'], i.e. not splitting b,c. The splitAlternatives function is meant to address these issues.

When reviewing, please also note the change of type.startsWith('[') to type.includes('['). All in all I am not 100% confident of the correctness of this pr. The type generation no longer throws an error, but I don't know whether or not the output is correct, and whether the change works for all possible types.

[jacogr]

Thank you. I believe if ...

(a) it works on the problematic metadata,
(b) it yields the same output when running yarn build:metadata (so no changes)

... then we should be ok. Will dive through the logic.

(There are some small linting issues CI picked up, it is a bit pedantic...)

[sander2]

Regarding (a), this is the output it generates:

      bootstrapPersonalSupply: AugmentedQuery<ApiType, (arg: ITuple<[ITuple<[InterbtcPrimitivesCurrencyId, InterbtcPrimitivesCurrencyId]>, AccountId32]> | [ITuple<[InterbtcPrimitivesCurrencyId, InterbtcPrimitivesCurrencyId]> | [InterbtcPrimitivesCurrencyId | { Token: any } | { ForeignAsset: any } | { LendToken: any } | { LpToken: any } | { StableLpToken: any } | string | Uint8Array, InterbtcPrimitivesCurrencyId | { Token: any } | { ForeignAsset: any } | { LendToken: any } | { LpToken: any } | { StableLpToken: any } | string | Uint8Array], AccountId32 | string | Uint8Array]) => Observable<ITuple<[u128, u128]>>, [ITuple<[ITuple<[InterbtcPrimitivesCurrencyId, InterbtcPrimitivesCurrencyId]>, AccountId32]>]> & QueryableStorageEntry<ApiType, [ITuple<[ITuple<[InterbtcPrimitivesCurrencyId, InterbtcPrimitivesCurrencyId]>, AccountId32]>]>;

Focusing on the arg type, and substituting the expanded currencyId for brevity, the generated type is:

ITuple<[ITuple<[InterbtcPrimitivesCurrencyId, InterbtcPrimitivesCurrencyId]>, AccountId32]> | 
    [
        ITuple<[InterbtcPrimitivesCurrencyId, InterbtcPrimitivesCurrencyId]> | 
            [EXPANDED_CURRENCY_ID, EXPANDED_CURRENCY_ID], 
        AccountId32 | string | Uint8Array
    ]

So we have...

• Tuple(Tuple(CurrencyId), AccountId)
• Array(Tuple(CurrencyId), ExpandedAccountId)
• Array(Array(ExpandedCurrencyId), ExpandedAccountId)

But we don't have:

• Tuple(Array(ExpandedCurrencyId), AccountId)
• Tuple(Array(CurrencyId), AccountId)
• Tuple(Array(ExpandedCurrencyId), ExpandedAccountId)
• Tuple(Array(CurrencyId), ExpandedAccountId)
• Tuple(Tuple(ExpandedCurrencyId), ExpandedAccountId)
• Tuple(Tuple(CurrencyId), ExpandedAccountId)
• Array(Tuple(ExpandedCurrencyId), AccountId)
• Array(Tuple(CurrencyId), AccountId)
• Array(Array(ExpandedCurrencyId), AccountId)
• Array(Array(CurrencyId), AccountId)
• Tuple(Tuple(CurrencyId), AccountId)
• Tuple(Tuple(ExpandedCurrencyId), AccountId)
• Array(Tuple(CurrencyId), ExpandedAccountId)
• Array(Tuple(ExpandedCurrencyId), ExpandedAccountId)
• Array(Array(CurrencyId), ExpandedAccountId)

... That's a lot of alternatives.. Would these be expected to be present in the generated type?

[jacogr]

Where the alternatives help and are critical... Think about a simple enum type, let's define it as

enum Sample {
  Alice(AccountId32),
  Bob(u32)
}

When it generates, it should look more-or-less like Sample | { Alice: any } | { Bob: any } | Uint8Array (I will get to my pet-peeve any right at the end).

So basically when using the argument, you can pass either -

• an actual Sample instance (crated via createType)
• an object of the form { Bob: 123 } which the API then will convert via createType (key=Bob, value=123)
• an already-encoded SCALE Uint8Array (this would also go via createType back into a type representation)

The second option is what makes things easier to use since the API will convert to the correct type. Without having that the user always has to manually do the createType magic when using these. With that signature the user can pass through stuff that the feel comfortable with, i.e. JS types, not API types. The API does the type conversion in these case without the user having to do anything extra.

(On to any: it should in this case be something like { Bob: number | BN | Bbigint | string | Uint8Array } but the typegen doesn't go that deep. At some point it really should, but will probably make the signatures even more unwieldly, but with the benefit of not having any anywhere)

[jacogr]

So summarize my answer above - it would be great if the alternatives are supplied, i.e. users can have a mixture of stuff-already-created and pure JS objects for all. Alternatives are all to make it easier to use with TS complaining about type mismatches. (Since the API does convert from anything reasonable to the correct type)

[sander2]

So summarize my answer above - it would be great if the alternatives are supplied, i.e. users can have a mixture of stuff-already-created and pure JS objects for all. Alternatives are all to make it easier to use with TS complaining about type mismatches. (Since the API does convert from anything reasonable to the correct type)

Right, that makes sense. I just checked that this PR creates the same output as the unmodified code, when run on a metadata that does not contain nested tuples. Given that the output for nested tuples seems to be correct (even though incomplete), and that the failing type generation is blocking us, would you be open to merging this PR and opening an issue for generating the alternatives?

[jacogr]

Happy to merge as-is since it does move use into a much better spot, although it is incomplete.

[jacogr]

Happy with the output as-is. (Also tested the changes with yarn build:metadata and it is equivalent)

Just needs some small adjustments to make CI happy.

[sander2]

Thanks! I pushed a fix for the formatting and made an issue for the missing alternatives: #5399

[jacogr]

Thank you.

[polkadot-js-bot]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.