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

fix: set two state but re-render once #992

Merged
merged 11 commits into from
Mar 28, 2019
Merged

fix: set two state but re-render once #992

merged 11 commits into from
Mar 28, 2019

Conversation

yuanyan
Copy link
Collaborator

@yuanyan yuanyan commented Mar 21, 2019

let reRender = 0;
function MyComponent() {
  const [foo, setFoo] = useState(0);
  const [bar, setBar] = useState(0);
  return (
    <div>
      <span onClick={() => {
        setFoo(1);
        setBar(1);
       // Set two but re-render once
      }
      }>{reRender++}</span>
    </div>
  );
}

@yongningfu
Copy link
Collaborator

let reRender = 0;
function MyComponent() {
const [foo, setFoo] = useState(0);
const [bar, setBar] = useState(0);
return (


<span onClick={() => {
setFoo(1);
setBar(1);
// Set two but re-render once
}
}>{reRender++}

);
}

我跑了一下这个demo, click后 reRender为2,符合预期? rax: 1.0.0 rax-cli: 1.2

packages/rax/src/__tests__/hooks.js Outdated Show resolved Hide resolved
packages/rax/src/__tests__/hooks.js Outdated Show resolved Hide resolved
packages/rax/src/vdom/updater.js Outdated Show resolved Hide resolved
packages/rax/src/vdom/updater.js Outdated Show resolved Hide resolved
packages/rax/src/vdom/updater.js Show resolved Hide resolved
@yuanyan
Copy link
Collaborator Author

yuanyan commented Mar 21, 2019

let reRender = 0;
function MyComponent() {
const [foo, setFoo] = useState(0);
const [bar, setBar] = useState(0);
return (

<span onClick={() => {
setFoo(1);
setBar(1);
// Set two but re-render once
}
}>{reRender++}

);
}

我跑了一下这个demo, click后 reRender为2,符合预期? rax: 1.0.0 rax-cli: 1.2

还没发版本

const { current } = create;
if (current) {
// Set this to true to prevent re-entrancy
const previousIsRendering = Host.isUpdating;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这块逻辑要下掉么? 因为目前的render或者forceUpdate更新是同步的,所以可能会和官方有些不一致的表现

比如把目前存在的case做个改造

    it(
      'in sync mode, useEffect is deferred and updates finish synchronously ' +
        '(in a single batch with different state)',
      () => {
        const container = createNodeElement('div');
        let logs = [];
        function Counter(props) {
          useEffect(
            () => {
              // Update multiple times. These should all be batched together in
              // a single render.
              render(<Counter count={2} />, container);
              render(<Counter count={3} />, container);
              render(<Counter count={4} />, container);
              render(<Counter count={5} />, container);
              render(<Counter count={6} />, container);
              render(<Counter count={7} />, container);
            },
            [],
          );
          logs.push('Count: ' + props.count);
          return <span>{'Count: ' + props.count}</span>;
        }
        render(<Counter count={0} />, container);
        // Even in sync mode, effects are deferred until after paint
        expect(logs).toEqual(['Count: 0']);
        expect(container.childNodes[0].childNodes[0].data).toEqual('Count: 0');
        // Now fire the effects
        logs = [];
        jest.runAllTimers();
        // There were multiple updates, but there should only be a
        // single render
        expect(logs).toEqual(['Count: 7']);
        expect(container.childNodes[0].childNodes[0].data).toEqual('Count: 7');
      },
    );

Copy link
Collaborator

@yongningfu yongningfu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed

@yuanyan yuanyan merged commit 2fe4be9 into master Mar 28, 2019
@delete-merged-branch delete-merged-branch bot deleted the rax-core branch March 28, 2019 08:27
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

Successfully merging this pull request may close these issues.

2 participants