Skip to content

fix: assign properties of mutations in withMutation #288

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.

Sign up for GitHub

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

Draft
michael-small wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix: assign properties of mutations in withMutation #288

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions libs/ngrx-toolkit/src/lib/with-mutations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,25 @@ function createMutationsFeature<Result extends MutationsDictionary>(
keys.reduce(
(acc, key) => ({
...acc,
[key]: async (params: never) => {
const mutation = mutations[key];
if (!mutation) {
throw new Error(`Mutation ${key} not found`);
}
const result = await mutation(params);
return result;
},
[key]: Object.assign(
async (params: never) => {
const mutation = mutations[key];
if (!mutation) {
throw new Error(`Mutation ${key} not found`);
}
const result = await mutation(params);

return result;
},
{
status: mutations[key].status,
value: mutations[key].value,
isPending: mutations[key].isPending,
isSuccess: mutations[key].isSuccess,
error: mutations[key].error,
hasValue: mutations[key].hasValue,
Comment on lines +132 to +137
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is intended to make store.<mutation> usable as a full Mutation object (e.g. store.increment.isPending()), but the existing with-mutations.spec.ts suite only asserts the derived <mutation>IsPending/<mutation>Status signals and does not cover property access on the method itself. Add a regression test that calls store.increment.isPending()/status() (and ideally hasValue()), so the runtime shape is verified.

Suggested change
status: mutations[key].status,
value: mutations[key].value,
isPending: mutations[key].isPending,
isSuccess: mutations[key].isSuccess,
error: mutations[key].error,
hasValue: mutations[key].hasValue,
status: () => mutations[key].status(),
value: () => mutations[key].value(),
isPending: () => mutations[key].isPending(),
isSuccess: () => mutations[key].isSuccess(),
error: () => mutations[key].error(),
hasValue: () => mutations[key].hasValue(),

Copilot uses AI. Check for mistakes.
},
Comment on lines +121 to +138
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new Object.assign accesses mutations[key].status/value/... during feature creation. If mutationsFactory accidentally returns an object with an undefined entry, this will now throw a generic Cannot read properties of undefined at store init time, and the explicit Mutation ${key} not found error inside the async wrapper will never run. Consider validating const mutation = mutations[key] once per key (outside the wrapper) and throwing the custom error before reading any properties, or otherwise guarding property reads.

Copilot uses AI. Check for mistakes.
),
Comment on lines +121 to +139
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only a fixed subset of Mutation properties are copied onto the wrapped function. This means any mutation subtype that adds extra properties (e.g. HttpMutation adds uploadProgress, downloadProgress, headers, statusCode) will still lose those when exposed via the store method. To make this future-proof and fully preserve mutation capabilities, prefer assigning from the mutation object itself (e.g. copy all enumerable properties from mutation) rather than enumerating a hard-coded list.

Copilot uses AI. Check for mistakes.
}),
{} as MethodsDictionary,
),
Expand Down