ngrx / platform

Reactive State for Angular
https://ngrx.io
Other
8.01k stars 1.97k forks source link

RFC: Add a reactivity layer to signalStore (withEvents, withReducer, and withEffects) #4408

Open MikeRyanDev opened 3 months ago

MikeRyanDev commented 3 months ago

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

signals

Intro

signalStore provides the basis for a reusable state container built on top of Angular Signals. This RFC aims to enhance signalStore with a reactivity layer.

Architecture The changes in this RFC are built on the following principles:

In @ngrx/{store,effects}, we model the above principles using:

This RFC extends signalStore with the primitives necessary to implement a reactive architecture:

Goals/NoGoals

This RFC intends to:

This RFC does not:


APIs

Events

A signalStore can define the events that it emits:

Defining Events

import { signalStore, withEvents } from '@ngrx/signals';

export const Store = signalStore(
  withEvents({
    canvasReady: (canvas: HTMLCanvasElement) => canvas,
    canvasLeftClick: (point: Point) => point,
    canvasRightClick: (circle: Circle | null) => circle,
    updateRadius: (circle: Circle, radius: number) => ({ circle, radius }),
    closeRadiusOverlay: () => ({}),
  }),
);

Emitting an Event

The inner store exposes an emit function that methods may call to emit an event. It borrows function signatures from the event definitions:

signalStore(
  withMethods((store) => ({
    init(canvas: HTMLCanvasElement) {
      store.emit('canvasReady', canvas);
    },
    redo() {
      store.emit('redo');
    },
    undo() {
      store.emit('undo');
    },
  })),
)

Listening for an event

signalStore instances will expose an on(...) method that returns an Observable of that event's data payload:

store.on('canvasLeftClick').subscribe(event => console.log(event));

Reducers

Reducers for managing state in response to events can be registered with the withReducer(...) feature. Unlike reducers from @ngrx/store, these reducers can either return the new state, a function that updates the state, or an array of functions that update state:

const Store = signalStore(
  withState({
    circles: [],
    activePoint: null,
  }),
  withReducer((state, event) => {
    switch (event.type) {
      case 'canvasLeftClick': {
        return {
          ...state,
          circles: [
            ...state.circles,
            { x: event.payload.x, y: event.payload.y, radius: 10 },
          ],
        };
      }

      default: {
        return state;
      }
    }
  }),
);

Effects

Developers can define long-running effects using observables, async iterators, promises, and functions using the withEffects feature:

const Store = signalStore(
  withEvents({
    canvasReady: (canvas: HTMLCanvasElement) => canvas,
    canvasLeftClick: (point: Point) => point,
  }),
  withEffects((store) => ({
    handleLeftClick$: store.on('canvasReady').pipe(
      switchMap((event) => {
        return fromEvent<MouseEvent>(event.payload, 'click');
      }),
      tap((event) => {
        store.emit('canvasLeftClick', { x: event.clientX, y: event.clientY });
      })
    ),
  })),
);

Event -> State Change

This RFC recommends changing patchState(...) to emit an event like @patch-state that uses an underlying reducer to update the state. This ensures that end developers can reap the benefits of event-driven state management, even if they opt to exclusively use patchState and withMethods. A reactive core for signalStore enables use cases like undo/redo, state change aggregation, and integration with the Redux Devtools browser extension.

Direct Dependence on RxJS

This RFC introduces a direct dependency between signalStore and RxJS. This matches the intent of NgRx: merging Angular and RxJS more deeply. However, it does not require that developers consume the reactive APIs or know how to use RxJS to use signalStore.

Breaking Changes

This will introduce breaking changes to signalStore in situations where developers have already used withState(...), withComputed(...), or withMethods(...) to define properties on signalStore that conflicts with this new prop:

Additionally, type information about events must be stored on StoreFeature, introducing a breaking change for any third-party libraries shipping their own store features.

Examples

Circle Drawer

The following implements Circle Drawer from 7GUIs. It demonstrates event-driven state management and side effects for a local component

Describe any alternatives/workarounds you're currently using

No response

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

MikeRyanDev commented 3 months ago

Effects alternative:

Effects

An effect can be registered using the withEffect feature. An optional eventName can be supplied for an effect, which connects the output of the effect to the event with the same name.

As an event:

signalStore(
  withEvents({
    loadTodos: emptyProps(),
    loadTodosSuccess: props<{ todos: Todo[] >(),
  }),
  withEffect('loadTodosSuccess', (store, http = inject(HttpClient) =>
    store.on('loadTodos').pipe(
      exhaustMap(() => this.http.get<Todo[]>('/todo')),
      map(res => ({ todos: res.data })),
    )
  )
);

As a silent effect:

signalStore(
  withEvents({
    loadTodos: emptyProps(),
    loadTodosSuccess: props<{ todos: Todo[] >(),
  }),
  withEffect((store, http = inject(HttpClient) =>
    store.on('loadTodosSuccess').pipe(
      tap(data => localStorage.setItem('todos', JSON.stringify(data.todos)))
    )
  )
);
manfredsteyer commented 3 months ago

That looks interesting. I like it.

Two things come to mind:

Effects with on-function

I'd love to have the option to use the on-function instead of switching to withReducer.

Broadcasting Events to all stores

It would be nice if we could broadcast an event to all stores. This is because a Signal Store can often be compared to a feature slice in ngrx/store. In most cases, we know which store should receive the event so we can maintain good action hygiene. But sometimes, it might be handy to just send out an event and let the stores decide what to do with it.

Another argument is that this is also possible today in ngrx/store. Also, sending out an event without knowing who and how many subscribers are interested is IMHO one key to eventing.

anthony-b-dev commented 3 months ago

I'm a fan of adding the redux pattern into signalStore. As an inexperienced dev, I'm a little confused by the heavy RxJS use in a signal based API, though. Seems a little counterintuitive. Especially needing to listen to events through an observable. My thought is that we should use composition through various computed() methods to keep everything inside signal-land as much as possible. This is more like selectors and would translate better.

I've used ngrx/store in projects before and I would be able to translate the concepts better if we kept on() syntax in the reducers.

Why the change from dispatch + actions to emit + events?

Maybe this is a future version thing, but the helper methods to create actions, action groups, feature creators, etc.. would also be great to have for the signal store.

Overall I love that this is being considered!

MikeRyanDev commented 3 months ago

@anthony-b-dev I'm a little confused by the heavy RxJS use in a signal based API, though. Seems a little counterintuitive. Especially needing to listen to events through an observable

Yeah, I can understand why it's confusing! Signals are a great reactive primitive for state values but don't make much sense for events. Pushing a value into a signal does not guarantee that the consumer will receive an update. Check out this example and notice how few times the observable subscription or the computed function is called relative to the number of times the count() signal changes.

For this reason, signals are a bad reactive primitive for events (you wouldn't want to miss a click!). The approach laid out in this RFC embraces signals for state and observables for events until better primitives are available for either.


@anthony-b-dev I've used ngrx/store in projects before and I would be able to translate the concepts better if we kept on() syntax in the reducers.

@manfredsteyer I'd love to have the option to use the on-function instead of switching to withReducer.

I, too, would prefer to use the on function like this:

withReducer(
  on('loadTodos', (state, event) => { ... }),
)

However, unlike props and emptyProps, we wouldn't be able to use the same on function exported by @ngrx/store. I don't want to have two identically named functions with different uses across @ngrx/store and @ngrx/signals. Do either of you have an idea here for the design of this API?


@manfredsteyer It would be nice if we could broadcast an event to all stores.

@anthony-b-dev Why the change from dispatch + actions to emit + events?

In my opinion, that's the purpose of @ngrx/store and @ngrx/effects. I'd encourage reaching for the global Store and Actions when you need a global event stream. I've used the terms emit and event for signal store to disambiguate between the global Store (from @ngrx/store) and local stores (from @ngrx/signals).

I could imagine a future version of @ngrx/store shipping with its own feature for signalStore that makes it easy to map signal store events into actions. 👀

manfredsteyer commented 3 months ago

@anthony-b-dev Falling back to the global store is definitely an option. But then, I need two stores. I'd love to use the Signal Store's lightweight approach in the whole application, especially when there are only a few situations where I need a global cross-feature event.

I have a further argument: If we directly wire events to the store, there is no huge difference between dispatching an event and calling a store method.

tomastrajan commented 3 months ago

Would be amazing, events ARE necessary, the "method" approach of the signal store felt like a step back which doesn't support complex use cases without circular deps between individual signal stores.

gabrielguerrero commented 3 months ago

One approach that I was considering the other day, I think we could make ngrx compatible with @ngrx/signals, what if we keep using the regular actions, and withReducer could easily accept the same payload of createReducer, and withEffects can be as suggested. So, to summarise, we reuse the actions and part of the reducers logic , I am not sure if part of the ngrx effects could also be reused; this logic could be extracted to its own packages and shared by ngrx and ngrx-signals. This will also ease the migration path for users trying to move from ngrx to ngrx-signals (or making them coexist). I think the only part of ngrx that doesnt port easily is selectors, so people will need to transform them to computed manually.

markostanimirovic commented 3 months ago

Would be amazing, events ARE necessary, the "method" approach of the signal store felt like a step back which doesn't support complex use cases without circular deps between individual signal stores.

Both approaches excel in different use cases. The "method" approach is far away from stepping back in my opinion. Together with the overall SignalStore design, it provides flexibility and extensibility. Therefore, it's already possible to implement a Redux-like/event-based style on top of it. 😉 https://github.com/angular-architects/ngrx-toolkit?tab=readme-ov-file#redux-withredux

tomastrajan commented 3 months ago

Would be amazing, events ARE necessary, the "method" approach of the signal store felt like a step back which doesn't support complex use cases without circular deps between individual signal stores.

Both approaches excel in different use cases. The "method" approach is far away from stepping back in my opinion. Together with the overall SignalStore design, it provides flexibility and extensibility. Therefore, it's already possible to implement a Redux-like/event-based style on top of it. 😉 https://github.com/angular-architects/ngrx-toolkit?tab=readme-ov-file#redux-withredux

100% true, was just a bit impulsive expression of frustrations when initially wrapping my head around and working with plain signalStore as provided out of the box. With new default being methods instead of events, it felt like a step back in a way that it optimizes for simpler use cases (which might be more common) and would lead to a tangled dep graphs with circular deps in projects at the scale I encounter the most, eg stores importing each other to call their methods.

KevTale commented 3 months ago

I'm very much in favor of this. Event driven approach is the key that was blocking me to use signalStore.

As mentioned by someone above, I wish each signalStore could listen to the others though. I understand the global store would be the solution instead but that would be a shame to install another state manager (ngrx/store) just to handle the rare use cases where we need global stores.

I would rather have a single opiniated solution that handles both feature store and global store, signalStore seems a perfect fit! That's less things to learn for new comers and less overhead in general.

mfp22 commented 3 months ago

@tomastrajan is exactly right:

If we directly wire events to the store, there is no huge difference between dispatching an event and calling a store method.

There's a great NgConf talk about action hygiene covering this.

What you need for reactivity is to allow a signal store to react to events from another signal store then, or just any observables. Then people can create signal stores or observables for events and import them into other stores. Maybe withSources that you can feed an object into it that has keys that match method names and values that are observables that trigger those methods. There's nothing special about reducers if the code is located in the same place with or without them. I guess pure functions would be nice. But then how about withAdapter for those? And effects are not reactive at all. It would be better to define the HTTP requests as observables you can react to like any other event and refer to it directly.

And then if you could decouple the store lifecycle from the injector hierarchy you can have stores shared between multiple components without premature injection in a parent/ancestor injector. Good for lazy loading both code and data. Create a function to inject stores specifically that also subscribes to all the underlying sources to trigger the HTTP calls at that time, and then unsubscribe when all are destroyed. Like TanStack Query, or simply how RxJS should be used.

Basically I'm describing the implementation of StateAdapt. If you really want to make something both minimal and reactive, it's going be something that looks like what I did there. I'm happy to collaborate if there's any interest. If signal store could evolve to handle reactivity like StateAdapt, I'd love that. It satisfies all the principles of unidirectionality, but better because RxJS is more unidirectional than actions and effects. It also still shows up in Redux DevTools in a proper action hygiene way.

I never wanted to create a state management library. The only reason I created StateAdapt is because I thought I would have to work up some NgRx contributor ladder before ever being taken really seriously, and it would take more work to convince people of the benefits of a fully reactive solution without concrete examples to show how eaay this all could be. And then even if that failed, I could just keep promoting it as an alternative. Maybe I should have tried harder to work with NgRx. StateAdapt was originally just a "dynamic state" inside NGXS and then a "dynamic reducer" within NgRx actually.

mfp22 commented 3 months ago

I never wanted to create a state management library.

Note to anyone reading afraid I'm going to drop StateAdapt support or something: I won't stop pushing StateAdapt aggressively until NgRx implements similar syntax, and then if that day comes, I'll write a migration script myself for any syntax differences. And then still push it in other frameworks.

MikeRyanDev commented 3 months ago

@manfredsteyer I have a further argument: If we directly wire events to the store, there is no huge difference between dispatching an event and calling a store method.

jack-nicholson-nodding-yes.gif

In all seriousness, I'm thinking of this similarly to React's useState/useReducer hooks. Most developers will have a happy time using signalStore with patchState and withMethods. Once the complexity of the problem necessitates the complexity of the event-driven pattern, you've got access to withEvents and withReducer.


@tomastrajan Would be amazing, events ARE necessary, the "method" approach of the signal store felt like a step back which doesn't support complex use cases without circular deps between individual signal stores.

I love the methods approach, especially with the rxMethod helper. I hope signalStore covers various use cases, and devs can reach for events and effects when methods stop scaling up.


@gabrielguerrero One approach that I was considering the other day, I think we could make ngrx compatible with @ngrx/signals, what if we keep using the regular actions, and withReducer could easily accept the same payload of createReducer, and withEffects can be as suggested.

My concern is how syntactically noisy this would be. @ngrx/store's reducer API is optimized for writing reducers that get registered globally. I think we can make the code much more compact for signalStore and reserve @ngrx/store's reducer API for the global side.


@KevTale I would rather have a single opiniated solution that handles both feature store and global store, signalStore seems a perfect fit!

You and me both 🙂 I want to get there, but it'll be incremental.

valiant-code commented 3 months ago

I was a huge fan of SignalStore for the main reason that it didn't encourage redux patterns, it seems like so much complex code that ends up being a pain to trace for very little value to me. There are a lot of people that seem to like this proposal, so maybe there are actually good use cases I haven't seen (or maybe people just like what they are already familiar with, including me). But I just question if it's worth complicating the package for this, seems like a fair bit of additional cognitive load to understand. Keep It Simple and all that. Just my opinion, I recognize it would still be optional.

In an attempt to be actually constructive, I'd ask if there's anyway to avoid having to pass the event names as a string? Seems clunky and part of why I called it hard to trace what is actually happening in redux. store.documentMouseup.emit() instead of store.emit('documentMouseup'))

danielglejzner commented 3 months ago

So this is why I would focus on improving before going through with it.

  1. Proposed reactivity layer is not unidirectional like ngrx store

  2. Not sure if that was a goal at all but would be nice. However currently it doesn't help migrate to ngrx store

  3. It seems redundant with current signal store APIs.

  4. With proposed changes applied , devs are going to have a hard to know which one to use.

  5. withReducer does the same thing as withMethods but with worse syntax + not even like ngrx store.

  6. withEvents is like createActionGroup but still different and the events are stuck in the store and there's no way to move them out and still use them in the store.

  7. withEffects is just side effects but withMethods already supported that too.

In my personal opinion to make the RFC good everything about it would have to be different.

MikeRyanDev commented 3 months ago

@valiant-code Keep It Simple and all that. Just my opinion, I recognize it would still be optional.

The docs will encourage devs to keep it simple, too. Again, this is like useState vs useReducer in React land. 95% of signalStores won't need to tap into these reactive APIs. They'll have access to them when they'd benefit from the aggregation of state change logic that a reducer provides.


@valiant-code I'd ask if there's anyway to avoid having to pass the event names as a string? Seems clunky and part of why I called it hard to trace what is actually happening in redux. store.documentMouseup.emit() instead of store.emit('documentMouseup'))

Yeah, this would be possible. Both can be strongly typed. Also, unlike Redux or @ngrx/store, this RFC intentionally colocates the event definitions and reducers in the same store definition. I'm not sure what your experience has been, but the indirection of @ngrx/store leads to hard-to-trace implementations. This is meant to be a lot less indirect. I want these APIs to empower developers to build small, well-encapsulated state machines.

markostanimirovic commented 3 months ago

Thanks everyone for your feedback! Before I share mine, I want to point out two very important things to avoid any potential confusion:


A few months ago, I shared my thoughts on supporting the Redux-like approach in the SignalStore in this tweet.

The idea is to define actions/events outside of the SignalStore:

export const BooksStore = signalStore(
  { providedIn: 'root' },
  withEntities<Book>(),
  withRequestStatus(),
  withReducer(
    on(DashboardPageActions.opened, setPending),
    on(BooksApiActions.booksLoadedSuccess, ({ books }) => [
      setAllEntities(books),
      setFulfilled(),
    ]),
    on(BooksApiActions.booksLoadedError, ({ error }) => setError(error)),
  ),
);

export const AuthorsStore = signalStore(
  { providedIn: 'root' },
  withEntities<Author>,
  withRequestStatus(),
  withReducer(
    on(DashboardPageActions.opened, setPending),
    on(AuthorsApiActions.authorsLoadedSuccess, ({ authors }) => [
      setAllEntities(authors),
      setFulfilled(),
    ]),
    on(AuthorsApiActions.authorsLoadedError, ({ error }) => setError(error)),
  ),
);

Actions can be dispatched to the Dispatcher service provided at the root level:

@Component({
  template: `
    <h1>Library Dashboard</h1>

    <book-list
      [books]="booksStore.entities()"
      [isPending]="booksStore.isPending()"
    />

    <author-list
      [authors]="authorsStore.entities()"
      [isPending]="authorsStore.isPending()"
    />
  `
})
export class DashboardComponent implements OnInit {
  readonly #dispatcher = inject(Dispatcher);
  readonly booksStore = inject(BooksStore);
  readonly authorsStore = inject(AuthorsStore);

  ngOnInit(): void {
    this.#dispatcher.dispatch(DashboardPageActions.opened());
  }
}

By decoupling actions from the SignalStore, we can listen to the same event in multiple SignalStores. Also, we can reuse existing action creator APIs:

const DashboardPageActions = createActionGroup({
  source: 'Dashboard Page',
  events: {
    opened: emptyProps(),
  },
});

Effects can be defined similarly to 'traditional' NgRx effects:

export const BooksStore = signalStore(
  { providedIn: 'root' },
  withEntities<Book>(),
  withRequestStatus(),
  withReducer(
    on(DashboardPageActions.opened, setPending),
    on(BooksApiActions.booksLoadedSuccess, ({ books }) => [
      setAllEntities(books),
      setFulfilled(),
    ]),
    on(BooksApiActions.booksLoadedError, ({ error }) => setError(error)),
  ),
  withEffects((store, dispatcher = inject(Dispatcher), booksService = inject(BooksService)) => ({
    loadBooks: dispatcher.on(DashboardPageActions.opened).pipe(
      exhaustMap(() =>
        booksService.getAll().pipe(
          mapResponse({
            next: (books) => BooksApiActions.booksLoadedSuccess({ books }),
            error: (error) => BooksApiActions.booksLoadedError({ error }),
          })
        )
      )
    ),
  }))
);

Installing @ngrx/store and @ngrx/effects packages in a project just to use APIs such as createAction, createActionGroup or ofType doesn't seem like a good idea to me, but then the question is how to avoid API duplication in NgRx packages?

For action creators, I like the suggestion from @mikezks to create the @ngrx/actions package and move these APIs there.

The ofType operator can be moved to the @ngrx/operators package.


Last but not least, I'd create a sub-package/plugin for event-driven/Redux-like APIs within the @ngrx/signals package. Of course, I'm open to discussion - if we realize that some things cannot (easily) be done this way and by integrating into the core package we can achieve much better results in terms of reactivity.

gabrielguerrero commented 3 months ago

What @markostanimirovic posted is the ideal solution for me. I even attempted to do it but I stopped because I found myself duplicating quite a bit of ngrx code, and that's why I suggested maybe making a bit of refactoring to extract the common bits. Using the common bits of ngrx not only will help migration efforts but also will help to make ngrx devs quickly productive due to the familiar api

danielglejzner commented 3 months ago

What @markostanimirovic posted its the ideal solution for me, I even attempted to do it but I stopped because I found myself duplicating quite a bit of ngrx code, and is why I suggested maybe making a bit of refactoring to extract the common bits. Using the common bits of ngrx not only will help migration efforts but also will help to make ngrx devs quicly productive due to the faimilar api

Definitely makes much more sense than original idea

brandonroberts commented 3 months ago

@danielglejzner as @markostanimirovic mentioned, we are looking for feedback, positive and negative, about the technical details of the RFC. Not just comments for the sake of social media engagement.

Either stay on topic or you will be restricted from interacting further.

danielglejzner commented 3 months ago

@brandonroberts i think you missed or did not read my feedback here where I pointed on how to improve what has been proposed in the RFC.

Please refer to my other comment with a list of things.

Marko even addressed some of what I have pointed out.

Please tell me where is the problem you see? :)

MikeRyanDev commented 3 months ago

@markostanimirovic I'm specifically not looking to solve global eventing with this proposal, and am more specifically looking at local eventing. I will take @alex-okrushko's suggestion and update the RFC to follow a more traditional design doc structure with specific goals/no-goals and illustrative examples. I'll fold in your feedback and update the doc. 👍

mfp22 commented 3 months ago

With events, any chance of Redux DevTools visibility for signal stores?

And if events can't be treated as events and are just extra decoupling between components and state changes, is that a sacrifice on DX to enable

well-encapsulated state machines

?

Does the future of NgRx look more like XState?

malek-itani commented 3 months ago

it's amazing to see the redux pattern added for signalStore, especially if it has a similar API as ngrx/store it allows Devs that are already using the ngrx/store to adopt faster signalStore with its new reactive layer

gabrielguerrero commented 3 months ago

@markostanimirovic @MikeRyanDev, I have been thinking, what if we could solve both local and global events? maybe besides the Dispatcher which is global was there also a LocalDispatcher ? or something similar I think there are many ways to implement it api wise, but the idea is the dev can choose to do local vs global depending on their needs

rainerhahnekamp commented 3 months ago

I think we "derailed" here a little bit. Most of us are talking about Redux, but I don't have the impression that this is what @MikeRyanDev had in mind.

I will take @alex-okrushko's suggestion and update the RFC

I have a lot of questions and comments, but I will wait for the update.

abhijit-chikane commented 3 months ago

I only see one issue with current signal store which is injection of stores into one another is not possible as it creates cyclic dependency but if there is a way to achieve this without adding redux overhead that would be much helpful. I also like the idea of creating plugin for redux style apis to incorporate it into signal store if that helps the people who wanted to use goodies of the both worlds.

any which way I like signal store concept overall. As it doesn’t need too much boilerplate and it just work seamlessly. Already started using it in my project and looking forward to seeing how this turns up with my project.

Thanks and happy coding🍾

MikeRyanDev commented 3 months ago

As promised, the RFC has been updated to better communicate the intent of these APIs. To state it clearly:

abhijit-chikane commented 3 months ago

I would rather say we will get more benefits if we implement more features around the singalStore like this withRequestStatus('someApiStatus') store feature and some build in functions to create the deepSignals something more useful and core to the signal fun I have implemented this in the below sample project

https://idx.google.com/signal-store-demo-5061337

rainerhahnekamp commented 3 months ago

@MikeRyanDev, the clarification was gold. Could you maybe even include in the description (perhaps as the first line) that this RFC is not about Redux and the communication between SignalStores? This clarification might help eliminate a majority of the existing misunderstandings.

Official Redux Extension

Please let me know if you plan to create a separate, official Redux extension. Otherwise, we’ll continue to work on the community withRedux extension.

Concerns about exposing events as Observables

I have some doubts about whether SignalStore is the best tool to implement for your use case. Here’s my understanding of the typical process:

1.  A DOM event is triggered in the component.
2.  The component passes the event to the store.
3.  The store processes the event and changes the state.
4.  The component is notified by the state change and re-renders.

The SignalStore notifies the component via a signal. This glitch-free effect enhances reactivity by ensuring that intermediate steps are not processed, as they will not be rendered. Your example on StackBlitz actually shows that quite nicely. You want the computation to run only against the final value, not with every increment.

Events are necessary for data entering the store. For that, we already have the rxMethod.

withEvents as edge case

Therefore, I see withEvents as an edge case.

NgRx has a history of being careful not to add too many features and enforce a certain style.

Reintroducing the possibility of observables into a store, which has signals in its name and is part of a framework clearly oriented towards signals, might lead developers to revert to old styles and not fully utilize signals.

Looking at the specs of the Circle Drawer, I think it might be possible to implement it without withEvents. I would have to try it out to be sure.

Potential Use Cases

We should explore, if there are further use cases where glitch-free operation might cause problems.

For instance, when one SignalStore depends on another it has to run side effects when the dependency’s state changes.

However, in those applications where we migrated already from the global store, glitch-free didn't cause an issue.

Final Note

If this RFC goes through, please ensure that developers clearly understand its intended use case to avoid potential misapplication.

Cheers!

MikeRyanDev commented 3 months ago

@rainerhahnekamp This is excellent feedback. Thank you! Let me try to get some answers:

Please let me know if you plan to create a separate, official Redux extension. Otherwise, we'll continue to work on the community withRedux extension.

If this RFC goes through, it's trivial for us to support an official Redux dev tools extension. Maybe you and I can collaborate on this directly in NgRx? 😃

I have some doubts about whether SignalStore is the best tool to implement for your use case.

This RFC does not change SignalStore's state semantics. All state is still contained inside signals. This RFC's opinion is that observables are still king for handling and orchestrating events. I hope you agree that it's right for NgRx to still have a bit of Rx in it. 😃

Here's an updated counter example built out with a prototype of the reactive signalStore. Note that the update semantics are identical to the previous Stackblitz link I shared, which is expected since all of the state is still managed with Signals.

Here's a twist of the above counter example that doesn't use withReducer/withEvents and instead uses withMethods/patchState. However, since this version of signalStore is using an event sourcing pattern internally, it can still leverage the withHistory(...) feature I shared in the Circle Drawer example. That's a pretty cool unlock for signalStore developers!

Therefore, I see withEvents as an edge case.

Agreed! This supports advanced use cases in userland, and might be more popular with library authors creating new features for signalStore.

NgRx has a history of being careful not to add too many features and enforce a certain style.

I don't know if I necessarily agree. When @ngrx/store was new, there was a flurry of experimentation. Once it matured, care was taken during the long-term maintenance of the package to keep the API surface stable and focused. Signals are new! Let's experiment.

Reintroducing the possibility of observables into a store, which has signals in its name and is part of a framework clearly oriented towards signals, might lead developers to revert to old styles and not fully utilize signals.

I disagree with the general community sentiment that signals eliminate the need for observables. In all cases where developers used observables to manage state, yes, that's eliminated. No more behavior subjects, only signals. But for events? There's no comparison.

If this RFC goes through, please ensure that developers clearly understand its intended use case to avoid potential misapplication.

Yeah, this RFC already shows how confusing this will be for the community. If this lands, we'll figure out messaging, too.

MikeRyanDev commented 3 months ago

@mfp22 Does the future of NgRx look more like XState?

Not really, but bringing an XState state machine into signalStore would be much easier with these APIs!

MikeRyanDev commented 3 months ago

Also @mfp22, instead of thinking about how to make signalStore like your library (StateAdapt), the question I want to ask is: what primitives need to exist inside of signalStore for you to make StateAdapt work on top of it? It seems like you were able to achieve it on top of the infra we provided in @ngrx/{store,effect}.

mfp22 commented 3 months ago

@MikeRyanDev this RFC intentionally colocates the event definitions and reducers in the same store definition. I'm not sure what your experience has been, but the indirection of @ngrx/store leads to hard-to-trace implementations.

I have wrestled with this for 7 years. I understand the difficulty. But I still want the core benefit of the Flux pattern: Reduce bugs from scattered state control. This RFC looks like a big pivot in NgRx's direction away from its roots in Redux and Flux. Is it the only way to address indirection?

NgRx/Store indirection has 2 components:

  1. Boilerplate in many files to jump across
  2. Reactivity

The key to reducing the boilerplate is realizing that Flux's unidirectionality is just a form of reactivity. You can reimplement Flux with RxJS and achieve the exact same core benefit, with less code.

Ultimately it boils down to reactivity. Can developers tolerate the indirection of reactivity itself?

Actually, there's nothing inherently indirect about reactivity. Instead of A referencing and controlling B, B references and reacts to A. It only feels indirect because it's backwards from what we're used to.

So I would be sad if we shifted away from unidirectionality without trying a new approach first. I'm willing to collaborate on anything at all that could increase reactivity in NgRx.

what primitives need to exist inside of signalStore for you to make StateAdapt work on top of it? It seems like you were able to achieve it on top of the infra we provided in @ngrx/{store,effect}.

With NgRx/Store I just used the global store object and created a dynamic reducer to contain all the StateAdapt weirdness. Most of the engineering was internal to StateAdapt. I can think of some ways to use parts of StateAdapt on top of Signal Store, but there are certain reactivity patterns that I think are unlikely to be possible without some foundational changes.

mfp22 commented 3 months ago

To be specific:

Sources

Events need to be definable externally, or existing sources/observables from StateAdapt can't be incorporated in signal stores.

It's good that signal store events would be exposed as observables. This would allow StateAdapt stores to react to them as well. Now if signal stores could react to other signal store events, that would be fantastic.

On-demand state decoupled from injector tree

This is a big feature that no state management library has except for Angular Query and StateAdapt (that I've seen). Even RxJS requires a a custom variation of share.

In order for consumers of state to be reactive, they can't be tasked with managing their dependencies. Data sources should declare what they are, and self-manage. If there is a data$ observable, nothing that uses it should have to know how to make it emit what it already says it's supposed to emit. It is data$. It knows when it can use a cached value or when it's stale and needs to refetch, it knows how to provide data to N consumers without them knowing about each other, and it cleans up after itself when it is no longer being used. This goes for all observables of state.

Injectables can do this stuff when their consumers are injectors. But I'm working in an app with 100+ sibling routes, and many of them use some of the same data as others, so none of them can own this state. The parent route/injector would have to initialize and fetch the state too early, and then it couldn't refresh automatically when navigating away from and to routes that need it.

To enable this, I would need signal stores to be provided in 'root' and a special function to inject & subscribe to the store in components. store = use(BooksStore).

I've also asked the Angular team about supporting a service lifecycle method that runs the first time an injectable is injected into a component or route guard/resolver. We would also need a method for when the last one is destroyed so the injectable could clean up. Signals store and other libraries could build on top of this, and for StateAdapt behavior to be supported, it is when state is active that source observables are subscribed to so that HTTP requests can be automatically canceled or unused websockets can be cleaned up.

rainerhahnekamp commented 3 months ago

@MikeRyanDev, also, thanks for your detailed answer and kind words. 👍

If this RFC goes through, it's trivial for us to support an official Redux dev tools extension. Maybe you and I can collaborate on this directly in NgRx? 😃

That would be great. I think I'll wait two weeks and see how this RFC progresses. If it doesn't lend toward the "Redux" direction, I would add the "global dispatching" feature to our withRedux extension. (after pinging you and Marko).

I disagree with the general community sentiment that signals eliminate the need for observables. In all cases where developers used observables to manage state, yes, that's eliminated. No more behavior subjects, only signals. But for events? There's no comparison.

In our project, the question Signals/RxJs came down to managing race conditions. If we didn't need to manage them, glitch-free was surprisingly not an issue.

We could even implement dependencies between stores by using Signals as triggers...