-
Notifications
You must be signed in to change notification settings - Fork 102
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
Add Translate Schema When Starting a New Repo With Subgraph #1011
base: main
Are you sure you want to change the base?
Conversation
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.
Awesome work! Quick questions.
I'm a bit worried about throwing at all in this file. I think the best outcome is to collect a list of warning messages and log them at the end - we already do this with the etherscan template if e.g. the ABI is not found. This would make it so create-ponder
always succeeds, but the schema file might be only partially generated.
@@ -0,0 +1,265 @@ | |||
import { gql } from "graphql-tag"; |
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.
Any reason not to use parse
from graphql
? Looks like graphql-tag
uses it under the hood anyway.
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.
No real reason, it was just the first thing that I found that works
const namedTypeNode = nonNullTypeNode.type as NamedTypeNode; | ||
if (!namedTypeNode?.name?.value) { | ||
console.log(field); | ||
throw new Error(`Unsupported id type for ${tableName}`); |
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.
Are you aware of a test case where this is reachable?
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.
Probably not. It's there as a check to see if the id type is not a list but yeah it probably won't happen on an already deployed subgraph.
definition.fields?.forEach((field) => { | ||
const fieldName = field.name.value; | ||
tables[tableName]!.descriptions[fieldName] = field.description?.value | ||
? `// ${field.description?.value}` |
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.
Epic that we can get the column descriptions as well!
}),\n`, | ||
); | ||
} else { | ||
throw new Error( |
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.
Same question as above - any idea if this is reachable?
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.
|
||
writeFileSync( | ||
schemaPath, | ||
await prettier.format(translatedSchema, { parser: "typescript" }), |
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.
FWIW, this will take care of whitespace and newline characters, so we can omit them above if it helps readability.
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.
Yee I agree
// Parse enums | ||
// Need to do all the enums first before processing other types | ||
// because there is no other way to determine if a graphql definition is referring to | ||
// an enum or another table | ||
Object.keys(schemaAST.definitions).forEach((definitionName) => { | ||
const definition = schemaAST.definitions[ | ||
definitionName as any as number | ||
] as DefinitionNode; | ||
if (definition.kind === "EnumTypeDefinition") { | ||
const enumName = definition.name.value; | ||
enums.push(enumName); | ||
const values: string[] = []; | ||
definition.values?.forEach((value) => { | ||
values.push(value.name.value); | ||
}); | ||
|
||
enumsResult.push( | ||
` ${enumName}: p.createEnum(${JSON.stringify(values)}),\n\n`, | ||
); | ||
} | ||
}); |
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.
Just a style nit - I'd write this as
// Build enums first because they are required to determine if a
// column type references an enum or another table.
for (const definition of schemaAST.definitions) {
if (definition.kind === "EnumTypeDefinition") {
const name = definition.name.value;
const values = definition.values?.map((value) => value.name.value) ?? [];
enums.push(name);
enumsResult.push(`${name}: p.createEnum(${JSON.stringify(values)})`);
}
}
…existing subgraph
- use graphql instead of graphql-tag - push warnings instead of throwing errors - some whitespace changes to make code more readable - refactor how enums are generated - distinguish parent and child for self-referencing tables
21397de
to
66a3ecf
Compare
schema.graphql
isn't automatically translated toponder.schema.ts
when a new repo is started with thesubgraph
option.This PR adds this function.
I tested the function with a few
schema.graphql
from Messari and they seem to work so far. There might still be some edge cases that haven't been accounted for.