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
feat(angular-query-experimental): run query options callbacks in injection context #7362
base: main
Are you sure you want to change the base?
feat(angular-query-experimental): run query options callbacks in injection context #7362
Conversation
…ction context This is to allow consumers to use `inject` inside callbacks like `queryFn`, `initialData` etc.
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 2d5a3df. 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
Sent with 💌 from NxCloud. |
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 2d5a3df:
|
Converted to draft as we probably want to add the same thing for |
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 |
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:
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 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. |
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 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 |
You first example wouldn't work because you are injecting inside the 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? |
This is to allow consumers to use
inject
inside callbacks likequeryFn
,initialData
etc.