fix: Generate custom type definitions

[daniel-savu]

A first attempt at fixing #4213. I wasn't sure where exactly to output lookup types, so I modified polkadot-types-from-def as a proof-of-concept:

• the metadata file is input to the script using the same logic from polkadot-types-from-chain, so CLI logic is refactored
• generates the definitions.ts file from a v14 metadata file instead of using manual definitions. This means any manual type definitions would be overwritten
• leaves the type-generation logic (which takes definitions.ts) unchanged, and produces decorated types in default/types.ts as before

The resulting definitions.ts file looks like this:

// Auto-generated via `yarn polkadot-types-from-defs`, do not edit
/* eslint-disable */

/* eslint-disable sort-keys */

export default {
  rpc: {},
  types: {
    /**
     * Lookup3: frame_system::AccountInfo<Index, pallet_balances::AccountData<Balance>>
     **/
    FrameSystemAccountInfo: {
      nonce: 'u32',
      consumers: 'u32',
      providers: 'u32',
      sufficients: 'u32',
      data: 'PalletBalancesAccountData'
    },
    /**
     * Lookup5: pallet_balances::AccountData<Balance>
     **/
    PalletBalancesAccountData: {
      free: 'u128',
      reserved: 'u128',
      miscFrozen: 'u128',
      feeFrozen: 'u128'
    },
...

polkadot-types-from-chain is not changed because it is now able to find the types from interfaces/default/types.ts to resolve imports in the api interface files.

There is one error left in this approach, which is that in interfaces/default/types.ts pallet-specific types fail to be resolved for custom pallets. For instance, OrmlTokensModuleCall, which comes from an external pallet, is successfully resolved. I'm not very knowledgeable about Substrate but maybe this has to do with our pallets or runtime needing to make these types public somehow?

[daniel-savu]

I removed this import because the definitions are type-asserted on usage anyway and the import doesn't resolve when the file is outside the polkadot-js/api package: https://github.com/polkadot-js/api/blob/master/packages/typegen/src/fromDefs.ts#L29

[jacogr]

Thank you.

So the missing imports needs to come from @polkadot/types/lookup.

So this means that this file generated should actually augment to that location (to share imports) via declare module at the top of the file. (for old-style definitions the augmented path is @polkadot/types/types/registry)

So where you see the above issues, if you just change to declare module '@polkadot/types/lookup' { at the file-top, the types should be available. (This uses the templates/lookup hbs template)

[daniel-savu]

So where you see the above issues, if you just change to declare module '@polkadot/types/lookup' { at the file-top, the types should be available. (This uses the templates/lookup hbs template)

This PR generates the same type definitions in two files:

• one with the templates/lookup/types.hbs(the one you suggested, which has declare module '@polkadot/types/lookup' { at the top)
• one with the /templates/tsDef/moduleType.hbs template

Both of these files have the same import errors seen in the screenshot above.

So the missing imports needs to come from @polkadot/types/lookup.

I just checked augment-api-query.ts and it imports some types from '@interlay/interbtc/interfaces/default' (generated from /templates/tsDef/moduleType.hbs) and others from '@polkadot/types/lookup' (generated from templates/lookup/types.hbs) -> these two import lines can be manually combined into a single import from '@polkadot/types/lookup' and it still works.

The errors are present in the types file itself, not in the files importing generated types.

[jacogr]

Ok, it seems the *Call types re not being generated since they alias through to Call. The same should actually happen on the *StandaloneCall, so there is a mismatch. (However, have been thinking that even these aliased types should be generated, took a brief look 2 weeks ago, but didn't get to a resolution there)

The *Call and *Event ones are not being generated as types. This is where they get dropped -

https://github.com/polkadot-js/api/blob/master/packages/typegen/src/generate/lookup.ts#L145-L146

You can also see why the InterBtc ones are not dropped, and thus creating the above issue - node_template also won't work as it stands. For an alignment fix, change path[0].toString().split('_')[1] === 'runtime' to path[0].toString().includes('_runtime') which will make the above go away before I revisit - cannot recall the "why" they weren't included.

[daniel-savu]

I tried both and only rebasing off jg-Call-Event-2 fixes the errors.

Custom RPCs are not picked up though - I only checked now

[jacogr]

Yes, RPCs are not yet with metadata definitions. So for those custom types are needed.

[jacogr]

I took a brief look last night, I think it looks good. Will give a proper look-over and merge in a short-while.

[daniel-savu]

There are still two issues to fix I think:

• (second bullet point here) Since custom RPC definitions are not included in the metadata, definitions.ts (which exposes manual definitions) should not be overwritten as this PR does. The auto-generated types should be output somewhere else
• All custom-type imports should come from '@polkadot/types/lookup'. However currently there are two almost identical files for these types being generated, and imports are picked up from both (described here)

[jacogr]

1. For the from metadata types, it probably makes sense to output a lookup.ts file that contains the definitions. Clear split with the old version still in-tact. (Have not looked as to the overhead/mangling that needs to be in place to make it happen). But we probably needs a lookup.types.ts as well that contains the module declaration then?

2. For the lookup imports, it should not come from the definitions, with the type augmentation straight from @polkadot/types/lookup should be fine... I think. (The manual definitions are a bit different)

On both these points, I'd rather keep the interfaces/lookup types split, it is cleaner for manual vs automagic. (Which is why it is done that way in the API, i.e. you can see where it comes from)

[jacogr]

Code looks spot-on. Would just adjust the lookup generated definitions.ts and types.ts naming to not have a conflict, but apart from that it is clean.

[lgtm-com]

This pull request introduces 1 alert when merging daa4daa into 52accf6 - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class

[daniel-savu]

Generating separate lookup types was easy, but I've not had success finding a place to add their import. The new types-lookup.ts declares the module '@polkadot/types/lookup', but it's not being picked up.

For the lookup imports, it should not come from the definitions, with the type augmentation straight from @polkadot/types/lookup

createImports works based on the definitions, not the types. There's currently some types that are already being imported from '@polkadot/types/lookup', but that's because they're specified in lookupDefinitions and added here.

In files like generate/query.ts, I tried something like

import * as customLookupTypes from '@polkadot/types/lookup';
...
const allTypes: ExtraTypes = {
  '@polkadot/types/augment': { lookup: lookupDefinitions },
  '@polkadot/types/interfaces': defaultDefs,
  ...extraTypes,
  ...customLookupTypes
};

but customLookupTypes is empty

[jacogr]

At least for usage, something like this to tell TS to augment will be needed - https://polkadot.js.org/docs/api/examples/promise/typegen#typescript-config

not sure that moves you forward, will have to take a peek.

[jacogr]

Ok, I see how I adjusted this - for instance generate/consts.ts, we have this - '@polkadot/types/augment': { lookup: lookupDefinitions },

So it needs to be something like -

'@polkadot/types/augment': { 
  lookup: { 
    ...lookupDefinitions, 
    ...userLookupDefinitions
  }
},

And then we need to pass in the user definitions. You are right in that it only imports the in-tree stuff, so that needs to be adjusted with the additional new definitions.

The alternative would be for the user-from-metadata to not augment @polkadot/types/lookup (i.e. own module definitions/declare) and then the passing of it via extra would work (assuming it has an own path). I do prefer the approach above since it also ties in with stuff like createType, i.e. you get some extra help along the way. However, either one will work.

[daniel-savu]

It should be done now - no errors left after typegen is run. I'll run some integration tests on the generated interfaces tomorrow to make sure all is good.

After this is merged I'll update the CLI arguments in this snippet in the docs. Let me know if there are other places that need updating too.

[jacogr]

Thanks an absolute million for this - it has been at the back of my mind (since 6.0 went out the door), but just never had a gap to attack.

This really helps all teams out there and (while I may have neglected this), it a critical piece.

[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.