镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Subtle caveat with async methods #184

Open

Description

@k1w1

React Easy State version: 6.3.0
Platform: browser

There is a subtle caveat when state is modified after an async function, or in any situation where the code is called asynchronously outside a React render cycle.

If a reference to new state is held across an async boundary then the new state is not observed and changes to it do no trigger a re-render.

Here is a code snippet that shows the problem. The key is that a local variable counter is used after the async call. If it is used before the async call it works correctly, or if counterStore is used after the call to get a reference to the value from the store again then it also works correctly.

import React from 'react';
import { store, view } from '@risingstack/react-easy-state';

const counterStore = store({
  counters: []
});

async function longFunction() {}

export default view(() => {
  const addCounter = async () => {
    counterStore.counters.push({value: 0});
    const counter = counterStore.counters[counterStore.counters.length-1];

    await longFunction();

    counter.value++;
    console.log("No re-render here!")
  }
  const incCounter = () => {
    counterStore.counters[counterStore.counters.length-1].value++;
  }
  return (
    <>
      <button onClick={() => addCounter()}>Add counter</button>
      <button onClick={() => incCounter()}>Increment counter</button>
      <ul>
        {counterStore.counters.map((counter, index) => (
          <li key={index}>{counter.value}</li>
        ))}
      </ul>
    </>
  );
});

It seems like you could just say "always reference the store when updating a value", but that is very hard to do, especially with complex code. The same above was created by removing all of the complexity and nested function calls that was otherwise hiding the problem.

This is disappointing because it undermines the awesome promise of react-easy-state. I think it is worth trying to solve so code like this can work transparently.

Theory of the problem

I think the problem exists because of this sequence:

  1. addCounter is called from an event handler, so changes to the state are proxied (observed), but the new values themselves are not wrapped in a proxy.
  2. This means that counter is not a proxied variable, even though it appears to have been correctly read from the store.
  3. Without the await function it works fine because the re-render is triggered when the element is added to the counterStore.counters array and the render doesn't happen until after the function exits (because React is batching the renders).

Second variation on problem

The same problem can be triggered using setTimeout too:

import React from "react";
import { store, view } from "@risingstack/react-easy-state";

const counterStore = store({
  counters: []
});

export default view(() => {
  const addCounter = () => {
    setTimeout(() => {
      counterStore.counters.push({ value: 0 });
      const counter = counterStore.counters[counterStore.counters.length - 1];
      setTimeout(() => {
        counter.value++;
        console.log("No re-render here!");
      }, 1000);
    }, 1000);
  };
  const incCounter = () => {
    counterStore.counters[counterStore.counters.length - 1].value++;
  };
  return (
    <>
      <button onClick={() => addCounter()}>Add counter</button>
      <button onClick={() => incCounter()}>Increment counter</button>
      <ul>
        {counterStore.counters.map(counter => (
          <li>{counter.value}</li>
        ))}
      </ul>
    </>
  );
});

Activity

  1. solkimicreb commented on May 20, 2020

    @solkimicreb
    Contributor

    This seems like a very valid issue, luckily the fix will probably be easy and will be included in the next release.

    Easy State is wrapping nested store data during get-time if the immediate parent object is already observed by at least one view or autoEffect. This is done to improve performance when someone stores a lot of unobserved data inside stores somewhy.

    We will remove this optimization in the next release which will fix this issue (and let us do some other cool things). It should not cause any noticeable performance changes for normal use cases.

  2. k1w1 commented on May 20, 2020

    @k1w1
    Author

    Great. I was thinking over the problem more last night and this is the same conclusion I came to. Thanks.

    As soon as the code is in git I would be happy to test it.

  3. solkimicreb commented on May 21, 2020

    @solkimicreb
    Contributor

    This will be included in the 7.0.0 release. You can try it now with npm i @risingstack/react-easy-state@alpha, related codesandbox demo is here.

  4. k1w1 commented on May 21, 2020

    @k1w1
    Author

    Thanks. I have confirmed that this works in my actual application too. You are awesome.

    I am curious, what change actually fixes the issue? It wasn't clear to me from the commits.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions