RFC: withFeatureFactory #4922
Replies: 10 comments
-
Hey, I think that's If the team as a whole agrees to move forward with this, here are a few thoughts from my side. NameI'd like to challenge the name. At the moment, everything starting with
One-Parameter LimitationThe one-parameter limitation is not a problem at all. We have the same for TestingFor the tests, make sure to verify it works with the input constraints. If something can go wrong in terms of typings, it usually happens there first. Future of
|
Beta Was this translation helpful? Give feedback.
-
Thank you for this first feedback. NameHere are some other propositions :
TestingYes, it is so easy to lose the incoming input types. |
Beta Was this translation helpful? Give feedback.
-
Another question, should the inner store expose I do not know why it is expose in |
Beta Was this translation helpful? Give feedback.
-
@RomainDood Is it possible create features with generic types using your solution? For example, how would you implement the following feature: export function withSelectedEntity<Entity>(entityMap: EntityMap<Entity>) {
return signalStoreFeature(
withState<SelectedEntityState>({ selectedEntityId: null }),
withComputed(({ selectedEntityId }) => ({
selectedEntity: computed(() => {
const selectedId = selectedEntityId();
return selectedId ? entityMap()[selectedId] : null;
}),
}))
);
}
// store
export const UsersStore = signalStore(
withEntities<User>(),
withFeature(({ entityMap }) => withSelectedEntity(entityMap)),
); |
Beta Was this translation helpful? Give feedback.
-
@markostanimirovic , very interesting case 😉 ! But I will make some more try to find a proper solution for this case. |
Beta Was this translation helpful? Give feedback.
-
Feature with generic typeTo handle features with generic type, I propose something like that: const { selector: selectedEntitySelector, withFeature: withSelectedEntity } =
createGenericFeature(<Entity>(entityMap: Signal<EntityMap<Entity>>) =>
toSignalStoreFeatureResult(
signalStoreFeature(
withState<{ selectedEntityId: string | null }>({
selectedEntityId: null,
}),
withComputed(({ selectedEntityId }) => ({
selectedEntity: computed(() => {
const selectedId = selectedEntityId();
return selectedId ? entityMap()[selectedId] : null;
}),
}))
)
)
);
const testPassingGenericType = signalStore(
withEntities<Book>(),
withSelectedEntity((store) => selectedEntitySelector(store.entityMap)) 👈 I find no way to simplify this part
);
const myStore = new testPassingGenericType();
const result = myStore.selectedEntity; // Signal<Book | null> The const { selector: selectedEntitySelector, withFeature: withSelectedEntity } =
createGenericFeature(<Entity>(entityMap: Signal<EntityMap<Entity>>) =>
toSignalStoreFeatureResult(
withState<{ selectedEntityId: string | null }>({
selectedEntityId: null,
}),
withComputed(({ selectedEntityId }) => ({
selectedEntity: computed(() => {
const selectedId = selectedEntityId();
return selectedId ? entityMap()[selectedId] : null;
}),
}))
)
); I hope their is other way to handle features with generic type. During all my test, it was required to use 2 functions ( Here a reproduction: https://stackblitz.com/edit/stackblitz-starters-31qrd2nq?file=withFeature%2Fhandle-feature-with-generic.ts What do you think ? |
Beta Was this translation helpful? Give feedback.
-
Feature with Generic types proposition 2Another way that is, I think acceptable is something like that: function withSelectedEntity2<
Input extends SignalStoreFeatureResult, // required
Store extends StoreInput<Input>, // required
Entity // Generic type parameter
>(entries: (store: Store) => Signal<EntityMap<Entity>>) {
const entityMap = entries({} as Store);
const feature = signalStoreFeature(
withState<{ selectedEntityId: string | null }>({
selectedEntityId: null,
}),
withComputed(({ selectedEntityId }) => ({
selectedEntity: computed(() => {
const selectedId = selectedEntityId();
return selectedId ? entityMap()[selectedId] : null;
}),
}))
);
// 👇 pass explicitly the types parameters
return toGenericFeature<Input, GenericFeatureResult<typeof feature>>(feature);
}
const Test2FeatureWithGenericStore = signalStore(
withEntities<Book>(),
withSelectedEntity2((store) => store.entityMap)
);
const test2FeatureWithGenericStoreImpl = new Test2FeatureWithGenericStore();
test2FeatureWithGenericStoreImpl.selectedEntity; // Signal<Book | null> This proposition needs to expose some functions and types helper but, it think it is pretty ok to use it like that. Note: the examples does not handle the logic, It just handle the typing part |
Beta Was this translation helpful? Give feedback.
-
@RomainDood good work! That said, I think |
Beta Was this translation helpful? Give feedback.
-
Although it simplifies feature usage: withSelectedEntity((store) => store.entityMap) vs: withFeature((store) => withSelectedEntity(store.entityMap)) Significant complexity is introduced in feature creation: export function withSelectedEntity<
Input extends SignalStoreFeatureResult, // required
Store extends StoreInput<Input>, // required
Entity // Generic type parameter
>(entries: (store: Store) => Signal<EntityMap<Entity>>) {
const entityMap = entries({} as Store);
const feature = signalStoreFeature(
withState<{ selectedEntityId: string | null }>({
selectedEntityId: null,
}),
withComputed(({ selectedEntityId }) => ({
selectedEntity: computed(() => {
const selectedId = selectedEntityId();
return selectedId ? entityMap()[selectedId] : null;
}),
}))
);
// 👇 pass explicitly the types parameters
return toGenericFeature<Input, GenericFeatureResult<typeof feature>>(feature);
} vs: export function withSelectedEntity<Entity>(entityMap: Signal<EntityMap<Entity>>) {
return signalStoreFeature(
withState<SelectedEntityState>({ selectedEntityId: null }),
withComputed(({ selectedEntityId }) => ({
selectedEntity: computed(() => {
const selectedId = selectedEntityId();
return selectedId ? entityMap()[selectedId] : null;
}),
}))
);
} I don't think it's a good candidate to become part of the core package:
Therefore, I'm going to convert this issue to a discussion. We can revisit it if a new proposal with reduced complexity jumps in. |
Beta Was this translation helpful? Give feedback.
-
I understand, I may have something that can works with both using generic types or not. const withSelectedEntity = withFeatureFactory(
<
//👇 List all your generics types
MyGenericTypes extends GenericTypes<['entity']>
>({
entityMap,
}: {
// 👇 Ref the generic types like that
entityMap: Signal<EntityMap<MyGenericTypes['entity']>>;
}) =>
signalStoreFeature(
withState({
selectedEntityId: null as string | null,
}),
withProps((store) => ({
selectedEntity: entityMap[store.selectedEntityId() ?? ''],
})),
withMethods(() => ({}))
)
); The idea is to create a branded type mapper, by using GenericTypes<...>, then instead of resolving generic types to And inside the const TestPassingGenericType = signalStore(
withEntities<Book>(),
withSelectedEntity((store) => ({
entityMap: store.entityMap, // 👈 config Signal<EntityMap<Book>>
}))
);
const myStore = new TestPassingGenericType();
const result = myStore.selectedEntity; // Signal<Book> It will try to make a demo soon, I am close to find a solution (but it takes a lot of times). I do not know all the limitations about this solution |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
Which @ngrx/* package(s) are relevant/related to the feature request?
signals
Information
Hi,
I believe I've found a better approach than the existing
withFeature
function to implement customSignalStore
features that depend on an input value from the store.Currently, to add a custom feature that requires an input from the store, developers must manually use the
withFeature
utility.While this works, it introduces boilerplate and breaks the composability of features, since the custom feature cannot be passed directly into the
signalStore
factory.The goal is to make it possible to use custom
signalStoreFeature
s in the same way as built-in functions likewithState
,withMethods
, etc.This improves readability and results in cleaner, more consistent code.
Here's a comparison between using
withFeature
and the proposedwithFeatureFactory
:The returned type of the inner function must match the type of the first parameter of the feature (in this case,
filteredBooksFeature
).This is fully type-safe and enforced by TypeScript.
However, there is one limitation: the custom feature (like
filteredBooksFeature
) must accept exactly one parameter.Otherwise, the inner function (
withBooksFilter(({ entities }) => entities)
) would have to return an array, which would be semantically strange and less intuitive.To avoid this, I chose to enforce that the feature factory only accepts a single parameter.
If multiple values need to be passed, they can be wrapped in a single object with multiple properties.
Here's a reproduction where I explored different approaches to find the most ergonomic and type-safe solution:
https://stackblitz.com/edit/stackblitz-starters-31qrd2nq?file=withFeature%2F1-simple-only-one-helper-to-use.spec.ts
And I made an article about that (https://dev.to/romain_geffrault_10d88369/ngrx-signalstore-hacks-beautiful-dx-with-custom-features-1n4k)
Describe any alternatives/workarounds you're currently using
No response
I would be willing to submit a PR to fix this issue
Beta Was this translation helpful? Give feedback.
All reactions