-
-
Notifications
You must be signed in to change notification settings - Fork 161
chore: dedup typescript typegen logic #993
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
Conversation
export const Constants = { | ||
${schemas | ||
.sort(({ name: a }, { name: b }) => a.localeCompare(b)) | ||
.map((schema) => { | ||
const schemaEnums = types | ||
.filter((type) => type.schema === schema.name && type.enums.length > 0) | ||
.sort(({ name: a }, { name: b }) => a.localeCompare(b)) | ||
return `${JSON.stringify(schema.name)}: { | ||
${schemas.map((schema) => { | ||
const schemaEnums = introspectionBySchema[schema.name].enums | ||
return `${JSON.stringify(schema.name)}: { | ||
Enums: { | ||
${schemaEnums.map( | ||
(enum_) => | ||
`${JSON.stringify(enum_.name)}: [${enum_.enums | ||
.map((variant) => JSON.stringify(variant)) | ||
.join(', ')}]` | ||
)} | ||
} | ||
}` | ||
})} | ||
})} |
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.
note
Forgot to remove this loop, now we can use the introspectionSchema directly where the values are already filtered and sorted.
${relationships | ||
.filter( | ||
(relationship) => | ||
relationship.schema === table.schema && | ||
relationship.referenced_schema === table.schema && | ||
relationship.relation === table.name | ||
) |
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.
note
Noticed that I forgot this loop, instead I now push those into the introspectedSchema
object under each table/view so we don't have to filter and sort everytime.
Also extracted the typegen into it's own dedicated function reducing duplication across tables/views definitions.
const getFunctionTsReturnType = (fn: PostgresFunction, returnType: string) => { | ||
return `${returnType}${fn.is_set_returning_function ? '[]' : ''}` | ||
} |
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.
Only used in one place, I think we can inline this
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.
Yup, those two will be growing a lot in the next PR though which is why I've did the split here. So in the next PR we can focus over the changes scoped to those functions: https://github.com/supabase/postgres-meta/pull/971/files#diff-c903ec71df16a8ae5f665c19c21b273f74c2f8eb02f5e8a3552ebe69d18be18fR214-R265
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.
Ack, thx for the rationale 👍
return 'unknown' | ||
} | ||
|
||
const getFunctionSignatures = ( |
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.
Also only used in one place
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.
Pull Request Test Coverage Report for Build 18155869351Details
💛 - Coveralls |
What kind of change does this PR introduce?