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: Certain uses of render props result in remounts on every render #18199

Closed
Rinaldo opened this issue Mar 3, 2020 · 5 comments
Closed

Bug: Certain uses of render props result in remounts on every render #18199

Rinaldo opened this issue Mar 3, 2020 · 5 comments
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@Rinaldo
Copy link

Rinaldo commented Mar 3, 2020

React version: 16.13

Both of the following code snippets produce an input component that loses focus on render:
Sandbox

// render prop is passed a component it can render
const Wizard = ({ components, render }) => {
  const [state, setState] = React.useState({})
  const BaseComp = components[0]
  const Component = props => <BaseComp state={state} setState={setState} {...props} />
  return render({ Component })
}

const ChildComponent = ({ state, setState }) => {
  React.useEffect(() => console.log("mounting"), []) // for demonstration purposes only
  const { foo = "" } = state
  const handleChange = ({ target: { name, value } }) => {
    setState(prevState => ({ ...prevState, [name]: value }))
  }
  return <input name="foo" value={foo} onChange={handleChange} />
}

const Frame = ({ Component }) => (
  <div style={{ margin: 20 }}>
    <Component /> // {Component()} works
  </div>
)

const App = () => (
  <Wizard
    components={[ChildComponent]}
    render={Frame}
  />
)

Sandbox

// render prop is passed already rendered children
const Wizard = ({ components, render }) => {
  const [state, setState] = React.useState({})
  const BaseComp = components[0]
  const children = <BaseComp state={state} setState={setState} />
  return render({ children })
}

const ChildComponent = ({ state, setState }) => {
  React.useEffect(() => console.log("mounting"), []) // for demonstration purposes only
  const { foo = "" } = state
  const handleChange = ({ target: { name, value } }) => {
    setState(prevState => ({ ...prevState, [name]: value }))
  }
  return <input name="foo" value={foo} onChange={handleChange} />
}

const createFrame = () => ({ children }) => (
  <div style={{ margin: 20 }}>{children}</div>
)

const Frame = createFrame()

const App = () => (
  <Wizard
    components={[ChildComponent]}
    render={props => {
      const Comp = createFrame()
      // const Comp = Frame // uncomment this line and comment out the one above and it works
      return <Comp {...props} /> // return createFrame()(props) also works
    }}
  />
)

Context

In an app I'm working on there is a wizard component that does state management for an array of pages. It takes a render prop which is passed the current page and other stuff like status and navigation functions so that the look of the wizard is separated from the state management. As the render props passed to different wizards have gotten more complex, we've had issues with inputs losing focus and even some app crashes.

Preliminary Investigation

In both of the above examples, ChildComponent is being remounted on every render of the wizard.

One thing I noticed in the 2nd example is that it can be fixed by swapping Frame and createFrame(). I find this concerning because I thought functional components were pure functions and when working with pure functions you can swap the left side of an assignment for the right side without affecting the result.

Edit:
It seems to be partially a syntax issue. Both examples can be made to work by avoiding JSX

@Rinaldo Rinaldo added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Mar 3, 2020
@Rinaldo Rinaldo changed the title Bug: Certain uses of render props cause inputs to lose focus Bug: Certain uses of render props cause remounts on every render Mar 3, 2020
@Rinaldo Rinaldo changed the title Bug: Certain uses of render props cause remounts on every render Bug: Certain uses of render props result in remounts on every render Mar 3, 2020
@jddxf
Copy link
Contributor

jddxf commented Mar 3, 2020

You were writing const Component = props => <BaseComp state={state} setState={setState} {...props} /> in the first example and createFrame in the second. So in both examples, a new type of component is generated on every render. This a hint that React should unmount the old component and mount the new one instead of reusing the old one .

@Rinaldo
Copy link
Author

Rinaldo commented Mar 3, 2020

@jddxf So why does avoiding JSX seem to solve the problem?
In the first example, why does {Component()} work when <Component /> doesn't?
In the second example, why does createFrame()(props) work?

@jddxf
Copy link
Contributor

jddxf commented Mar 3, 2020

The non-jsx equivalent of <Component /> is React.createElement(Component) rather than {Component()}. Only things passed to React.createElement as the first parameter will be regarded as component types by React.

@gaearon
Copy link
Collaborator

gaearon commented Mar 3, 2020

This should explain what's happening: https://reactjs.org/docs/reconciliation.html#elements-of-different-types

In general, defining components inside other components (or Hooks) is a mistake. Instead, usually you want to declare them outside.

@gaearon gaearon closed this as completed Mar 3, 2020
@jayarjo
Copy link

jayarjo commented Nov 6, 2022

Calling it as a function fixes constant re-mounting problem, but produces a lot of these instead:

image

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