Skip to content

fix(signals): patchState does not block on error - #5184

Open
rainerhahnekamp wants to merge 1 commit into
mainfrom
signals/fix/patch-state
Open

fix(signals): patchState does not block on error#5184
rainerhahnekamp wants to merge 1 commit into
mainfrom
signals/fix/patch-state

Conversation

@rainerhahnekamp

Copy link
Copy Markdown
Contributor

Signals can throw an error if they are accessed. patchState needs to call getState in order to provide the current state to an updater function.

This fix catches errors in the internal getState call and keeps the errored signals in a proxy, where accessing them throws again.

That means patchState does not block unrelated updates, and the user can apply the normal error handling features like try/catch, ErrorHandler, or the upcoming error boundaries.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Closes #5183

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Signals can throw an error if they are accessed. `patchState`
needs to call `getState` in order to provide the current state
to an updater function.

This fix catches errors in the internal `getState` call and keeps the errored signals in a proxy, where accessing them throws
again.

That means `patchState` does not block unrelated updates, and
the user can apply the normal error handling features like
`try/catch`, `ErrorHandler`, or the upcoming error boundaries.
@rainerhahnekamp
rainerhahnekamp force-pushed the signals/fix/patch-state branch from 58807f0 to 3ae6393 Compare July 1, 2026 00:23
@rainerhahnekamp
rainerhahnekamp marked this pull request as ready for review July 1, 2026 22:57
for (const key of Reflect.ownKeys(newState)) {
if (stateKeys.includes(key)) {
const signalKey = key as keyof State;
if (currentState[signalKey] !== newState[signalKey]) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a reason why the equality check is removed? This will now also invoke the setter of unchanged state keys.


for (const key of touchedKeys) {
if (stateKeys.includes(key as string | symbol)) {
signals[key].set(draftState[key]);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

An AI review flagged a potential regression.
Is this intended, or should we add this to the breaking changes?

it('applies linked state updates after their source updates', () => {
  const numberStore = signalStoreFeature(
    withState({ id: 1 }),
    withLinkedState(({ id }) => ({
      level: () => id() * 2,
    }))
  )(getInitialInnerStore());

  patchState(numberStore, { level: 5, id: 2 });

  // This succeeded before, but now resolves in  { id: 2, level: 4 }
  expect(getState(numberStore)).toEqual({ id: 2, level: 5 });
});

It has to do with the order in which we set the values. Doing patchState(numberStore, { id: 2, level: 5 }); is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@ngrx/signals: patchState blocks when State signal throws an error

2 participants