feat(signal): extends merged features results up to 25

ngrx / platform

Reactive State for Angular

https://ngrx.io

Other

7.96k stars 1.95k forks source link

feat(signal): extends merged features results up to 25 #4315

Closed SonyStone closed 2 months ago

SonyStone commented 2 months ago

PR Checklist

Please check if your PR fulfills the following requirements:

[x] The commit message follows our guidelines: https://github.com/ngrx/platform/blob/main/CONTRIBUTING.md#commit
[x] 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?

[ ] Bugfix
[x] 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 #4314

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

netlify[bot] commented 2 months ago

Deploy Preview for ngrx-io canceled.

Built without sensitive environment variables

Name	Link
Latest commit	b1f2c32838ec147733b6db2b5adb93e076c6e5b0
Latest deploy log	https://app.netlify.com/sites/ngrx-io/deploys/6639f46fd3e0ab00085d1402

brandonroberts commented 2 months ago

Let's discuss this in the issue first

SonyStone commented 2 months ago

You could fix this issue now just by those add types, which has no breaking changes for curent users.

markostanimirovic commented 2 months ago

You could fix this issue now just by those add types, which has no breaking changes for curent users.

Let's see. The question is - is this really an issue that should be fixed?

For example, Observable.pipe supports typing for up to 9 operators. If you have more than 9 operators in your chain, you're probably on the wrong way.

Before trying to find a solution or workaround for a problem that occurs in a system, we should first check if the system is properly designed. In other words, having a SignalStore with 20+ features is probably a wrong design choice, because that store does too many things and it could be very hard to maintain.

Anyway, adding additional signatures will be considered, but not 20+.

abhijit-chikane commented 1 week ago

any update on this?

rainerhahnekamp commented 1 week ago

@abhijit-chikane you find the discussion and issue at #4314