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

Generate artifacts for @updatable scalars #325

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

Conversation

PatrykWalach
Copy link
Collaborator

Follow up to #314
Part of updatable fields #312

Copy link
Collaborator

@rbalicki2 rbalicki2 left a comment

Choose a reason for hiding this comment

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

some nits, then I'll re-review


ClientFieldParameterType(client_field_parameter_type)
pub struct DualStringProxy<'a> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice, I like this a lot. Let's add a comment to say why we're using this.

However, it's a bit unfortunate that we have to sometimes write through DualStringProxy, and sometimes let DualStringProxy drop and write directly to readonly_data_type and updatable_data_type.


Maybe we should pass a single object around, query_type_declaration: RegularAndUpdatableQueryTypeDeclaration, which contains methods:

  • write_scalar_field(name, value, is_updatable)
  • write_linked_field(name, value, subfields, is_updatable)

or perhaps

  • write_linked_field_start(name, value, is_updatable)
  • close_linked_field

the object could keep track of the indentation

IMO this gets us closer to what we probably should be doing, which is creating TypeScript ASTs, instead of concatenating a bunch of strings.


Let's make this change ☝️ as a separate PR that has no concept of updatable fields (i.e. just create a QueryTypeDeclaration object)

@PatrykWalach PatrykWalach changed the title Generate artifacts for @updatablescalars Generate artifacts for @updatable scalars Jan 26, 2025
associated_data
.type_name
.clone()
.map(&mut |_| updatable_data_type.0.clone()),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I also don't like recursively cloning two Strings here parameter_type and updatable_data_type

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