feat(signals): enhance withComputed to accept computation functions
#4822
Conversation
❌ Deploy Preview for ngrx-io failed.Built without sensitive environment variables
|
✅ Deploy Preview for ngrx-site-v19 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| @@ -54,4 +59,34 @@ describe('withComputed', () => { | |||
| 'p1, s2, m1, Symbol(computed_secret)' | |||
| ); | |||
| }); | |||
|
|
|||
| it('adds computed automatically if the value is function', () => { | |||
There was a problem hiding this comment.
we can add a type test to signal-store.types.spec.ts to validate the new withComputed behavior just in case
modules/signals/src/with-computed.ts
Outdated
| [P in keyof ComputedSignals]: ComputedSignals[P] extends () => infer V | ||
| ? Signal<V> | ||
| : ComputedSignals[P]; |
There was a problem hiding this comment.
Provided Signals are also compatible with () => infer V type, but we should not change their type (e.g. when DeepSignal is provided to withComputed by using deepComputed helper).
| [P in keyof ComputedSignals]: ComputedSignals[P] extends () => infer V | |
| ? Signal<V> | |
| : ComputedSignals[P]; | |
| [K in keyof ComputedInput]: ComputedInput[K] extends Signal<any> | |
| ? ComputedInput[K] | |
| : ComputedInput[K] extends () => infer V ? Signal<V> : never; |
When adding a type test, please also add one computed signal by using deepComputed and one writable signal to ensure their type hasn't been changed.
There was a problem hiding this comment.
Done, I went here also with the term ComputedDictionary. Also used unknown instead any. Please see me question in the comment below.
modules/signals/src/with-computed.ts
Outdated
| return withProps(signalsFactory); | ||
| return withProps((store) => { | ||
| const signals = signalsFactory(store); | ||
| const stateKeys = Reflect.ownKeys(signals); |
There was a problem hiding this comment.
| const stateKeys = Reflect.ownKeys(signals); | |
| const computedResultKeys = Reflect.ownKeys(signals); |
| @@ -145,7 +145,7 @@ export class BookSearch { | |||
|
|
|||
| Computed signals can be added to the store using the `withComputed` feature. | |||
| This feature accepts a factory function as an input argument, which is executed within the injection context. | |||
| The factory should return a dictionary of computed signals, utilizing previously defined state signals and properties that are accessible through its input argument. | |||
| The factory should return a dictionary containing either computed signals or functions that return values (which will automatically be wrapped in computed signals), utilizing previously defined state signals and properties that are accessible through its input argument. | |||
There was a problem hiding this comment.
| The factory should return a dictionary containing either computed signals or functions that return values (which will automatically be wrapped in computed signals), utilizing previously defined state signals and properties that are accessible through its input argument. | |
| The factory should return a dictionary containing either computed signals or computation functions (which are automatically converted to computed signals), utilizing previously defined state signals and properties that are accessible through its input argument. |
There was a problem hiding this comment.
@markostanimirovic @timdeschryver, do we want to use the term "computation functions"? I am afraid that this could confuse users.
There was a problem hiding this comment.
I think this is fine. We can always revisit later.
|
@markostanimirovic, @timdeschryver. I've applied the suggested changes with the following additions:
|
… to functions
Example:
```ts
const CounterStore = signalStore(
withState({ count: 0 }),
withComputed(({ count }) => ({
doubleCount: () => count() * 2,
})),
);
```
- Updated withComputed to automatically wrap functions in computed signals.
- Added tests to verify automatic computation and mixing user-defined computeds.
- Improved documentation to clarify the factory function's return type.
Co-authored-by: Marko Stanimirović <markostanimirovic95@gmail.com>
Co-authored-by: Marko Stanimirović <markostanimirovic95@gmail.com>
Co-authored-by: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com>
Co-authored-by: Marko Stanimirović <markostanimirovic95@gmail.com>
Co-authored-by: Marko Stanimirović <markostanimirovic95@gmail.com>
Co-authored-by: Marko Stanimirović <markostanimirovic95@gmail.com>
rainerhahnekamp
force-pushed
the
signals/feat/add-computed-on-computation-function-in-with-computed
branch
from
9352161 to
5e38ee7
Compare
|
As discussed, the latest commit enables access to methods within withComputed. This is a necessary step toward supporting a potential withResource feature in the 20.x release line. When a resource is in an error state, accessing its value will throw. Therefore, users must first verify the presence of a value using the hasValue method. In the meantime, I am going to prepare the updated docs (in a separate PR). Since this is a significant change, I've also asked @timdeschryver for a re-review. const Store = signalStore(
withResource(() =>
// spreads members of the resource to the store, including hasValue as method
resource({
loader: () => Promise.resolve({ firstName: 'John', lastName: 'Doe' }),
})
),
withComputed((store) => ({
safeUser: () =>
store.user.hasValue()
? store.user.value()
: { firstName: '', lastName: '' },
}))
); |
rainerhahnekamp
force-pushed
the
signals/feat/add-computed-on-computation-function-in-with-computed
branch
from
7b0cf69 to
5e38ee7
Compare
|
I'd very much like to have the ability to provide a function with a parameter that returns a signal in order to create an equivalent of a factory selector in ngrx store world. I know this can be achieved in the Just asking because of this comment which would I assume make that hard:
But maybe the recommendation is to just use |
Co-authored-by: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com>
Co-authored-by: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com>
Co-authored-by: michael-small <33669563+michael-small@users.noreply.github.com>
Yes, factory functions for computed are actually methods. So they should go in |
…ction-in-with-computed
markostanimirovic
changed the title
withComputed to automatically add computed…withComputed to accept computation functions
Example:
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Closes #4782
Does this PR introduce a breaking change?