-
-
Notifications
You must be signed in to change notification settings - Fork 98
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
Take LoadingType object union into account for FragmentStore #1389
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: b846ed9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for houdini-docs-next canceled.
|
✅ Deploy Preview for houdinigraphql canceled.
|
248b0e9
to
0775223
Compare
This comment was marked as outdated.
This comment was marked as outdated.
59d390a
to
5bcbc86
Compare
5bcbc86
to
c9d3466
Compare
bd8bf5a
to
b846ed9
Compare
export type GraphQLDefaultScalar = string | number | boolean | ||
|
||
export type GraphQLValue = GraphQLDefaultScalar | null | GraphQLObject | GraphQLValue[] | undefined | ||
export type GraphQLLoadedValue = |
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.
Maybe i'm missing something but is there a problem with just updating GraphQLObject
to always have the loaded value? If so, let's merge these 2 types with something like type GraphQLLoadedValue = GraphQLValue | LoadingType
. Also, reading this outloud I think i would prefer a name like GraphQLValueWithLoading
. a "loaded graphql value" sounds like it's already loaded (ie no loading values would be present).
Fixes #1388
By defining new GraphQLLoadedValue & GraphQLLoadedObject types and including LoadingType in GraphQLValue's definition, every type in the library take could accept a loading value reflects this fact in its types. This allows typescript to infer things correctly
To help everyone out, please make sure your PR does the following:
pnpm run tests
andcd integration && pnpm run tests
pnpm changeset