Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
40 changes: 40 additions & 0 deletions modules/signals/spec/state-source.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
StateSource,
watchState,
withHooks,
withLinkedState,
withMethods,
withState,
} from '../src';
Expand Down Expand Up @@ -246,6 +247,45 @@ describe('StateSource', () => {
TestBed.tick();
expect(userChangedCount).toBe(2);
});

describe('error behavior', () => {
function setupStore() {
const Store = signalStore(
{ providedIn: 'root', protectedState: false },
withState({ name: '' }),
withLinkedState(() => ({
value: () => {
throw new Error('Failed to read value.');
},
}))
);

return TestBed.inject(Store);
}

it('patches an unrelated state slice without throwing', () => {
const store = setupStore();

expect(() => patchState(store, { name: 'foo' })).not.toThrow();
expect(store.name()).toBe('foo');
});

it('throws when an updater reads the errored state slice', () => {
const store = setupStore();

expect(() =>
patchState(store, ({ value }) => ({ name: String(value) }))
).toThrow('Failed to read value.');
});

it('throws when patching the errored state slice', () => {
const store = setupStore();

expect(() => patchState(store, { value: undefined })).toThrow(
'Failed to read value.'
);
});
});
});

describe('getState', () => {
Expand Down
58 changes: 43 additions & 15 deletions modules/signals/src/state-source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,24 +82,24 @@ export function patchState<State extends object>(
Partial<NoInfer<State>> | PartialStateUpdater<NoInfer<State>>
>
): void {
const currentState = untracked(() => getState(stateSource));
const newState = updaters.reduce(
(nextState: State, updater) => ({
...nextState,
...(typeof updater === 'function' ? updater(nextState) : updater),
}),
currentState
);

const signals = stateSource[STATE_SOURCE];
const stateKeys = Reflect.ownKeys(stateSource[STATE_SOURCE]);
const draftState = untracked(() => getSafeState(stateSource));
const touchedKeys = new Set<keyof State>();

for (const updater of updaters) {
const partial =
typeof updater === 'function' ? updater(draftState) : updater;

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.

signals[signalKey].set(newState[signalKey]);
}
for (const key of Reflect.ownKeys(partial) as Array<keyof State>) {
touchedKeys.add(key);
draftState[key] = partial[key] as State[keyof State];
}
}

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.

} else if (typeof ngDevMode !== 'undefined' && ngDevMode) {
console.warn(
`@ngrx/signals: patchState was called with an unknown state slice '${String(
Expand All @@ -114,6 +114,34 @@ export function patchState<State extends object>(
notifyWatchers(stateSource);
}

function getSafeState<State extends object>(
stateSource: StateSource<State>
): State {
const signals: Record<string | symbol, Signal<unknown>> = stateSource[
STATE_SOURCE
];
const state = {} as State;

for (const key of Reflect.ownKeys(signals)) {
try {
(state as Record<string | symbol, unknown>)[key] = signals[key]();
} catch (error) {
Object.defineProperty(state, key, {
get() {
throw error;
},
set() {
throw error;
},
enumerable: true,
configurable: true,
});
}
}

return state;
}

/**
* @description
*
Expand Down
Loading