ShacharHarshuv commented 2 weeks ago

This is to allow consumers to use inject inside callbacks like queryFn, initialData etc.

vercel[bot] commented 2 weeks ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

nx-cloud[bot] commented 2 weeks ago

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 2d5a3df4e6bcf735f804e7fad868aedb47289977. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

🟥 Failed Commands
`nx affected --targets=test:format,test:sherif,test:knip,test:eslint,test:lib,test:types,test:build,build,test:attw --parallel=3`

Sent with 💌 from NxCloud.

codesandbox-ci[bot] commented 2 weeks ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 2d5a3df4e6bcf735f804e7fad868aedb47289977:

Sandbox	Source
@tanstack/query-example-angular-basic	Configuration
@tanstack/query-example-react-basic-typescript	Configuration
@tanstack/query-example-solid-basic-typescript	Configuration
@tanstack/query-example-svelte-basic	Configuration
@tanstack/query-example-vue-basic	Configuration

ShacharHarshuv commented 2 weeks ago

Converted to draft as we probably want to add the same thing for injectMutation (and maybe others?)

riccardoperra commented 1 week ago

Shouldn’t properties usually be injected in a class field or inside the query options callback?

Just my opinion, this can lead to unexpected usage from consumers and gives too much freedom 🤔

Angular doesn’t allow to inject inside callback or methods, but as a workaround you can explicitly use runInInjectionContext

ShacharHarshuv commented 1 week ago

I get what you are saying, but in practice, and in many use cases, not being able to do it here just makes DX more difficult.

Here's an example:

cosnt todosQueryOptions = () => {
  const http = inject(HttpClient);

  return queryOptions({ queryKeys: ['todos'], ... });
}

Now I want to invalidate that query (or optimistically update) after creating a new todo, in a type safe way:

onSuccess: async (response) => { queryClient.invalidateQueries(todosQueryOptions()) }

This will fail because onSuccess is not in an injection context, and todosQueryOptions is an injection function.

The way I see it, Angular "injection context" is set in this way because code that doesn't run as initialization of a component could be run from any context and "required" any arbitrary injector. In other words - Angular wouldn't know what injector to use.

However - in this case - since there is a place you are "injecting" the query, it's fair to set it up so that all of the queries work will be (by default) in the same injection context. You can still wrap it in a different injection context with a different injection if you want, but I think that use case will not be very common.

riccardoperra commented 1 week ago

I understood the issue 😄

@Injectable({providedIn: 'root'})
class TodoQuery { 
  readonly http = inject(HttpClient);

  todosQueryOptions() {
    const http = inject(HttpClient);

    return queryOptions({ queryKeys: ['todos'], ... });
  }
}

then

readonly todoQuery = inject(TodoQuery);

onSuccess: async (response) => { queryClient.invalidateQueries(this.todoQuery.todosQueryOptions()) }

I always used options with a service like above so I've didn't had this error before.

At the moment you have the same issue on react and solid adapters. In solid you can wrap the fn with runWithOwner in order to get the context, in react I think you cannot do it until you create a hook for the query option

function useTodosQueryOptions() { 
   const variable = useContext(...)
   return { ... }

}

function App() { 
  const todosQueryOptions = useTodosQueryOptions();

  const query = useQuery(todosQueryOptions());
}

which in Angular the corrispective approach could be doing something like injectTodosQueryOptions

ShacharHarshuv commented 1 week ago

You first example wouldn't work because you are injecting inside the todosQueryOptions method. When you call it in a non-injection context it will throw. I assume you meant that you don't inject that method by use the member instead? I find that approach cumbersome as I would ideally want to encapsulate my dependencies as much as possible, and also separate things like "query options" to different files / symbols and not couple them to the same service I'm using them for.

This becomes more annoying if my query options creator function accepts an argument (like id).

Overall, while workarounds exist, I believe my proposed change improves DX and doesn't have many negative side-effects. cc @arnoud-dv What do you think?

TanStack / query

feat(angular-query-experimental): run query options callbacks in injection context #7362

☁️ Nx Cloud Report