fix(signals): expose readonly signals in slices

[rainerhahnekamp]

withState, signalState, and withLinkedState now pass a Signal instead of the original WritableSignal to the DeepSignal.

This prevents accidental misuse where consumers (e.g. ngModel or other APIs with incompatible types) could assert WritableSignal and write directly into the state.

Closes #4958

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/ngrx/platform/blob/main/CONTRIBUTING.md#commit
• Tests for the changes have been added (for bug fixes / features)
• Documentation has been added / updated (for bug fixes / features)

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?

const person = signalState({ title: 'Title'});
(person.title as WritableSignal<string>).set('Hallo'); <-- direct access to state

Closes #4958

What is the new behavior?

Will throw an error if set is called because the Signal is readonly.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

[netlify]

✅ Deploy Preview for ngrx-site-v19 ready!

Name	Link
🔨 Latest commit	ccb566d
🔍 Latest deploy log	https://app.netlify.com/projects/ngrx-site-v19/deploys/691e048ca88b6900087cc7f1
😎 Deploy Preview	https://deploy-preview-4959--ngrx-site-v19.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

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

[netlify]

✅ Deploy Preview for ngrx-io canceled.

Name	Link
🔨 Latest commit	ccb566d
🔍 Latest deploy log	https://app.netlify.com/projects/ngrx-io/deploys/691e048cc4f56a00084d427a

[markostanimirovic]

Let's merge this in v21 to prevent unintentional breaking changes.

[timdeschryver]

As Marko has mentioned this can be a breaking change.
Do we want to include it in the migration notes?
If so, we should flag this PR as breaking change to not forget about it.

[rainerhahnekamp]

@timdeschryver just added the label "breaking changes" as suggested

[markostanimirovic]

@rainerhahnekamp @timdeschryver

After revisiting this PR, I'm not sure if we should proceed with it, because maybe in the future we will introduce deep writable signals within the store while keeping them as read-only outside of the store instance. In that case, deep writables would be protected only at the TypeScript level, and we would have to revert this (and make another breaking change).

Therefore, I'm not sure if this change is worth it. E.g., for the state source, we also have protection only at the TS level. If someone wants to avoid it (and use as any/WritableStateSource), I see that as a usage issue, not a bug from our side.

[rainerhahnekamp]

@markostanimirovic

the thing was that it happened via two-way binding with ngModel. So users didn't apply bad practices or anything unnatural. So they didn't even receive any warnings, error messages, etc.

@manfredsteyer you reported this one. Can you elaborate on it?

[rainerhahnekamp]

@markostanimirovic, do you think changes like deep-writables - even if they are only inside the store - can be done without breaking changes? As you remember, even the multi-signals approach in v20 should have been internal-only, but it turned out to be the opposite.

[markostanimirovic]

@markostanimirovic, do you think changes like deep-writables - even if they are only inside the store - can be done without breaking changes? As you remember, even the multi-signals approach in v20 should have been internal-only, but it turned out to be the opposite.

Yes, maybe there would be a breaking change for deep writables, but regardless, I'm wondering do we need to introduce this one.