fix(angular-query-experimental): run mutation callback in injection context

[ShacharHarshuv]

Previously, the injectMutation callback ran in injection context only if accessed in the same context as the component initialization (i.e. before the effect callback run). By wrapping the effect callback in injection context, it is ensured that the callback will always run in injection context, and any inject calls will not fail.

Note there is a separate issue here, and it's that the callback is always run twice - once synchronously when initialization the mutation (when you create the MutationObserver), and secondly as the first callback of the effect. This is something that can potentially be improved to prevent redundant code run.

Another related issue - I would argue that if any signal dependencies of the mutation changed, you don't necessarily want to immediately run the callback again. You probably want to be "lazy" and run it again only when the mutation object is accessed. With the current implementation, that callback might unnecessarily run when there are a lot of dependency changes, but no mutation is used. (For example, imagine there is an "opened todo item" state, and a delete button on it. The user might switched the opened todo item state frequently without actually using the delete mutation).

No breaking changes, any code that worked before should still work.

[vercel[bot]]

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

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
query	⬜️ Ignored (Inspect)	Visit Preview		Jun 21, 2024 2:53pm

[nx-cloud[bot]]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 8d537b726f02fc9beabd457973d094ba90ceee16. 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

---

✅ Successfully ran 1 target

• 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]]

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 8d537b726f02fc9beabd457973d094ba90ceee16:

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]

Resolved conflicts. @arnoud-dv can you review?

[ShacharHarshuv]

The tests failures don't seem to be related to my change, as they fail in the react adapter.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 0.00%. Comparing base (93674fe) to head (82597e8). Report is 209 commits behind head on main.

:exclamation: Current head 82597e8 differs from pull request most recent head 2eeb6b4

Please upload reports for the commit 2eeb6b4 to get more accurate results.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #7360       +/-   ##
==========================================
- Coverage   41.42%       0   -41.43%     
==========================================
  Files         184       0      -184     
  Lines        7331       0     -7331     
  Branches     1531       0     -1531     
==========================================
- Hits         3037       0     -3037     
+ Misses       3889       0     -3889     
+ Partials      405       0      -405     

Components	Coverage Δ
@tanstack/angular-query-devtools-experimental	∅ <ø> (∅)
@tanstack/angular-query-experimental	∅ <ø> (∅)
@tanstack/eslint-plugin-query	∅ <ø> (∅)
@tanstack/query-async-storage-persister	∅ <ø> (∅)
@tanstack/query-broadcast-client-experimental	∅ <ø> (∅)
@tanstack/query-codemods	∅ <ø> (∅)
@tanstack/query-core	∅ <ø> (∅)
@tanstack/query-devtools	∅ <ø> (∅)
@tanstack/query-persist-client-core	∅ <ø> (∅)
@tanstack/query-sync-storage-persister	∅ <ø> (∅)
@tanstack/react-query	∅ <ø> (∅)
@tanstack/react-query-devtools	∅ <ø> (∅)
@tanstack/react-query-next-experimental	∅ <ø> (∅)
@tanstack/react-query-persist-client	∅ <ø> (∅)
@tanstack/solid-query	∅ <ø> (∅)
@tanstack/solid-query-devtools	∅ <ø> (∅)
@tanstack/solid-query-persist-client	∅ <ø> (∅)
@tanstack/svelte-query	∅ <ø> (∅)
@tanstack/svelte-query-devtools	∅ <ø> (∅)
@tanstack/svelte-query-persist-client	∅ <ø> (∅)
@tanstack/vue-query	∅ <ø> (∅)
@tanstack/vue-query-devtools	∅ <ø> (∅)

[ShacharHarshuv]

Can this be merged?

[kfrancois]

@arnoud-dv I believe this one's good to be merged, wdyt?

[arnoud-dv]

Hi I just ran the test from this PR on my laptop and it passes with the current implementation. Can you create a test or a repro that demonstrates the issue?

[kfrancois]

Nice catch @arnoud-dv ! Thank you for checking this out.

@ShacharHarshuv I added a test in https://github.com/kfrancois/query/commit/eb05ac27314401d551e76e962ad58c758cd79a04#diff-aa4e4fa79cbfdc8cb15c0cfb49c3b858294ec82865c5e2cdbf0ba98e2927dfe4R459-R502 which I verified fails with the current implementation. If you think this can be useful, feel free to take that change along in this PR

[ShacharHarshuv]

@arnoud-dv Apparently the error wasn't caught by the test, thanks for noticing. I took @kfrancois's approach of using an "errorSpy" and manually catching the error. Now the test will fail as expected without my change.