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

Allow scalarToLiteralMapping to declare TS type for extended scalar type #836

Merged

Conversation

CarsonF
Copy link
Collaborator

@CarsonF CarsonF commented Jan 12, 2024

As far as I've been able to deduce there's no need to jump to the type's first base type to determine the TS literal value.
This is because the function already jumps to the type's material_id, which has already worked out the "base concrete type".
https://github.com/edgedb/edgedb-js/blob/dfcf3d48f74fd78c47146b505a4e942d4b33d849/packages/driver/src/reflection/queries/types.ts#L165-L169
https://github.com/edgedb/edgedb-js/blob/dfcf3d48f74fd78c47146b505a4e942d4b33d849/packages/driver/src/reflection/queries/types.ts#L139-L144
This change resulted in no changes in my generated query builder, schema, or query files, so I'm fairly confident in it. Though not 100% and I don't know the difference between bases and ancestors (maybe they are the same for ScalarType?).


With that change in place I was able to add the logical change: allowing scalarToLiteralMapping to declare scalars that should use their own TS literal type instead of the base type.
My use case is this:

scalar type RichText extending json;
scalarToLiteralMapping['default::RichText'] = { type: 'TSShapeOfRichText' };

@CarsonF CarsonF force-pushed the customizeable-scalar-literal-mapping branch from dfcf3d4 to 7c92551 Compare January 12, 2024 19:18
Copy link
Collaborator

@scotttrinh scotttrinh left a comment

Choose a reason for hiding this comment

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

Yeah, these changes make sense to me, thanks for investigating!

@scotttrinh scotttrinh merged commit c991a82 into geldata:master Jan 12, 2024
6 of 7 checks passed
@CarsonF CarsonF deleted the customizeable-scalar-literal-mapping branch January 12, 2024 21:02
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