-
Notifications
You must be signed in to change notification settings - Fork 115
@joystream/types - api augmentation #1177
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
Conversation
|
So far tested generating the defs, but I used latest node build which removed the contracts module from the runtime and there was some renaming of AccountInfo -> ServiceProviderRecord, so this was reflected in: Perhaps we can check-in updated versions of these types (if latest iznik branch is merged into this types-augment branch) then we can confirm matching results. |
| { | ||
| "extends": "./tsconfig-base.json", | ||
| "compilerOptions": { | ||
| "noEmit": true, // No need to actually create any output |
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.
is the json file correctly parsed despite the non-standard comments in json ?
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.
Yes, comments seem to be allowed in tsconfig.json (related PR: microsoft/TypeScript#5450) even though it's technically not a valid json.
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.
This file is actually used during the CI checks now (it's part of check:augment script which checks the validity of auto-generated files and is ran as the last step of checks)
|
So this is really great, tested the types definitions working nicely. I had problem using one of our modules though because of a name clash with another substrate module, the
typescript infers general
I can't remember off the top of my head if there are other modules with identical names to a substrate module where this might happen as well. Is there a way to change the order of which type defs are looked at first with some magic ts-config settings? The obvious solution would be to rename our own module, but since we don't even use the substrate council module it would be nicer to find a way to drop the Edit: |
I cannot reproduce this issue. I'm using a project with those paths inside The right types are inferred both by VSCode and Sometimes changes in |
|
Ok so just an update, I added |
Ah yes, I also restarted VScode and that was what resolved it after modifying tsconfig file 👍 |
mnaamani
left a comment
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.
This is a fantastic PR making coding in the IDE much more pleasant now that all type definitions are available. Nice work.
Implements #1165
Brief summary of PR content
@polkadot/typegenand create all the auto-generated files based on current@joystream/typeswith just one command (yarn workspace @joystream/types generate:all). This is furhter described in the section below.eslintupgrade to allow the CI chekcs to pass.types/augmentandtypes/augment-codecI changed the directory structure a little bit for more clarity, now the generated augment files can be found in two folders:
types/augment- those augments are relying on interfaces automatically generated by@polkadot/typegenbased ontypes/augment/all/definitions.tsand can be re-generated from current@joystream/typesand runtime metadata by runningyarn workspace @joystream/types generate:augment(requires running local node!). Those support augmentation forapi.query,api.consts,api.tx,api.createTypeetc.types/augment-codec- those augments are using our custom codec types classes instead of interfaces automatically generated by@polkadot/typegenallowing us to access custom getters/methods, ie.api.createType('SomeEnum').isOfTypeorapi.createType('ContentId').encode(). Currently they only supportapi.createType. They can be re-generated withyarn workspace @joystream/types generate:codec-defsAll auto-generated files can be updated at once by running
yarn workspace @joystream/types generate:all(requires running local node first in order for the script to be able to connect to it and fetch the current metadata used to decorate the api)NOTE: All projects that previosly used
"@polkadot/types/augment": ["path/to/types/src/definitions/augment-types.ts"]insidetsconfig.jsoncan switch to"@polkadot/types/augment": ["path/to/types/augment-codec/augment-types.ts"]for full backward-compatibilityExample of using
types/augment-codec:tsconfig.json:src/someScript.ts:Example of using
types/augment:tsconfig.json:src/someScript.ts:Future possabilities
I think that by making a few additional adjustments to the scripts we should be able to combine the benefits of using
types/augment(api agugmentation based on runtime metadata) andtypes/augment-codec(using our custom getters etc. in types) by generatingaugment-api.tsinsidetypes/augment-codec. This will require a few additional hours of work, but could be very beneficial before we decide to fully migrate to@polkadot/typegen(sacrificing our custom getters, but making@joystream/typestrivial to manage)