lukemorales / query-key-factory

A library for creating typesafe standardized query keys, useful for cache management in @tanstack/query
https://www.npmjs.com/package/@lukemorales/query-key-factory
MIT License
1.16k stars 32 forks source link

PhpStorm freezes on code completion when using mergeQueryKeys() #74

Open tklaas opened 1 year ago

tklaas commented 1 year ago

There are quite a few problems you can see in the attached video using version 1.3.2

https://github.com/lukemorales/query-key-factory/assets/5176891/f8e863e8-455f-4f82-9718-16dbcd1689fe

I tried a few things to narrow down the source of the lag:

image

File: index.ts

import { mergeQueryKeys } from '@lukemorales/query-key-factory';
import { teamQueries } from 'src/queries/queries/teams';

export const queries = mergeQueryKeys(teamQueries)

File teams.ts

import { createQueryKeys } from '@lukemorales/query-key-factory';
import TeamMember from 'src/models/TeamMember';

export const teamQueries = createQueryKeys('teams', {
  allStats: () => ({
    queryKey: [''],
    queryFn: () => {
      const query = TeamMember.includes([{members: 'person'}]).all()
      return query.then(data => data.data);
    }
  }),
})

I have also submitted a bug report to Jetbrains for the freezing UI, but it sees that the problem is related to query-key-factory or my setup of using this lib https://youtrack.jetbrains.com/issue/WI-73631/Indexing-on-each-start-of-PhpStorm-and-slow-unusuable-code-completion https://youtrack.jetbrains.com/issue/WEB-62694/Slow-highlighting-when-use-lukemorales-query-key-factory-lib

lukemorales commented 1 year ago

@tklaas this is the first report on an issue like this, but I can see mergeQueryKeys being the cause of the slowness as it is iterating over each array member dynamically to infer the type of each object. May I suggest using createQueryKeyStore instead and see how it behaves?

tklaas commented 1 year ago

@lukemorales using createQueryKeyStore in my index.ts works and there is no lag or freezing UI. At leats when I wrote the query keys directly within this file

export const queries = createQueryKeyStore({
  teams: {
    allStats: () => ({
        queryKey: [''],
        queryFn: () => {
          const query = Team.all()
          return query.then(data => data.data);
        }
      })
}
})

But I cannot separate my queries in different files since I am getting these errors:

in index.ts - "teams" as key got the TS error:

TS2322: Type '{ allStats: () => { queryKey: string[]; queryFn: () => Promise<PersistedSpraypaintRecord[]>; }; }' is not assignable to type 'QueryFactorySchema'.   Property 'allStats' is incompatible with index signature.     Type '() => { queryKey: string[]; queryFn: () => Promise<PersistedSpraypaintRecord[]>; }' is not assignable to type 'FactoryProperty | DynamicKey'.       Type '() => { queryKey: string[]; queryFn: () => Promise<PersistedSpraypaintRecord[]>; }' is not assignable to type 'DynamicKey'.         Type '{ queryKey: string[]; queryFn: () => Promise<PersistedSpraypaintRecord[]>; }' is not assignable to type 'KeyTuple | QueryKeyRecord | DynamicQueryFactoryWithContextualQueriesSchema | DynamicQueryFactorySchema | DynamicKeySchemaWithContextualQueries'.           Type '{ queryKey: string[]; queryFn: () => Promise<PersistedSpraypaintRecord[]>; }' is not assignable to type 'DynamicQueryFactorySchema'.             Type '{ queryKey: string[]; queryFn: () => Promise<PersistedSpraypaintRecord[]>; }' is not assignable to type 'QueryKeyRecord'.               Types of property 'queryKey' are incompatible.                 Type 'string[]' is not assignable to type 'KeyTuple'.                   Type 'string[]' is not assignable to type '[ValidValue, ...(ValidValue | undefined)[]]'.                     Source provides no match for required element at position 0 in target.

and in any file where I am using it, there is no code completion:

image
lukemorales commented 1 year ago

@tklaas I've created this dummy repro to match what you just shared and I get no typescript errors:

import { createQueryKeyStore } from '@lukemorales/query-key-factory';

type PersistedSpraypaintRecord = {
  id: string;
};

const Team = {
  all: async () => ({ data: [] as PersistedSpraypaintRecord[] }),
};

export const queries = createQueryKeyStore({
  teams: {
    allStats: () => ({
      queryKey: [''], // BTW, you should use `null` here if you don't need any dynamic values
      queryFn: () => {
        const query = Team.all();
        return query.then((data) => data.data);
      },
    }),
  },
});

queries.teams.allStats(); // works just fine

Can you reproduce your error in codesandbox?

tklaas commented 1 year ago

Can you reproduce your error in codesandbox? I will try to get a "working" example with the error

queryKey: [''], // BTW, you should use `null` here if you don't need any dynamic values

Yes, I have seen using nullin the documentation, but the following error is shown to me with null

image
tklaas commented 1 year ago

Here is an example what I described in this comment: https://github.com/lukemorales/query-key-factory/issues/74#issuecomment-1663671917

The slow code completion works by using createQueryKeyStore, but I won't be able to separate my queries into multiple files as I could with mergeQueryKeys

import { createQueryKeyStore } from '@lukemorales/query-key-factory';

type PersistedSpraypaintRecord = {
  id: string;
};

const Team = {
  all: async () => ({ data: [] as PersistedSpraypaintRecord[] }),
};

const teamQueries = createQueryKeyStore({
  teams: {
    allStats: () => ({
      queryKey: [null],  // TS error for type null
      queryFn: () => {
        const query = Team.all()
        return query.then(data => data.data);
      }
    })
  },
})

teamQueries.teams.allStats(); // this works fine

const teamQueriesObject = {
  allStats: () => ({
    queryKey: [''],
    queryFn: () => {
      const query = Team.all()
      return query.then(data => data.data);
    }
  })
}

const queries = createQueryKeyStore({
  teams: teamQueriesObject  // teams got the TS error below
}) 

queries.teams.allStats(); // this completion does not work.

TS error for teams: (I really like the new layout for TS errors from Jetbrains :) )

image
lukemorales commented 1 year ago

Oh, ok I think I know what the issues are:

  1. I completely overlook this in your example, but when you create a queryKey definition with a function allStats: () =>, the type-system will understand that you're creating a dynamic key, where the function has arguments and you need to pass these arguments to the queryKey array, so for your example the correct way would be:

    const teamQueries = createQueryKeyStore({
    teams: {
    allStats: {
      queryKey: null,
      queryFn: () => Team.all().then(data => data.data),
    }
    },
    })
  2. For the second approach:

    
    const teamQueriesObject = {
    allStats: {
    queryKey: [''],
    queryFn: () => Team.all().then(data => data.data),
    }
    }

const queries = createQueryKeyStore({ teams: teamQueriesObject // teams got the TS error below })


What's happening is that the type-system needs to infer the array as a tuple (`[string] instead of string[]`), and since it can't, it starts complaining. Two ways of fixing that are: 
a. For this case, using `null` without initializing the definition with a function.
b. For dynamic keys, declare with `as const` to make it a tuple: `queryKey: [page, limit] as const`
```ts
const teamQueriesObject = {
  allStats: {
    queryKey: [''] as const,
    queryFn: () => Team.all().then(data => data.data),
  }
}

const queries = createQueryKeyStore({
  teams: teamQueriesObject  // this now works
}) 
lukemorales commented 1 year ago

But your issue gave me an ideia of a improvement, perhaps I should deprecate mergeQueryKeys for being low in performance and expose an util that allows what you're trying to do:

// queries/teams.ts
const teamQueriesObject = createQueryScopeDefinition({ // name TBD
  allStats: {
    queryKey: [''],
    queryFn: () => {
      const query = Team.all()
      return query.then(data => data.data);
    }
  }
})

// queries/index.ts
const queries = createQueryKeyStore({
  teams: teamQueriesObject
}) 
tklaas commented 1 year ago

Thanks for your quick replies and helping with the typescript errors. 👍 I think I can now refactor my code to use createQueryKeyStore with your provided workaround to use different files for scope separation.

yakobe commented 11 months ago

Hi @lukemorales

I am also hitting performance issues in phpstorm with mergeQueryKeys. Am i right in understanding that we need to wait for the createQueryScopeDefinition util mentioned in https://github.com/lukemorales/query-key-factory/issues/74#issuecomment-1665796165 before we can use createQueryKeyStore and import from separate files with code completion etc?

Thanks for this great library 🙂

lukemorales commented 7 months ago

Hi folks, sorry for taking so long to tackling this issue, I'm able to put more effort in this lib for the next few weeks. I just released v1.1.3 with some improvements to the type inference on mergeQueryKeys, could you upgrade your versions and tell me if PhpStorm still freezes? cc @yakobe @tklaas

tklaas commented 5 months ago

@lukemorales

I got time to test it today. switching from v1.3.2 to 1.3.4 solved the issue. PhpStorm ist not freezing anymore and reverting to 1.3.2 makes the issue reproducable. Thanks for fixing this 👍

Is there any advantage using mergeQueryKeys instead createQueryKeyStore? with createQueryKeyStore I have the advantage that all keys are defined within index.ts so there are no duplicated keys possible

// index.ts
export const queries = mergeQueryKeys(teamQueries)

//teams.ts
export const teamQueries = createQueryKeys('teams',  { ... })

vs.

// index.ts
export const queries = createQueryKeyStore({
   teams: teamQueries,
})

//teams.ts
export const teamQueries = {...}