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

Bug: shows duplicate the last elements of an array stored with useRef #22564

Closed
stvkoch opened this issue Oct 14, 2021 · 4 comments
Closed

Bug: shows duplicate the last elements of an array stored with useRef #22564

stvkoch opened this issue Oct 14, 2021 · 4 comments
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@stvkoch
Copy link

stvkoch commented Oct 14, 2021

I'm keeping the last values passed as props without force a re-render, I'm using the useRef to store the elements without re-render the output.

The weird part is that the values showed are different from what I'm storing, duplicating the last elements.

React version: 17.0.2
image

Link to code example:

https://codesandbox.io/s/stupefied-ride-m1did?file=/src/App.js

import React from "react";

const ComR = React.memo(function Compo({ id, value }) {
  const lastElements = React.useRef([0, 0, 0, 0, 0]);
  const [_, ...m] = [...lastElements.current, value]; // remove first and insert last
  lastElements.current = m;
  console.log("rendering", id, value, memo.current);
  return (
    <div>
      {id} - {lastElements.current.join(", ")}
    </div>
  );
});

export default function App() {
  const [value, setValue] = React.useState(0);

  React.useEffect(() => {
    setInterval(() => {
      setValue(Math.ceil(Math.random() * 10000));
    }, 7000);
  }, [setValue]);

  return (
    <div className="App">
      <ComR id="1" value={value} />
    </div>
  );
}

The current behavior

The console.log is showing different what is printing into the component

image

The expected behavior

expects work as linear array operations, since was called/rendered once

@stvkoch stvkoch added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Oct 14, 2021
@stvkoch stvkoch changed the title Bug: shows duplicate the last elements of an array stored useRef Bug: shows duplicate the last elements of an array stored with useRef Oct 14, 2021
@irinakk
Copy link
Contributor

irinakk commented Oct 15, 2021

This is the expected behaviour, because render is triggered twice in app wrapped by <React.StrcitMode /> in dev mode(wont affect production build), but the duped console logs are somehow eliminated in React 17(will fixed by #22030 ) so you only see the log of the first rendering, the second log with the final value [0, 0, 0, 1301, 1301] is hidden.

Anyway its no good adding side effects in rendering, as in your code example, you might need to use state instead.

const ComR = React.memo(function Compo({ id, value }) {
  const [lastElements, setElements] = React.useState([0, 0, 0, 0, 0]);
  React.useEffect(() => {
    setElements((els) => {
      const [_, ...m] = els;
      return [...m, value]
    })
  }, [value])
  return (
    <div>
      {id} - {lastElements.join(',')}
    </div>
  );
});

@bvaughn
Copy link
Contributor

bvaughn commented Oct 15, 2021

Anyway its no good adding side effects in rendering

This is right! You can learn more here: https://reactjs.org/docs/strict-mode.html#detecting-unexpected-side-effects

Calling setTimeout is a side effect, which is why you have it in an effect:

  React.useEffect(() => {
    setInterval(() => {
      setValue(Math.ceil(Math.random() * 10000));
    }, 7000);
  }, [setValue]);

But modifying the ref (reading or writing) during render is also a side effect:

const ComR = React.memo(function Compo({ id, value }) {
  const lastElements = React.useRef([0, 0, 0, 0, 0]);
  const [_, ...m] = [...lastElements.current, value];

  // This is also a side effect;
  // the only time this should be done is for the "lazy initialization" pattern.
  lastElements.current = m;

I wrote a bit more about why this is on PR #18545, although we have not yet enabled this warning.

@bvaughn
Copy link
Contributor

bvaughn commented Oct 15, 2021

Going to close this issue because it seems like a misunderstanding that has hopefully been rectified by the two replies above. Let me know if anything is still unclear though.

@bvaughn bvaughn closed this as completed Oct 15, 2021
@gaearon
Copy link
Collaborator

gaearon commented Mar 30, 2022

To follow up on this, we've changed the logging behavior in 18 to be more intuitive.
#21783 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests

4 participants