-
Notifications
You must be signed in to change notification settings - Fork 9
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
Document @solana/accounts
with TypeDoc
#51
Document @solana/accounts
with TypeDoc
#51
Conversation
|
BundleMonUnchanged files (127)
No change in files bundle size Final result: ✅ View report in BundleMon website ➡️ |
@solana/addresses
with TypeDoc@solana/accounts
with TypeDoc
a7aa081
to
bb0abcd
Compare
@@ -1,5 +1,6 @@ | |||
{ | |||
"$schema": "https://typedoc.org/schema.json", | |||
"extends": ["../typedoc.base.json"], | |||
"entryPoints": ["src/index.ts"] | |||
"entryPoints": ["src/index.ts"], | |||
"readme": "none" |
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 stops TypeDoc from vaccuming up the README
for display in the package docs' index, in favour of the comment with @packageDocumentation
.
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 this what we want, though? Because it looks like you just pasted the README anyway. Otherwise we will have to maintain both, right?
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.
I have a similar concern here. I think this is all amazing work. I'm just not sure about how we should proceed going forward with this information redundancy.
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.
Pasted the README? The idea here is that all documentation should be:
- In the code, if nowhere else
- Autogenable from the code
In a world where the README prose lives in the index.ts
file as @packageDocumentation
and the types/functions documentation lives on the types/functions themselves, then the README is fully codegennable from the code.
If you look at the preview site, the autogenerated package documentation has everything the README has now (and a little bit more).
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.
Nice!! Looking super slick.
packages/accounts/src/account.ts
Outdated
* @example | ||
* ```ts | ||
* const BaseAccount: BaseAccount = { | ||
* executable: false, | ||
* lamports: lamports(1_000_000_000n), | ||
* programAddress: address('1111..1111'), | ||
* }; | ||
* ``` |
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 missing space
.
* @example | ||
* ```ts | ||
* // Encoded | ||
* const myEncodedAccount: Account<Uint8Array, '1234..5678'> = { |
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.
I think it would be slightly more clear if the example string literal looked like a base58 address, instead of integers.
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.
We'll have to change that in a separate PR because this style is used in multiple packages.
* Asserts that an account stores decoded data, ie. not a `Uint8Array`. | ||
* | ||
* Note that it does not check the shape of the data matches the decoded type, only that it is not a | ||
* `Uint8Array`. |
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.
Interesting. I find the function signature a bit misleading then, since it requires the type parameter TData
, which would imply that it's checking against the provided type. If it's just asserting it's not a Uint8Array
, the type parameter can probably be elided. Perhaps we just rename the generic parameter.
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.
I doesn't require a TData
really because it is inferred from the first argument. So you give me a Account<Token | Uint8Array>
and you want to narrow that down to Account<Token>
, you use this function.
* Asserts that all input accounts store decoded data, ie. not a `Uint8Array`. | ||
* | ||
* As with {@link assertAccountDecoded} it does not check the shape of the data matches the decoded | ||
* type, only that it is not a `Uint8Array`. |
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.
Does this function work for an array of accounts that are all different TData
? Say you have a response from getProgramAccounts
with token accounts and mint accounts, this should work fine, right?
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, that can work if TData
equals Token | Mint
basically.
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.
It doesn't, actually. #49
* Given a {@link MaybeAccount}, asserts that the account exists and allows it to be used as an | ||
* {@link Account} type going forward. | ||
* | ||
* @typeParam TAddress - Supply a string literal to define an account having a particular address. | ||
* @typeParam TData - The nature of this account's data. It can be represented as either a | ||
* `Uint8Array` – meaning the account is encoded – or a custom data type – meaning | ||
* the account is decoded. |
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.
Calling out that, similar to assertAccountDecoded
, this function does not validate that the account has the provided TData
layout.
@@ -1,5 +1,6 @@ | |||
{ | |||
"$schema": "https://typedoc.org/schema.json", | |||
"extends": ["../typedoc.base.json"], | |||
"entryPoints": ["src/index.ts"] | |||
"entryPoints": ["src/index.ts"], | |||
"readme": "none" |
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 this what we want, though? Because it looks like you just pasted the README anyway. Otherwise we will have to maintain both, right?
Addresses #50
bb0abcd
to
b3a5bc7
Compare
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.
Alright, this is going to be an absolute ton of work, I'm going to need help, and this isn't the terminal state of the docs by any means, but rather a better starting point (ie. with the docs finally beside the functions / values / types to which it pertains). I'm going to land this and we'll keep moving forward.
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.
Uh… I guess I need a stamp from anyone first.
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.
👌
Merge activity
|
This PR moves content from the
README
into the code.Preview here: https://illustrious-gumption-b4d2fe.netlify.app/modules/_solana_accounts.html
Addresses #50