HoudiniGraphql / houdini

The disappearing GraphQL client
http://www.houdinigraphql.com
MIT License
893 stars 94 forks source link

Align Result and QueryResult #1303

Open gschulze opened 3 months ago

gschulze commented 3 months ago

Describe the feature

I discovered that there are two types that are quite similar: Result and QueryResult.

One is defined as:

export type Result<DataType> = {
    isFetching: boolean
    partial: boolean
    source?: DataSource | null
    data?: DataType | null
    error?: Error | null
}

the other one as

export type QueryResult<_Data = GraphQLObject, _Input = GraphQLVariables> = {
    data: _Data | null;
    errors: {
        message: string;
    }[] | null;
    fetching: boolean;
    partial: boolean;
    stale: boolean;
    source: DataSources | null;
    variables: _Input | null;
};

Why is it called isFetching one time and fetching the other? I'm asking because our project uses derived stores, which propagate the fetching field with a transformed data field. Instead of defining our own Result type, I would rather use an existing one, so I discovered the inconsistency.

It would be nice if these types used the same name for all fields.

Criticality

None

AlecAivazis commented 2 months ago

Yea this is a bit of a wart that has developed over the years. I'll leave this issue open to remind us when we set up for 2.0

AlecAivazis commented 1 month ago

@SeppahBaws what to do think about getting this done along with the upcoming 2.0 release?

SeppahBaws commented 1 month ago

Yep sounds good. Are you talking about houdini-svelte 2.0 or houdini 2.0 here?

Maybe it's also worth creating a milestone or two here in github so we have a more clear view of what we want to add/fix when?

AlecAivazis commented 1 month ago

Are you talking about houdini-svelte 2.0 or houdini 2.0 here?

Whichever needs the change. We're going to have to roll out 2.0 versions for both since the graphql version will change

Maybe it's also worth creating a milestone

Good idea! I went ahead and did that.