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

Ability to use reducer-style action with Immer #628

Closed
allpro opened this issue Feb 19, 2021 · 4 comments
Closed

Ability to use reducer-style action with Immer #628

allpro opened this issue Feb 19, 2021 · 4 comments

Comments

@allpro
Copy link
Contributor

allpro commented Feb 19, 2021

I have a large dataset that requires maximum update performance. This object is never mutated; only replaced with a new dataset. A normal action() takes many seconds to update state, even though it's a simple value setter...

action((state, data) => {
  state.data = data;
})

It's Immer that is slow so I tried a reducer-style action to bypass Immer. Since data is always a new object, immutability is automatic anyway.

action((state, data) => ({ ...state, data })

However I quickly found the return-state style does not work when Immer is enabled and there is a computed value (getter) in the model. This limitation seems to be in both v3 and v4. (I'm still using v3)

My request is some way to bypass Immer in an action.
I could use a reducer() method for this value and then dispatch() an action in the thunk to set it, but this syntax seems clumsy and non-standard. (But is what I'm doing for now.)

One obvious option is for the standard reducer-style return-state to work without disabling Immer.

Alternatively a partial state could be returned from an action, which would be shallow-merged into state. This avoids having computed values included in the returned state data. For example...

const model = {
  data: {},
  other: [],
  item: computed((state, id) => state.data[id]),
  setData: action((state, payload) => ({ data: payload }),
  fetch: thunk((actions, payload) => {
    getSomething(payload)
      .then(actions.setData);
  })

This could work like:

  • The setData() action returns { data: {...} }
  • The model's state object is cloned for immutability
  • The returned partial state is shallow-merged into state

This achieves the equivalent of { ...state, data: {...} }, but without the issue of computed values inside the state object. Just one idea to work around the current problem.

@avevlad
Copy link

avevlad commented Feb 19, 2021

Same issue, why immer or easy-peasy so slow set state?

Store:

// analogs.store.ts
setAnalogs: action((state, payload) => {
    state.analogs = payload;
}),

Handle click button action in react component:

console.time("dgb: http request");
const response = await client.post(
  ENDPOINTS.assessmentApi + `/3564/analogs`,
  payload
);
console.timeEnd("dgb: http request");

console.time("dgb: setAnalogs");
// setAnalogsAsReactUseState(response.data.data.analogs);
assessmentActions.setAnalogs(response.data.data.analogs);
console.timeEnd("dgb: setAnalogs");

console.time setAnalogs 1260ms :(

Test on: [email protected] and [email protected]

Kapture.2021-02-19.at.12.41.45.mp4

@ctrlplusb
Copy link
Owner

Hi both, I want to look into this. I had one previous report by someone who was assigning a large payload of JSON to their store. The perf hit comes with Immer doing Proxy wrapping and deep checking. I understand your data may be sensitive but I would really appreciate some sort of example of the size of data you are attempting to assign to your store so that I can build a test case based on this and come up with a recommendation for these cases. 🙏

@ctrlplusb
Copy link
Owner

ctrlplusb commented Feb 27, 2021

Hi both, I am experimenting with a configuration option for action and actionOn that allows you disable Immer per action.

It is available via [email protected]

Can you please try this and report back your experiences?

setAnalogs: action(
  (state, payload) => ({
    ..state,
    analogs: payload,
  }),
  { immer: false },  // 👈 pass in a config as 2nd argument to `action`
),

@allpro
Copy link
Contributor Author

allpro commented Feb 28, 2021

The solution described would serve my need perfectly. However I won't be able to test it for a while because starting the final sprint of the current release. Therefore I can't do any upgrading and don't have time to create a test environment for this. I'll upgrade EP at the start of next release to this version, or whatever is current then. I'll test then and post back here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 📋 Backlog
Development

No branches or pull requests

3 participants