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

Simplify the schema module #798

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Simplify the schema module #798

wants to merge 1 commit into from

Conversation

obmarg
Copy link
Owner

@obmarg obmarg commented Dec 30, 2023

Why are we making this change?

The cynic schema module is overly verbose - the same data could be expressed with less code.

What effects does this change have?

  • Removes the Field trait, moving its data into the HasField & HasInputField traits.
  • Moves the second generic parameter of HasInputField to an associated type.

This reduces the size of the schema module quite considerably, which should have a reasonable impact on compile times (~a 10% reduction in compile times of my test crate that uses the GitHub schema). It also has the bonus impact of simplifying some of the query building where clauses.

This is probably a breaking change though so should maybe be a major release.

Fixes #712

@obmarg obmarg changed the title wip simplifying the schema module Simplify the schema module Dec 30, 2023
Copy link

netlify bot commented Dec 30, 2023

Deploy Preview for cynic-querygen-web canceled.

Name Link
🔨 Latest commit 63e6194
🔍 Latest deploy log https://app.netlify.com/sites/cynic-querygen-web/deploys/6590595b5c3444000897bd3d

@obmarg obmarg force-pushed the obmarg/push-nnuummwqwyov branch from 6953023 to f17bc77 Compare December 30, 2023 17:54
@obmarg obmarg force-pushed the obmarg/push-nnuummwqwyov branch from f17bc77 to 63e6194 Compare December 30, 2023 17:54
@obmarg obmarg added this to the 4.0.0 milestone Jan 9, 2024
@obmarg
Copy link
Owner Author

obmarg commented Apr 7, 2024

A further optimisation that could maybe be made to this: deduplicating fields. At the moment there's a separate field struct created for each object<>field pair, with (iirc) the string literal living on the HasField impl.

I could try to have a single field struct for each field name that's present in the schema, with only seperate HasField impls for each object<>field pair. I think this would be particularly effective if I find somewhere else to store the field name literals...

Worth looking into making the same optimisation with arguments...

@obmarg obmarg added the breaking A breaking change label Jun 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking A breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simplify schema module further.
1 participant