Skip to content
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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

beepidibop
Copy link

schema.graphql isn't automatically translated to ponder.schema.ts when a new repo is started with the subgraph 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.

image

Copy link
Collaborator

@0xOlias 0xOlias left a 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";
Copy link
Collaborator

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.

Copy link
Author

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}`);
Copy link
Collaborator

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?

Copy link
Author

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}`
Copy link
Collaborator

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(
Copy link
Collaborator

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?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Graph docs had some other types other than @entity, but yeah it should be a skip+warning instead of throwing an error
image


writeFileSync(
schemaPath,
await prettier.format(translatedSchema, { parser: "typescript" }),
Copy link
Collaborator

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yee I agree

Comment on lines 35 to 54
// 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`,
);
}
});
Copy link
Collaborator

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)})`);
  }
}

- 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants