Skip to content

generate client pointers entrypoints #506

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

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

Conversation

PatrykWalach
Copy link
Member

@PatrykWalach PatrykWalach commented Apr 27, 2025

Stuff to be done:

  • better stable enumerate (now I enumerate all traversal results)
  • fix imports in entrypoints
  • prefix refetch queries (so entrypoint__0.ts has __refetch__0__0.ts, __refetch__0__1.ts, etc.)

I'd like feedback if what I've done so far seems correct

The main idea here was:

  • for client fields FieldToCompletedMergeTraversalStateMap stores a single entry, the merged selection set is the same for loadable and regular fields
  • for client pointers, we want one not loadable entry for reader_selection_set and an entry for each loadable since they have different merged selection sets

That's why I replaced ClientObjectSelectableId in FieldToCompletedMergeTraversalStateMap with new type ClientSelectableKey.

The ClientSelectableKey is a tuple of the ClientObjectSelectableId and either:

  • None - this is non loadable selection
  • Some(path) - this is loadable selection under some path, the path has a parent pointing to different key in the map

So far I've only tested this with query below and it seems to generate correct (besides the bugs above) entrypoints, and refetch query.

self {
  age
  __refetch
  self {
    tagline
  }
}
pointer Pet.self to Pet {
  debug: alt_tagline
  link
}

Part of #53

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.

Mind if we hop on a call so that I can understand what's going on? TBH I'm a bit lost, but it could be cause I'm really tired 😅

pub struct PathToClientPointer {
pub linked_fields: Vec<NormalizationKey>,
pub field_name: ClientObjectSelectableName,
pub parent_selectable_key: ClientSelectableKey,
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is this struct going to be used for? I'm not sure I follow, in particular, what the parent_selectable_key is going to be used for

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, we can hop on call

@PatrykWalach PatrykWalach force-pushed the 53-generate-pointer-entrypoints branch from 20a7f92 to c3485dd Compare May 4, 2025 22:37
@PatrykWalach
Copy link
Member Author

PatrykWalach commented May 4, 2025

  1. Right now this seems to be 90% working, refetchQuery for unnested pointer requests too many fields 😭 (but I have a feeling it'd run at runtime)
  2. Also refetch queries don't have readerAst so we use selections for that, this means, we don't need usedRefetchQueries 😄

This looks really clean

@PatrykWalach PatrykWalach force-pushed the 53-generate-pointer-entrypoints branch 6 times, most recently from ce57b31 to 4910884 Compare May 8, 2025 08:33
@PatrykWalach
Copy link
Member Author

This is fully working! 😄

@rbalicki2
Copy link
Collaborator

Omg I didn't notice your comment!!! Yes!!!! I should be able to finally get through the backlog tomorrow or next week

Once this lands and we add some uses of it in our demos, let's cut a release 😎

@PatrykWalach PatrykWalach force-pushed the 53-generate-pointer-entrypoints branch from 4910884 to 712fae8 Compare May 15, 2025 18:11
uncommited changes
do stuff
Uncommited changes
format
changes
stuff
do
do
@PatrykWalach PatrykWalach force-pushed the 53-generate-pointer-entrypoints branch 2 times, most recently from 87b92dc to a0fc693 Compare May 17, 2025 10:47
@@ -64,6 +66,7 @@ pub struct RootRefetchedPath {
pub enum MergedServerSelection {
ScalarField(MergedScalarFieldSelection),
LinkedField(MergedLinkedFieldSelection),
ClientPointer(MergedLinkedFieldSelection),
Copy link
Collaborator

Choose a reason for hiding this comment

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

did we discuss this? I'm a bit worried that this is the wrong move here, but TBH I feel like I would have noticed this and commented on it in the past.

Okay, so what's happening here (lol I'm literally prompting myself like I'm an LLM):

  • scalar server fields, object server fields and client pointers exist in the same namespace, so there's no chance of a conflict here, that's fine
  • this flows into the reachable variables. These flow into get_used_variable_definitions, so I think that's fine. Because if a variable is used in a client pointer, but not elsewhere, it should show up as a required parameter of the client pointer's loadable field type. (Does it??)

Okay, so as far as I can tell, it's not obviously broken. But does it belong? There are a few places we ignore this variant. Maybe that's indicative of it not belonging.

Okay, let's chat about this sometime.

Copy link
Member Author

@PatrykWalach PatrykWalach May 25, 2025

Choose a reason for hiding this comment

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

I did it this way so it's easy to traverse in current_target_merged_selections when client pointer or refetch is in another pointer

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