Closed rainerhahnekamp closed 1 week ago
@rainerhahnekamp I had a need for this in an app, and I gave it a go at trying to implement it, I got it working, and then just noticed you had a version in stackblitz, my version has a few improvements like it keeps the wrapped feature input type in case you need it as well
import { SignalStoreFeature } from '@ngrx/signals';
import type {
SignalStoreFeatureResult,
SignalStoreSlices,
} from '@ngrx/signals/src/signal-store-models';
import type { StateSignal } from '@ngrx/signals/src/state-signal';
import { Prettify } from '@ngrx/signals/src/ts-helpers';
export function withFeatureFactory<
Input extends SignalStoreFeatureResult,
Feature extends SignalStoreFeature<any, any>,
>(
featureFactory: (
store: Prettify<
SignalStoreSlices<Input['state']> &
Input['signals'] &
Input['methods'] &
StateSignal<Prettify<Input['state']>>
>,
) => Feature,
): SignalStoreFeature<
Input & (Feature extends SignalStoreFeature<infer In, any> ? In : never),
Feature extends SignalStoreFeature<any, infer Out> ? Out : never
> {
return ((store: any) => {
const { slices, methods, signals, hooks, ...rest } = store;
return featureFactory({
...slices,
...signals,
...methods,
...rest,
} as Prettify<
SignalStoreSlices<Input['state']> &
Input['signals'] &
Input['methods'] &
StateSignal<Prettify<Input['state']>>
>)(store);
}) as Feature;
}
Thanks, I never used it honstly. Was just a quick prototype.
In our extensions we still depend on the non-public types. This feature would improve the situation, but only if it lands in the core (also depends on internal types).
That's a good point. I also use non-public types in a few places, but this is mainly because I need to allow methods to use things from the store. If this were part of the core, it would reduce the need to expose those non-public types.
Having thought on this more, maybe this could be an overload of the signalStoreFeature like
signalStore(
withMethods(() => {
const httpClient = inject(HttpClient);
return {
load() {
return httpClient.get<Person[]>('someUrl');
},
};
}),
signalStoreFeature((store) => withEntityVersioner(() => store.load())),
);
I would love to write:
signalStore(
withMethods(() => {
const httpClient = inject(HttpClient);
return {
load() {
return httpClient.get<Person[]>('someUrl');
},
};
}),
withEntityVersioner((store) => store.load()),
);
But as I asked in https://github.com/ngrx/platform/issues/4272, it requires to expose more types to the public API.
@gabrielguerrero, I think - and please correct me if I'm wrong - we are talking here about two different features.
signalStoreFeature
because that's the closest thing at hand. So your usage of signalStoreFeature
is bound to a specific signalStore
.flowchart LR
signalStoreFeature --> Service --> signalStore
signalStoreFeature
which needs to get features of an existing signalStore
by not depending on it. flowchart LR
ServiceImplementation --> signalStore
withFeatureFactory --> GenericService
withFeatureFactory --> ServiceImplementation
signalStoreFeature --> GenericService
I'd we need both but maybe not under the same name.
Hey @rainerhahnekamp, I was indeed referring to having the withFeatureFactory be just an overloaded in signalStoreFeature, my reasoning was that the names are similar and I was not sure about the name withFeatureFactory when its main goal is just to give access to the store to another store feature, I don't want people to think they can be use to create store features, that's signalStoreFeature job, which is where I got the idea of maybe making it part signalStoreFeature, an overloaded version that receives factory function with the store as param. I'm just brainstorming, really; I'm not really sure it is a good idea; maybe we just need a different name for withFeatureFactory.
Is this proposal also intended to enable other signalStoreFeatures to manipulate the state of a store?
Something like:
const counterStore = signalStore(
{ providedIn: 'root' },
withState(intitialState),
witSharedState((store) => withIncrement<counter>('items', store))
);
function withIncrement<Entity extends object>(
key: keyof Entity,
sharedStoreIn: StateSignal<Entity>
) {
return signalStoreFeature(
withMethods(() => {
const sharedStore = sharedStoreIn as any; //How to type the store?
return {
withIncrement() {
patchState(sharedStore, { [key]: sharedStore[key]() + 1 });
},
};
})
);
}
Even if it is not intended, could someone help me to type out that use case? https://stackblitz.com/edit/ngrx-signal-store-starter-23tsna?file=src%2Fmain.ts
Probably, it could also be an idea to let developers choose if they want to share store elements and offering a system like "shareState()", "shareMethods(), "shareStore()".
@Donnerstagnacht
you find a working and typed solution at https://stackblitz.com/edit/ngrx-signal-store-starter-z3q5i5?file=src%2Fmain.ts
Your withIncrement
will not get the store but an wrapper function which patches the state. By doing that we can decouple withIncrement
from the specific store and make it therefore reusable.
@Donnerstagnacht you can also do it with already available feature store Input condition (as documented here)
https://stackblitz.com/edit/ngrx-signal-store-starter-rlktqe?file=src%2Fmain.ts
@rainerhahnekamp in your last example, it seems weird to move the updaterFn
out of the custom feature. I like the withFeatureFactory
, but I'd rather use it like this:
https://stackblitz.com/edit/ngrx-signal-store-starter-qedtku?file=src%2Fmain.ts
But a type SignalStoreFeatureStore
would be needed (I am not sure about the naming of this type π
)
Hi @GuillaumeNury, yeah that's much better.
In the end, it is the old story about getting access to the internal types, right?
@GuillaumeNury, @Donnerstagnacht: I've updated my version. Turned out that the only type we really need is SignalState
and that one is public.
So we can use the version of @GuillaumeNury and not violate the encapsulation.
@rainerhahnekamp kind of. But with SignalState
+ 'withFeatureFactory' we can keep internals hidden π
I changed the generic of withIncrement
to allow the state to have non-numeric properties : https://stackblitz.com/edit/ngrx-signal-store-starter-pozgde?file=src%2Fmain.ts
The result looks wonderful! I hope to see this in v18.X π
@rainerhahnekamp I tried to implement again the withTabVisibility
feature, using withFeatureFactory
and I do not need internal types anymore π
https://stackblitz.com/edit/ngrx-signal-store-starter-dmybgr?file=src%2Fmain.ts
The only change is going from:
signalStore(
withTabVisibility({
onVisible: (store) => store.loadOpportunity(store.opportunityId()),
}))
to:
signalStore(
withFeatureFactory((store) =>
withTabVisibility({
onVisible: () => store.loadOpportunity(store.opportunityId()),
}),
)
)
There is no need for a separate withFeatureFactory
.
As described in https://ngrx.io/guide/signals/signal-store/custom-store-features#example-4-defining-computed-props-and-methods-as-input, the shown example can easily be solved with the existing functionalities:
interface Person {
firstname: string;
lastname: string;
}
function withEntityVersioner<Entity>() {
return signalStoreFeature(
{ methods: type<{ loader: () => Observable<Entity[]> }>() }, // <-- that's the key (and typesafe)
withState({ version: 1, entities: new Array<Entity>() }),
withMethods((store) => {
return {
update: rxMethod<unknown>(
pipe(
switchMap(() => store.loader()),
filter((entities) => entities !== store.entities()),
tap((entities) =>
patchState(store, (value) => ({
entities,
version: value.version + 1,
}))
)
)
),
};
}),
withHooks((store) => ({ onInit: () => store.update(interval(1000)) }))
);
}
signalStore(
withMethods(() => {
const httpClient = inject(HttpClient);
return {
loader() {
return httpClient.get<Person[]>('someUrl');
},
};
}),
withEntityVersioner() // does not compile
);
@rainerhahnekamp it is indeed possible to use what is currently available, but the DX does not feel good. What if I want multiple withEntityVersioner
? How to group code that should be grouped inside the same block?
IMHO, loader
method should be "inside" withEntityVersioner
, not before:
signalStore(
withFeatureFactory(
(store, httpClient = inject(HttpClient)) =>
withEntityVersioner({
loader: () => httpClient.get<Person[]>('someUrl')
})
)
);
@GuillaumeNury, I understand. The question is whether that use case happens so often that it verifies a new feature.
I could update my example and re-open this issue. What's your opinion on that?
For more advanced features, I find the state/method requirements not very ergonomic. I think that exposing a withFeatureFactory
function allows to keep all internals hidden: cleaner and smaller public API!
The withFeatureFactory
helper can be useful for advanced features that need dynamic access to the store. However, for better DX and full flexibility, dynamic access should be handled on the custom feature side.
Take a look at the implementation of withTabVisibility
feature: https://github.com/ngrx/platform/issues/4272#issuecomment-2227484624
Which @ngrx/* package(s) are relevant/related to the feature request?
signals
Information
This is a proposal for connector function which connects a signalStore to a feature and provides the feature access to store-specific elements.
A StackBlitz example is available here: https://stackblitz.com/edit/ngrx-signal-store-starter-wkuxdh
Sometimes, our Signal Store Features require features from the store they should be plugged in. Since features should be re-usable, direct coupling is impossible.
Following use case. A feature requires access to a loading function which is defined in
withMethods
of the store.We have the following feature
And a store which wants to use it:
The proposed feature is about a function
withFeatureFactory
which connects an autonomous feature to a specific store.The type of
withFeatureFactory
would look like this:This feature would be helpful in the discussion https://github.com/ngrx/platform/discussions/4338.
Although, it is different use case, it is very similar to the suggestion of @gabrielguerrero in https://github.com/ngrx/platform/issues/4314#issuecomment-2101575596
Describe any alternatives/workarounds you're currently using
The alternative is that the
load
function in the example would come from a service which thesignalStoreFeature
has to inject.I would be willing to submit a PR to fix this issue