Skip to content

refactor(signals)!: multiple signals in STATE_SOURCE #4779

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

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

Conversation

rainerhahnekamp
Copy link
Contributor

This PR is a necessary refactoring to support a state which allows different types of Signals, i.e. linkedSignal & signal but still presents itself as a single unit to the outside.

This commit splits the single Signal of STATE_SOURCE, which contains the state in the SignalStore and signalState, into multiple Signals.

An example. Given the following type:

type User = {
  firstname: string;
  lastname: string;
};

Before, STATE_SOURCE would have been the following type:

WritableSignal<User>;

With this change, it is:

{
  firstname: WritableSignal<string>;
  lastname: WritableSignal<string>;
}

Most changes affect the tests which focus on the STATE_SOURCE. Except for one test in signal-state.spec.ts ('does not modify STATE_SOURCE'), all tests could be updated to assert the new behavior.

Breaking Changes

  • Any code which accesses the hidden STATE_SOURCE will be impacted.
  • Breaking Changes to the public behavior are rare.

different object reference for state of STATE_SOURCE

STATE_SOURCE does not keep the object reference of the initial state upon initialization.

const initialObject = {
  ngrx: 'rocks',
};
const state = signalState(initialObject);

// before
state() === initialObject; // ✅

// after
state() === initialObject; // ❌

no Signal change on empty patched state

patchState created a clone of the state and applied the patches to the clone. That meant, if nothing was changed, the Signal still fired because of the shallow clone. Since the state Signal doesn't exist anymore, there will be no change in this case.

const state = signalState({ ngrx: 'rocks' });

let changeCount = 0;
effect(() => {
  changeCount++;
});

TestBed.flushEffects();
expect(changeCount).toBe(1);

patchState(state, {});

// before
expect(changeCount).toBe(2); // ✅

// after
expect(changeCount).toBe(2); // ❌ changeCount is still 1

Further Changes

  • signalStoreFeature had to be refactored because of typing issues with the change to WritableStateSource.
  • patchState get the NoInfer for updaters argument. Otherwise, with multiple updaters, the former updater would have defined the State for the next updater.

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[x] Refactoring with minor breaking changes to the public API

Does this PR introduce a breaking change?

[x] Yes
[ ] No

Copy link

netlify bot commented May 13, 2025

Deploy Preview for ngrx-io ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit f39fa13
🔍 Latest deploy log https://app.netlify.com/sites/ngrx-io/deploys/6823c6aa267b42000725bcf6
😎 Deploy Preview https://deploy-preview-4779--ngrx-io.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented May 13, 2025

Deploy Preview for ngrx-site-v19 failed.

Name Link
🔨 Latest commit f39fa13
🔍 Latest deploy log https://app.netlify.com/sites/ngrx-site-v19/deploys/6823c6aa44a9630008d28e06

This is a necessary refactoring to support a state which allows different types of Signals, i.e. `linkedSignal` & `signal` but still presents itself as a single unit to the outside.

This commit splits the single Signal of `STATE_SOURCE`, which contains the state in the `SignalStore` and `signalState`, into multiple Signals.

An example. Given the following type:

```typescript
type User = {
  firstname: string;
  lastname: string;
};
```

Before, `STATE_SOURCE` would have been the following type:

```typescript
WritableSignal<User>;
```

With this change, it is:

```typescript
{
  firstname: WritableSignal<string>;
  lastname: WritableSignal<string>;
}
```

Most changes affect the tests which focus on the `STATE_SOURCE`. Except for one test in `signal-state.spec.ts` ('does not modify STATE_SOURCE'), all tests could be updated to assert the new behavior.

## Breaking Changes

- Any code which accesses the hidden `STATE_SOURCE` will be impacted.
- Breaking Changes to the public behavior are rare.

### different object reference for state of `STATE_SOURCE`

`STATE_SOURCE` does not keep the object reference of the initial state upon initialization.

```typescript
const initialObject = {
  ngrx: 'rocks',
};
const state = signalState(initialObject);

// before
state() === initialObject; // ✅

// after
state() === initialObject; // ❌
```

### no Signal change on empty patched state

`patchState` created a clone of the state and applied the patches to the clone. That meant, if nothing was changed, the Signal still fired because of the shallow clone. Since the state Signal doesn't exist anymore, there will be no change in this case.

```typescript
const state = signalState({ ngrx: 'rocks' });

let changeCount = 0;
effect(() => {
  changeCount++;
});

TestBed.flushEffects();
expect(changeCount).toBe(1);

patchState(state, {});

// before
expect(changeCount).toBe(2); // ✅

// after
expect(changeCount).toBe(2); // ❌ changeCount is still 1
```

## Further Changes

- `signalStoreFeature` had to be refactored because of typing issues with the change to `WritableStateSource`.
- `patchState` get the `NoInfer` for `updaters` argument. Otherwise, with multiple updaters, the former updater would have defined the `State` for the next updater.
@rainerhahnekamp rainerhahnekamp force-pushed the signals/refactor/multi-signal branch from 52b0cba to f39fa13 Compare May 13, 2025 22:24
@rainerhahnekamp rainerhahnekamp changed the title refactor!: multiple signals refactor(signals)!: multiple signals in STATE_SOURCE May 13, 2025
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.

1 participant