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: method - rxMethod without RxJS #4581

Open
1 of 2 tasks
rainerhahnekamp opened this issue Nov 10, 2024 · 14 comments · May be fixed by #4597
Open
1 of 2 tasks

RFC: method - rxMethod without RxJS #4581

rainerhahnekamp opened this issue Nov 10, 2024 · 14 comments · May be fixed by #4597
Assignees

Comments

@rainerhahnekamp
Copy link
Contributor

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

signals

Information

Problem

With Angular introducing resource and rxResource for handling asynchronous tasks, SignalStore could benefit from a similar feature—a standalone rxMethod that operates without a dependency to RxJS: method.

Proposed Solution

The method function, defined as function method<Type>(consumer: (value: T | Signal<T>) => void), would provide a straightforward way to process Signal changes independently of RxJS.

const UserStore = signalStore(
  withState({selectedId: 0}),
  withMethods(store => ({
    setSelectedId: method<number>(selectedId => patchState(store, {selectedId}))
  }))
)

class UserComponent {
    userId = input.required<number>();
    userStore = inject(UserStore);
    
    constructor() {
      this.userStore.setSelectedId(this.userId);
    }
}

Differences to native effect()

It might be tempting to see method as a simple effect:

const UserStore = signalStore(
    // ...
    withMethods(store => ({
      setSelectedId(value: Signal<number>) {
        effect(() => {
          const selectedId = value()
          patchState(store, { selectedId });
        });
      }
    }))
)

However, method offers three distinctive advantages over effect:

  1. Flexible Parameter: The parameter can also be a simple number, not just a Signal.

  2. No Injection Context Required: Unlike an effect, which requires an injection context or an Injector, method can be used directly in an event listener without passing an Injector.

  3. Explicit Tracking: Only the Signal of the parameter is tracked, while other Signals within the consumer function (like anotherSignal() in the example) are not tracked. For example:

    method<number>(value => patchState(store, { counter: value + anotherSignal() }));

Discussion

However, the name method may not clearly convey its purpose, so a more descriptive alternative is worth considering. Possible options include signalMethod.

Describe any alternatives/workarounds you're currently using

Devs could go with a normal effect(). This would limit the use case of method to only Signals as inputs.

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

  • Yes
  • No
@aboudard
Copy link

First, great improvement !
Even if method would be in my opinion clear enough, I feel very uncomfortable with it, mostly because it just looks like native language element (method of a class).
So that would be my humble contribution, everything except method.
Reflecting rxMethod, signalMethod would be nice.

@rainerhahnekamp
Copy link
Contributor Author

@aboudard

looks like native language element (method of a class).

Exactly, I don't have the best feeling about the name either.

@Tiime-LcsGa
Copy link

Tiime-LcsGa commented Nov 14, 2024

I may have missed something so what would be the difference/benefit over this?

const UserStore = signalStore(
  withState({selectedId: 0}),
  withMethods(store => ({
    setSelectedId: (selectedId: number) => patchState(store, {selectedId})
  }))
)

class UserComponent {
    userId = input.required<number>();
    userStore = inject(UserStore);
    
    constructor() {
      this.userStore.setSelectedId(this.userId);
    }
}

EDIT: If I'm correct, the difference would be with effect right? But then, wouldn't these use cases be handled by the incoming events, with withReducer and withEffects ?

@aboudard
Copy link

EDIT: If I'm correct, the difference would be with effect right? But then, wouldn't these use cases be handled by the incoming events, with withReducer and withEffects ?

Yeah, I might be wrong but your example would not be triggered again when the userId would change, right ?

@LcsGa
Copy link

LcsGa commented Nov 15, 2024

EDIT: If I'm correct, the difference would be with effect right? But then, wouldn't these use cases be handled by the incoming events, with withReducer and withEffects ?

Yeah, I might be wrong but your example would not be triggered again when the userId would change, right ?

Okay I think I understand now! Is the goal triggering the method without calling the method?
Isn't it deflecting the withMethod feature?
Since events are coming in @ngrx/signals wouldn't it be their goal to handle these use cases?

@rainerhahnekamp
Copy link
Contributor Author

@Tiime-LcsGa, your example would throw a compilation error because you are passing a Signal as parameter to a function that accepts only number. method/signalMethod would fix that.

It is not a SignalStore feature. So not in the same group as withMethod or withReducer, etc. It is a utility function

@LcsGa
Copy link

LcsGa commented Nov 17, 2024

@Tiime-LcsGa, your example would throw a compilation error because you are passing a Signal as parameter to a function that accepts only number. method/signalMethod would fix that.

It is not a SignalStore feature. So not in the same group as withMethod or withReducer, etc. It is a utility function

I missed the fact that userId is a signal... 😅
I see what you mean when you're saying that it's a utility function but why wouldn't you do this instead?

const UserStore = signalStore(
  withState({selectedId: 0}),
  withMethods(store => ({
    setSelectedId: (selectedId: number) => patchState(store, {selectedId})
  }))
)

class UserComponent {
    userId = input.required<number>();
    userStore = inject(UserStore);
    
    constructor() {
      effect(() => this.userStore.setSelectedId(this.userId()));
    }
}

What are the benefits of this utility function?
I mean, Iwouldn't mind having method/signalMethod at all, but I feel like it is a little """"hacky"""" (an effect running under the hood)

@rainerhahnekamp
Copy link
Contributor Author

rainerhahnekamp commented Nov 17, 2024

@LcsGa,

(an effect running under the hood)

Oh, I'd say that this will become the way to go. effect will be a low-level function and we get utility functions for specific use cases that run effect internally. You might want to read the blog article from Angular about the future of the effect. I'll quote

Instead, we’ll focus on other ways of encouraging good patterns, including the addition of new reactivity helpers where needed.

https://blog.angular.dev/latest-updates-to-effect-in-angular-f2d2648defcd

...and since you asked what signalMethod brings in additionally. That's mentioned in the PR:

However, method offers three distinctive advantages over effect:

Flexible Parameter: The parameter can also be a simple number, not just a Signal.

No Injection Context Required: Unlike an effect, which requires an injection context or an Injector, method can be used directly in an event listener without passing an Injector.

Explicit Tracking: Only the Signal of the parameter is tracked, while other Signals within the consumer function (like anotherSignal() in the example) are not tracked. For example:

@LcsGa
Copy link

LcsGa commented Nov 17, 2024

@rainerhahnekamp thanks for all of your explanations! I get way better why you proposed this now!

I think what "bothered" me here is while rxMethod is a wrapper around a subject.next(...) + its subscription, unlike with method the "normal way" of using it is not by passing an observable as the argument.

=> Currently rxMethod is just a basic "notifier" that executes the operator chain you've passed to it.

=> With method the "normal way" would be to pass a signal (like an observable) and when the argument's value changes (outside of the store itself), the method gets recalled (under the hood).

This is what I found hacky...
On the other hand, I agree that it's something we would all benefit from, I'm just not sure if withMethod is the right tool/place to do so

rainerhahnekamp added a commit to rainerhahnekamp/ngrx that referenced this issue Nov 17, 2024
signalMethod is a function factory to process side effects on Signals or
static values.

It is similar to rxMethod but does not support RxJS.

signalMethod follows Angular’s pattern of RxJS-less utilities for Signals,
like resource and rxResource.

`signalMethod` expects a type and processor
function:

```typescript
const doubleLogger = signalMethod<number>(value => console.log(value * 2))

const value = signal(1);
doubleLogger(value); // tracks value and executes initially and on every change
```

It also supports static values, e.g., `doubleLogger(1)`.
rainerhahnekamp added a commit to rainerhahnekamp/ngrx that referenced this issue Nov 17, 2024
`signalMethod` is a factory function to process side effects on Signals or
static values.

It is similar to `rxMethod` but does not support RxJS.

`signalMethod` follows Angular's pattern of RxJS-less utilities for Signals,
like `resource` and `rxResource`.

`signalMethod` expects a type and processor function:

```typescript
const doubleLogger = signalMethod<number>(value => console.log(value * 2))

const value = signal(1);
doubleLogger(value); // tracks value and executes initially and on every change
```

It also supports static values, e.g., `doubleLogger(1)`.
rainerhahnekamp added a commit to rainerhahnekamp/ngrx that referenced this issue Nov 17, 2024
`signalMethod` is a factory function to process side effects on Signals or
static values.

It is similar to `rxMethod` but does not support RxJS.

`signalMethod` follows Angular's pattern of RxJS-less utilities for Signals,
like `resource` and `rxResource`.

`signalMethod` expects a type and processor function:

```typescript
const doubleLogger = signalMethod<number>(value => console.log(value * 2))

const value = signal(1);
doubleLogger(value); // tracks value and executes initially and on every change
```

It also supports static values, e.g., `doubleLogger(1)`.
@rainerhahnekamp rainerhahnekamp linked a pull request Nov 17, 2024 that will close this issue
3 tasks
@rainerhahnekamp
Copy link
Contributor Author

@LcsGa you can now check out the PR and especially the tests where you can see its usage. You'll see that there is no withMethods involved.

@LcsGa
Copy link

LcsGa commented Nov 17, 2024

Okay I get it now! I don't know why but as I read the first message you wrote there, I thought it was only usable in withMethods... It's a great addition then!! 👌🏼

@LcsGa
Copy link

LcsGa commented Nov 17, 2024

I just read the implementation of rxMethod. Wouldn't it be better to improve it by adding an overload to be able to also pass in a basic function (like your solution) instead of an operator?
The reason why I ask is because I saw that the returned function of rxMethod can either be called with a basic value, a signal or an observable which is the main purpose (with a signal) of your solution.
Moreover the signalMethod is as reactive as rxMethod so the rx prefix would be good for both signatures.

@rainerhahnekamp
Copy link
Contributor Author

Well, looks like I didn't describe it enough in the RFC:

With Angular introducing resource and rxResource for handling asynchronous tasks, SignalStore could benefit from a similar feature—a standalone rxMethod that operates without a dependency to RxJS: method.

With rxMethod you have a dependence to RxJS. Angular is going RxJS-optional. signalMethodis the "rxMethod without RxJS".

@LcsGa
Copy link

LcsGa commented Nov 18, 2024

Well, looks like I didn't describe it enough in the RFC:

With Angular introducing resource and rxResource for handling asynchronous tasks, SignalStore could benefit from a similar feature—a standalone rxMethod that operates without a dependency to RxJS: method.

With rxMethod you have a dependence to RxJS. Angular is going RxJS-optional. signalMethodis the "rxMethod without RxJS".

Okay I get everything now! 👌🏼

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

Successfully merging a pull request may close this issue.

4 participants