3

I'm writing a React app. I have a table of contacts:

// ... pure functional component that gets the contacts via props
return (
  <Paper>
    <table>
      <thead>
        <tr>
          {fields.map(renderHeaderCell)}
        </tr>
      </thead>
      <tbody>
        {contacts.map(renderBodyRow)}
      </tbody>
    </table>
  </Paper>
);

The renderBodyRow() function looks like this:

const renderBodyRow = contact => (
  <ContactRow
    key={contact.id}
    contact={contact}
    handleContactSave={handleContactSave}
 />
);

Now, when I update a contact and when the table isn't being sorted, the contact moves down the bottom of the list. But instead of rendering with the updated name, it renders with the old name. I assume this is because the contact.id key does not change. How can I get the row to render the new value?

For completeness sake (and because it could cause the problem), here is the ContactRow component. I don't think the problem is here thought

import PropTypes from 'prop-types';
import { equals, includes, map } from 'ramda';
import React, { useState } from 'react';

import { fields, groups, tendencies } from '../../config/constants';
import strings from './strings';

function ContactRow({ contact: original, handleContactSave }) {
  const [contact, setContact] = useState(original);
  const disabled = equals(contact, original);

  const handleSaveButtonClick = () => {
    handleContactSave(contact);
    setContact(original)
  };

  const handeCancelButtonClick = () => {
    setContact(original);
  };

  const renderOption = value => (
    <option key={`${contact.id}-${value}`} value={value}>
      {strings[value]}
    </option>
  );

  const renderBodyCell = key => {
    const value = contact[key];
    const testId = `contact-${key}${
      contact.id === 'new-contact' ? '-new-contact' : ''
    }`;
    const handleChange = e => {
      e.preventDefault();
      setContact({ ...contact, [key]: e.target.value });
    };
    return (
      <td key={`${key}-${contact.id}`}>
        {includes(value, [...groups, ...tendencies]) ? (
          <select value={value} data-testid={testId} onChange={handleChange}>
            {includes(value, groups)
              ? map(renderOption, groups)
              : map(renderOption, tendencies)}
          </select>
        ) : (
          <input value={value} data-testid={testId} onChange={handleChange} />
        )}
      </td>
    );
  };

  return (
    <tr>
      <td>
        <button
          aria-label={
            contact.id === 'new-contact' ? 'create-contact' : 'update-contact'
          }
          onClick={handleSaveButtonClick}
          disabled={disabled}
        >
          <span role="img" aria-label="save-icon">
            
          </span>
        </button>
        <button
          aria-label={
            contact.id === 'new-contact'
              ? 'cancel-create-contact'
              : 'cancel-update-contact'
          }
          disabled={disabled}
          onClick={handeCancelButtonClick}
        >
          <span role="img" aria-label="cancel-icon">
            
          </span>
        </button>
      </td>
      {map(renderBodyCell, fields)}
    </tr>
  );
}

ContactRow.propTypes = {
  contact: PropTypes.shape({
    /* fields */
  }),
  handleContactSave: PropTypes.func.isRequired
};

ContactRow.defaultProps = {
  contact: fields.reduce((acc, field) => ({ ...acc, [field]: 'N/A' }), {}),
  handleContactSave: () => {
    console.warn('No handleContactSave() function provided to ContactRow.');
  }
};

export default ContactRow;
isherwood
  • 58,414
  • 16
  • 114
  • 157
J. Hesters
  • 13,117
  • 31
  • 133
  • 249
  • `I assume this is because the contact.id key does not change` That's not correct, it's because the props didn't change, not because they key didn't change. If you want the new name, then `contact` must be an entirely new prop (shallow equals), don't just change the name on the existing object. – Adam Jenkins Mar 18 '19 at 13:22
  • @adam Interesting, I don't quite get what you mean. The `` component gets new contacts, therefore `renderBodyRow` should do too, right? Or is this not what you mean? – J. Hesters Mar 18 '19 at 13:29
  • Depends what you mean by "new contacts". I'm talking about the object references. If you're passing a new array that contains the same object references, then react might update the list, but it won't rerender the individual components in the list because the object references (i.e. the `contact` objects stored within the array) are the same as they were before. – Adam Jenkins Mar 18 '19 at 13:35
  • 1
    The `renderBodyRow` method needs to receive a new `contact` object, not the old `contact` object with the name property changed on it in order for the component to update. – Adam Jenkins Mar 18 '19 at 13:36
  • @Adam Okay I verified that it does Still no updates besides moving down in the list. – J. Hesters Mar 18 '19 at 13:44

1 Answers1

2

Ok, so I see it now. The only prop you are passing to renderBodyCell is key, no other props. This is bad practice (and just wrong). keys are used as internal optimization hints to react and should not be used for props.

  const renderBodyCell = key => {
    const value = contact[key];
    const testId = `contact-${key}${
      contact.id === 'new-contact' ? '-new-contact' : ''
    }`;
    const handleChange = e => {
      e.preventDefault();
      setContact({ ...contact, [key]: e.target.value });
    };
    return (
      <td key={`${key}-${contact.id}`}>
        {includes(value, [...groups, ...tendencies]) ? (
          <select value={value} data-testid={testId} onChange={handleChange}>
            {includes(value, groups)
              ? map(renderOption, groups)
              : map(renderOption, tendencies)}
          </select>
        ) : (
          <input value={value} data-testid={testId} onChange={handleChange} />
        )}
      </td>
    );
  };

Instead of passing in the key, you need to pass in the contact (or the contact and the key I guess, but I would hesitate to pass keys around as if they are meaningful unless you know exactly what you are doing).

EDIT: So technically, you were correct, the row wasn't being re-rendering because the key didn't change, but that's because you were using it as a prop when you shouldn't have been.

EDIT #2: Good time for you to go exploring about how React works. It is a very optimized machine. It doesn't just rerender components all the time, only when it needs to. In order to find out when it needs to rerender them, it checks props and state (or, in your case where you are doing this functionally, just the props - the function arguments) and compares them to the props the last time the component was rendered. If props are the same (shallow equals), then react just says screw it, I don't need to update, props are the same. At least that's the behaviour for PureComponent (which functional components are).

So if you want something to update, make sure the props you are passing it have changed.

Adam Jenkins
  • 51,445
  • 11
  • 72
  • 100
  • I'd like the downvote explained to me. I'm not a react expert, so if I got something wrong in my explanation let me know so I can learn please! – Adam Jenkins Mar 18 '19 at 17:20