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

EuiFieldSearch unnecessarily unmounting/mounting #877

Closed
legrego opened this issue May 25, 2018 · 2 comments
Closed

EuiFieldSearch unnecessarily unmounting/mounting #877

legrego opened this issue May 25, 2018 · 2 comments
Assignees
Labels

Comments

@legrego
Copy link
Member

legrego commented May 25, 2018

It appears that the EuiFieldSearch component is unnecessarily unmounting and remounting itself at times. I observed this when trying to listen to focus/blur events on the search component via its ref, but I noticed that I was adding/removing those listeners on every keypress within the input field.

Listeners aside, this is the smallest test case I could come up with. The incremental prop is not required, but it gets the point across faster.

Expected Behavior

The handleInputRef callback should be called once when the component is first mounted, and once when it is unmounted.

Actual Behavior

The handleInputRef callback is invoked once when the component is first mounted, again after every call to this.setState(), and once when it is unmounted.

/*
 * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
 * or more contributor license agreements. Licensed under the Elastic License;
 * you may not use this file except in compliance with the Elastic License.
 */

import React, { Component } from 'react';
import {
  EuiFieldSearch
} from '@elastic/eui';

export class FieldSearchInputTest extends Component {
  state = {
    searchTerm: ''
  };

  render() {
    return (
      <div>
        <EuiFieldSearch
          inputRef={this.handleInputRef}
          onSearch={this.onSearch}
          incremental={true}
        />
        {this.state.searchTerm}
      </div>
    );
  }

  handleInputRef = (ref) => {
    if (!ref) {
      console.log(`inputRef called without ref`);
    } else {
      console.log('inputRef called with ref', ref);
    }
  }

  onSearch = (searchTerm) => {
    this.setState({
      searchTerm
    });
  }
}
@legrego legrego added the bug label May 25, 2018
@chandlerprall chandlerprall self-assigned this May 25, 2018
@chandlerprall
Copy link
Contributor

Good catch, and thank you for the simple reproduction for testing! Perfect.

Thankfully, in this case React isn't un- and re-mounting any of the components, so any superfluous CPU time is spent removing/adding your event listeners (and anything else you're doing on ref creation).

What is going on? EuiFieldSearch creates a new ref function on every render, and React will mark the ref function for update if the previous ref does not strict-equals the current ref. This behaviour creates unexpected calls to the ref - as you've noticed - so I'll update the component to not create ref every render.

@legrego
Copy link
Member Author

legrego commented May 26, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants