Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC(@ngrx/signals): Add withProps feature #4504

Open
1 of 2 tasks
markostanimirovic opened this issue Aug 22, 2024 · 6 comments
Open
1 of 2 tasks

RFC(@ngrx/signals): Add withProps feature #4504

markostanimirovic opened this issue Aug 22, 2024 · 6 comments

Comments

@markostanimirovic
Copy link
Member

markostanimirovic commented Aug 22, 2024

Which @ngrx/* package(s) are relevant/related to the feature request?

signals

Information

This feature request suggests adding another SignalStore base feature - withProps. It would allow static properties or observables to be defined as SignalStore members.

export const BooksStore = signalStore(
  withEntities<Book>(),
  withRequestStatus(),
  withProps(({ isFulfilled }) => ({
    fulfilled$: toObservable(isFulfilled).pipe(filter(Boolean)),
  })),
);

It can also be used for defining all dependencies in a single place:

export const MyStore = signalStore(
  withProps(() => ({
    service1: inject(Service1),
    service2: inject(Service2),
  })),
  withMethods(({ service1, service2 }) => ({
    method1() {
      service1.foo();
    },
    method2() {
      service2.bar();
    },
  }))
);

The withProps feature will be useful for library authors to reuse the same config across multiple features:

const SOURCE = Symbol('SOURCE');

function withSource(source: string) {
  return signalStoreFeature(withProps({ [SOURCE]: source }));
}

export const BooksStore = signalStore(
  withSource('BooksStore'),
  withDevtools(),
  withEvents({
    loadedSuccess: props<{ books: Book[] }>(),
    loadedFailure: props<{ error: string }>(),
  })
);

In this example, both withDevtools and withEvents would use the source from the withSource feature.

Signatures

The withProps feature would have 2 signatures:

  1. withProps({ foo: 'bar' })
  2. withProps(() => ({ foo: 'bar' }))

  • The withProps factory function would accept previously defined state signals, computed signals, and properties.
  • withComputed, withMethods, and withHooks would accept previously defined properties.

I would be willing to submit a PR to fix this issue

  • Yes
  • No
@rainerhahnekamp
Copy link
Contributor

Yes, that would be an addition, we definitely should have. Just two questions:

  • Do we now have constraints for the property type, or can it be anything?
  • You stated that the factory function gets the existing slices and computeds. I guess it would also get the previous props? Why do we not pass the methods as well?

@k3nsei
Copy link

k3nsei commented Aug 23, 2024

As suggested something like this last week on discord when talking with @rainerhahnekamp. It would be super useful to open many new capabilities and to avoid hacks with Proxy applied on method in withMethods.

Are there any more details about limitations and where those props would be exposed? I'm hopping to have access to them in withComputed.

@markostanimirovic
Copy link
Member Author

I updated the RFC description with more details:

  • The withProps factory function would accept previously defined state signals, computed signals, and properties.
  • withComputed, withMethods, and withHooks would accept previously defined properties.

@GuillaumeNury
Copy link

This would be useful!

Can props be private the same way state/computed/methods are?

@ducin
Copy link
Contributor

ducin commented Aug 23, 2024

IMO it makes a lot of sense when integrating with libs/things from outside of angular ecosystem, which don't fit any of existing withState, withComputed etc.

The only thing which bothers me is the name - withProps. The "property" word doesn't outline what is the key here - being static. How about withStatic?

@k3nsei
Copy link

k3nsei commented Aug 23, 2024

@ducin thats why I also proposed alternative name withExplicit

Edited: Ahhh it was in other discussion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants