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

bake in patchState immutability for simplicity #4245

Closed
1 of 2 tasks
ducin opened this issue Feb 14, 2024 · 8 comments
Closed
1 of 2 tasks

bake in patchState immutability for simplicity #4245

ducin opened this issue Feb 14, 2024 · 8 comments

Comments

@ducin
Copy link
Contributor

ducin commented Feb 14, 2024

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

signals

Information

The proposal is to use immer or mutative in order to simplify immutable operations maintenance for developers.

Motivation

Signals require immutable updates. For nested structures, writing immutable code by hand is going to be cumbersome for devs. This proposal aims to remove this incovenience.

Example

the "hidden" immutability would be baked in patchState.

BEFORE:

withMethods((store) => ({
  updateQuery(query: string): void {
    patchState(store, (state) => ({ filter: { ...state.filter, query } }));
  },
...

AFTER:

withMethods((store) => ({
  updateQuery(query: string): void {
    patchState(store, (draft) => { draft.query = query; });
  },
...

and this is how mutative/immer normally work:

import { create } from 'mutative';

const baseState = {
  foo: 'bar',
  list: [{ text: 'coding' }],
};

const state = create(baseState, (draft) => {
  draft.list.push({ text: 'learning' });
});

Context

In react ecosystem people have to apply immutable data updates. That was one of the many painpoints with original redux, which introduced the need for quite a lot of boilerplate. The need for immutability in reducers was one of the factors that made redux quite unpopular within react community in last years. Later, @markerikson did a great job by introducing redux-toolkit (opinionated, 50%+ less code to do typical redux in apps), which has reduced the boilerplate significantly. One of the great improvements was introducing immer as a hard dependency. All redux-toolkit users I know are super happy with how immer removes the need for spreading the objects over and over again.

Drawbacks

The only downside I'm seeing is potentially the bundle size which currently is super small.
image

Describe any alternatives/workarounds you're currently using

Let people write spread objects as they are doing now.

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

  • Yes
  • No

OF COURSE (:

@ducin ducin changed the title bake in immutability for simplicity bake in patchState immutability for simplicity Feb 14, 2024
@rainerhahnekamp
Copy link
Contributor

There is already a similar issue, which is more about a compromise: #4030

We would have immutability on a type level and don't require immer. I created a PR for the prototype of the NgRx Signal Store. If there is interest, I could bring it up again.

@brandonroberts
Copy link
Member

Left some context in that issue also, but we've discussed this issue over time https://github.com/ngrx/platform/issues?q=is%3Aissue+immer+is%3Aclosed+

@markostanimirovic
Copy link
Member

The immerPatchState function will be added to the ngrx-immer package.

immerPatchState(store, (state) => {
  state.todos.push('foo');
});

Closing this issue in favor of timdeschryver/ngrx-immer#21

@markostanimirovic markostanimirovic closed this as not planned Won't fix, can't repro, duplicate, stale Feb 14, 2024
@timdeschryver
Copy link
Member

I'm using this issue to promote timdeschryver/ngrx-immer#21
If you want to contribute feel free to drop your name there, and it will get assigned to you

@rainerhahnekamp
Copy link
Contributor

@timdeschryver, I would take that one. So if there's a waiting list out there, please add me.

@ducin
Copy link
Contributor Author

ducin commented Feb 15, 2024

@rainerhahnekamp @markostanimirovic @timdeschryver FWIW I have just implemented (or rather, bridged) it here: https://github.com/ducin/sygnalyze. sygnalyze is meant to be a multi-purpose angular signals toolkit, also (but not only) with ngrx signal store extensions (in a separate entry). But having nothing to do with rxjs-based ngrx.

Although it very slightly overlaps with ngrx-immer, in the future maybe(?) also with angular architects ngrx toolkit (though not gonna copy anything from anywhere) let's treat it as a healthy competition and inspire each other 🍻 🤝

@rainerhahnekamp
Copy link
Contributor

@ducin, good, so I understand that I can still extend ngrx-immer and we might have multiple community solutions.

@ducin
Copy link
Contributor Author

ducin commented Feb 15, 2024

yeah, I think so

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

No branches or pull requests

5 participants