Feature/near data source template #824
Conversation
This will make it easier to add new network and will improve error messages when someone will be using fields that do not exist on a specific network but does in other (`temlates` for example used in NEAR were given a bunch of errors like the support was possible while it should have displayed an error on the `templates` node itself). Using `templates` on NEAR before this PR: ``` ✔ Apply migrations ✖ Failed to load subgraph from ../graph-node-dev/subgraphs/near/data-source/testnet.yaml: Error in ../graph-node-dev/subgraphs/near/data-source/testnet.yaml: Path: templates > 0 > source No value provided Path: templates > 0 > mapping Unexpected key in map: receiptHandlers Path: templates > 0 > mapping > abis No value provided ``` And with this PR: ``` $ ./bin/graph build ../graph-node-dev/subgraphs/near/data-source/testnet.yaml ✖ Failed to load subgraph from ../graph-node-dev/subgraphs/near/data-source/testnet.yaml: Error in ../graph-node-dev/subgraphs/near/data-source/testnet.yaml: Path: / Unexpected key in map: templates ```
maoueh
changed the base branch from
master
to
ci/sf/feature/schema-per-protocol
| } | ||
|
|
||
| generateModuleImports() { | ||
| return [] |
There was a problem hiding this comment.
I think you'll need to add 'near' here.
There was a problem hiding this comment.
It's not required because there is actually no near type involved in the generated code:
import { DataSourceTemplate, DataSourceContext } from "@graphprotocol/graph-ts";
export class receipts extends DataSourceTemplate {
static create(account: string): void {
DataSourceTemplate.create("receipts", [account]);
}
static createWithContext(account: string, context: DataSourceContext): void {
DataSourceTemplate.createWithContext("receipts", [account], context);
}
}
I tested the full flow of NEAR data source template and it worked great.
There was a problem hiding this comment.
Oh I see why I thought that, that's because you're not using the Account type from the near module. I would prefer that instead of using string directly. What do you think?
There was a problem hiding this comment.
I'm ambivalent.
If we go with Account, it should be used thoroughly everywhere, right now it's a mixed usage in graph-ts (mostly 50/50).
The second thing that makes it me doubt is how people on NEAR are used to work directly with string. I've research this a little bit and it seems AccountId is used more (https://github.com/near/near-sdk-rs/blob/05c389c75a568af3f37a3346d6f7b4064852e1de/near-sdk/src/types/account_id.rs#L39) although I've seen various example dealing with String directly. The AssemblyScript however does not seems to have an AccountId alias and seems to deal with string directly: https://github.com/near/near-sdk-as/blob/master/sdk-core/assembly/contract.ts#L12.
I prefer string personally as it make the cognitive load lower. If we choose to still wrap it, I'm voting more for AccountId to align with the Rust SDK for NEAR.
There was a problem hiding this comment.
Hmm I see 🤔
Well we can discuss that later then, I'm gonna merge this, thanks for the PR!!
evaporei
force-pushed
the
ci/sf/feature/schema-per-protocol
branch
from
6235e0a to
00427b7
Compare
|
@maoueh this is missing a conflict resolution because of the refactor you've done that got merged into |
|
We will wait until the actual feature in |
No description provided.