-
Notifications
You must be signed in to change notification settings - Fork 32
generate raw response types #764
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 type Query__subquery__rawResponse = { | ||
| query: { | ||
| node____id___v_id: { | ||
| __typename: "Node", |
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.
needs FIX
2c58992 to
e5fe58e
Compare
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.
Hell yeah. I can't wait to have a normalizeData API that has types that we can sent to Randall 🤩
| raw_response_type.push_str(&format!("{indent}}},\n")); | ||
| } | ||
| MergedServerSelection::ClientPointer(_) => {} | ||
| MergedServerSelection::InlineFragment(_) => { |
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.
This would be true for GraphQL in general, but we don't support anything besides abstract -> concrete type refinements via asConcreteType fields, so we can generate a discriminated union here.
e5fe58e to
81995b9
Compare
...s/pet-demo/src/components/__isograph/Viewer/NewsfeedPaginationComponent/raw_response_type.ts
Show resolved
Hide resolved
demos/pet-demo/src/components/__isograph/Query/OnlyOneRootLoadablePet/raw_response_type.ts
Show resolved
Hide resolved
01d1724 to
31c6598
Compare
93b5711 to
565e98e
Compare
3ec0f4e to
ec3d18b
Compare
rbalicki2
left a comment
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.
looks great! Only the plural thing seems like a blocker.
| lastName: string, | ||
| picture: string, | ||
| }, | ||
| picture_together?: (string | null), |
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.
- not a blocker if it's not trivial to fix, but
- we should confirm that if this isn't provided, we don't suspend. If we write
store.picture_together = data.picture_together, we would writeundefined, at which point this would cause us to suspend
export function readScalarFieldData(
field: ReaderScalarField,
storeRecord: StoreRecord,
root: StoreLink,
variables: Variables,
): ReadDataResult<
string | number | boolean | StoreLink | DataTypeValue[] | null
> {
const storeRecordName = getParentRecordKey(field, variables);
const value = storeRecord[storeRecordName];
// TODO consider making scalars into discriminated unions. This probably has
// to happen for when we handle errors.
if (value === undefined) {
return {
kind: 'MissingData',
reason: 'No value for ' + storeRecordName + ' on root ' + root.__link,
recordLink: root,
};
}
return { kind: 'Success', data: value };
}
- if it does cause us to suspend, I will inform Jerred about this, and he can use Isograph with that issue until we fix it
| node: { | ||
| id: string, | ||
| author: { | ||
| __typename: string, |
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.
🤔 can we make this a union of concrete strings of types that implement Actor? Not a blocker
demos/pet-demo/src/components/__isograph/Query/Newsfeed/raw_response_type.ts
Show resolved
Hide resolved
demos/pet-demo/src/components/__isograph/Query/OnlyOneRootLoadablePet/raw_response_type.ts
Show resolved
Hide resolved
| @@ -0,0 +1,10 @@ | |||
| export type Query__HomeRoute__rawResponse = { | |||
| pets: { | |||
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.
this should be an array
c9f8b7a to
e17885b
Compare
rbalicki2
left a comment
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.
shipitttt
add __typename in `create_merged_selection_set` if selection is empty foo
update demos update demos update demos
e17885b to
6444aa1
Compare
#177
This is part 3 of 3 in a stack made with GitButler:
create_merged_selection.rsif selections empty #766