update "Recommended IDs for New Entities" to "Using Bytes as IDs" #614
Conversation
There was a problem hiding this comment.
Great work. I think it's important to have the framing correct: The docs are talking about "now" and not the past and what has changed. Therefor, the best would probably be to assume a new dev reading it. For them, they do not care if there was once a ID that translated to String. They simply want to learn that ids should be 'Bytes` and some common pattern.
| ### Using Bytes as IDs | ||
|
|
||
| Every entity has to have an `id` that is unique among all entities of the same type. An entity's `id` value is set when the entity is created. Below are some recommended `id` values to consider when creating new entities. NOTE: The value of `id` must be a `string`. | ||
| You can use `Bytes` as the type for the `id` field of entities, and it is highly recommended to use `Bytes` wherever possible, and only use `String` for attributes that truly contain human-readable text, like the name of a token. |
There was a problem hiding this comment.
I think the alternative to Bytes is ID, which translates to String. Can you validate?
There was a problem hiding this comment.
The ID often corresponds to a string value. However, depending on the data source and implementation, it could also be another data type.
| - `event.params.id.toHex()` | ||
| - `event.transaction.from.toHex()` | ||
| - `event.transaction.hash.toHex() + "-" + event.logIndex.toString()` | ||
| - Remove unnecessary toHexString() calls. Instead of `transfer.id = event.transaction.hash.toHexString()`, |
There was a problem hiding this comment.
Again, the mind-set of the reader is that they do not know anything about IDs and want to learn. So there is nothing to "remove"
|
|
||
| use `let id = event.transaction.hash.concatI32(counter)` | ||
|
|
||
| For entities that store aggregated data, for example, daily trade volumes and the like, the id usually contains the day number. Here, too, using a byte string as the id is beneficial. Determining the id would look like |
| ``` | ||
|
|
||
| - Convert constant addresses to `Bytes`. Here, instead of `const BEEF_ADDRESS = '0xdead...beef'` | ||
| use `const BEEF_ADDRESS = Bytes.fromHexString('0xdead...beef')` |
There was a problem hiding this comment.
Yes, but we are talking about ids here. I'd do something like:
const id = Bytes.fromHexString('0xdead...beef')
| - `event.transaction.hash.toHex() + "-" + event.logIndex.toString()` | ||
| - `transfer.id = event.transaction.hash` | ||
|
|
||
| - For entities whose `id` consists of the concatenation of a byte array with some counter, concatenate byte arrays directly so here instead of |
There was a problem hiding this comment.
Referring to this comment: #614 (review)
As a new subgraph developer, mentioning an "old" way of doing things is just confusing. I'd focus only on the "now" and skip that section. Also, that example could be a bit more concrete in my opinion. Why not event.transaction.hash.concatI32(event.logIndex.toI32())
|
|
||
| - For entities whose `id` consists of the concatenation of a byte array with some counter, concatenate byte arrays directly so here instead of | ||
|
|
||
| ```typrescript |
| .concat(BigInt.fromI32(counter).toString()) | ||
| ``` | ||
|
|
||
| use `let id = event.transaction.hash.concatI32(counter)` |
There was a problem hiding this comment.
I think that example could be a bit more concrete. Why not event.transaction.hash.concatI32(event.logIndex.toI32()) as it was before?
| ### Recommended IDs for Creating New Entities | ||
|
|
||
| Every entity has to have an `id` that is unique among all entities of the same type. An entity's `id` value is set when the entity is created. Below are some recommended `id` values to consider when creating new entities. NOTE: The value of `id` must be a `string`. | ||
| You can use `Bytes` as the type for the `id` field of entities, and it is highly recommended to use `Bytes` wherever possible, and only use `String` for attributes that truly contain human-readable text, like the name of a token. Below are some recommended `id` values to consider when creating new entities. |
There was a problem hiding this comment.
I suggest to avoid personal voice like "You", "We", etc. to achieve a level of technical authority. You can read more about this here: https://ohiostate.pressbooks.pub/feptechcomm/chapter/3-1-voice-tone/
I'd rewrite the first sentence simply like:
It is highly recommended to use
Bytesas the type foridfields.
There was a problem hiding this comment.
noted, thanks for the resource
|
|
||
| `const id = Bytes.fromHexString('0xdead...beef')` | ||
|
|
||
| We provide the [Graph Typescript Library](https://github.com/graphprotocol/graph-ts) which contains utilities for interacting with the Graph Node store and conveniences for handling smart contract data and entities. You can use this library in your mappings by importing `@graphprotocol/graph-ts` in `mapping.ts`. |
There was a problem hiding this comment.
Similarly here but more strict: We can not use "we" language in docs. Who is "we"? The Graph is a protocol. I'd rephrase that paragraph:
There is the Graph Typescript Library which contains utilities for interacting with the Graph Node store and conveniences for handling smart contract data and entities. It can be imported into
mapping.tsfrom@graphprotocol/graph-ts.
2 participants
fixes #613.
made use of this blog