Passing handler results in undefined

Question

I can't seem to pass this handler correctly. TabItem ends up with undefined for onClick.

SearchTabs

export default class SearchTabs extends Component {
constructor(props) {
    super(props)
    const breakpoints = {
      [SITE_PLATFORM_WEB]: {
        displayGrid: true,
        autoFocus: true,
      },
      [SITE_PLATFORM_MOBILE]: {
        displayGrid: false,
        autoFocus: false,
      },
    };

    this.state = {
      breakpoints,
      filters: null,
      filter: null,
      isDropdownOpen: false,
      selectedFilter: null,
      tabs: null,
    };

    this.tabChanged = this.tabChanged.bind(this);
    this.closeDropdown = this.closeDropdown.bind(this);
  }
  ... more code

    createTabs(panels) {
        if(!panels) return;
        const tabs = panels.member.map((panel, idx) => {

          const { selectedTab } = this.props;
          const { id: panelId, headline } = panel;
          const url = getHeaderLogo(panel, 50);
          const item = url ? <img src={url} alt={headline} /> : headline;

          const classname = classNames([
            searchResultsTheme.tabItem,
            (idx === selectedTab) ? searchResultsTheme.active : null,
          ]);

          this.renderFilters(panel, idx, selectedTab);

          return (
            <TabItem
              key={panelId}
              classname={classname}
              idx={idx}
              content={item}
              onClick={this.tabChanged(idx, headline)}
            />
          );
        });

        return tabs;
      }

      tabChanged(idx, headline) {
        const { selectedTab } = this.props;
        const { selectedFilter } = this.state;
        const selectedFilterIdx = _.get(selectedFilter, 'idx', null);
        if (selectedTab !== idx) {
          this.props.resetNextPage();
          this.props.setTab(idx, selectedFilterIdx, headline);
          this.closeDropdown();
        }
      }
  render() {
    // const { panels, selectedTab } = this.props;
    // if (!panels || panels.length === 0) return null;
    //
    //
    // const { tabs, selectedTab } = this.props;

    return (
      <div>
        <ul>{this.state.tabs}</ul>
      </div>
    );
  }
}

export const TabItem = ({ classname, content, onClick, key }) => (
      <li key={key} className={`${classname} tab-item`} onClick={onClick} >{content}</li>
    );

so in TabItem onClick={onClick} ends up with undefined for onClick.

More info

here's how this used to work, when this was a function in the parent Container:

// renderDefaultTabs() {
  //   const { panels, selectedTab } = this.props;
  //
  //   if (!panels || panels.length === 0) return;
  //
  //   let filter = null;
  //
  //   const tabs = panels.member.map((panel, idx) => {
  //     const { id: panelId, headline } = panel;
  //     const url = getHeaderLogo(panel, 50);
  //     const item = url ?
  //       <img src={url} alt={headline} /> : headline;
  //     const classname = classNames([
  //       searchResultsTheme.tabItem,
  //       (idx === selectedTab) ? searchResultsTheme.active : null,
  //     ]);
  //
  //     filter = (idx === selectedTab) ? this.renderFilters(panel) : filter;
  //
  //     return (
  //       <li
  //         key={panelId}
  //         className={classname}
  //         onClick={() => {
  //           this.tabChanged(idx, headline);
  //         }}
  //       >
  //         {item}
  //       </li>
  //     );
  //   });

So I extracted that out to that SearchTabs including moving the tabChange d method to my new SearchTabs component. And now in the container the above now does this:

  renderDefaultTabs() {
    const {
      onFilterClick,
      panels,
      resetNextPage,
      selectedTab,
      selectedFilter,
      isDropdownOpen,
    } = this.props;

    return (<SearchTabs
      panels={panels}
      ...
    />);
  }

Note: renderDefaultTabs() is sent as a prop to in the render() of the container and the Search calls it back thus rendering it in the Search's render():

Container

render() {
    return (
      <Search
        request={{
          headers: searchHeaders,
          route: searchRoute,
        }}
        renderTabs={this.renderDefaultTabs}
        renderSearchResults={this.renderSearchResults}
        handleInputChange={({ input }) => {
          this.setState({ searchInput: input });
        }}
        renderAltResults={true}
      />
    );
  }

Search is a shared component our apps use.

Update

So I mentioned that the Container's render() passes the renderDefaultTabs function as a prop to <Search />. Inside <Search /> it ultimately does this: render() { <div>{renderTabs({searchResults})}</div>} which calls the container's renderDefaultTabs function which as you can see above, ultimately renders

So it is passing it as a function. It's just strange when I click a TabItem, it doesn't hit my tabChanged function whatsoever

Update

Christ, it's hitting my tabChanged. Errr..I think I'm good. Thanks all!

Possible duplicate of [ReactJS trying to call setState from onClick function and getting: Cannot update during an existing state transition error](https://stackoverflow.com/questions/44009524/reactjs-trying-to-call-setstate-from-onclick-function-and-getting-cannot-update) — Shubham Khatri, Aug 25 '17 at 07:51
not a dup, I looked at it and I'm doing exactly the same thing but mine still does not work. — PositiveGuy, Aug 25 '17 at 08:46

An Nguyen · Accepted Answer · 2017-08-25T08:57:59.240

0

onClick={this.tabChanged(idx, headline)}

This is not a proper way to pass a function to child component's props. Do it like (though it is not recommended)

onClick={() => this.tabChanged(idx, headline)}

UPDATE

I want to add more explanation. By onClick={this.tabChanged(idx, headline)}, you are executing tabChanged and pass its returned value to onClick.

With your previous implementation: onClick={() => { this.tabChanged(idx, headline); }}, now onClick will be a function similar to:

onClick = {(function() {
  this.tabChanged(idx, headline);
})}

So it works with your previous implementation. With your new implementation, onClick={() => this.tabChanged(idx, headline)} should work

edited Aug 25 '17 at 08:57

answered Aug 25 '17 at 07:47

An Nguyen

1,487
10
21

Why is it not recommended? – PositiveGuy Aug 25 '17 at 07:48
Arrow functions `() =>` will create a brand new function on every single `render`. You can refer here: https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-no-bind.md to do it properly – An Nguyen Aug 25 '17 at 07:49
`onClick={() => this.tabChanged(idx, headline)}` - that doesn't work, I tried it, it's still undefined. – PositiveGuy Aug 25 '17 at 08:24
ok, is there a slicker way to do that though? seems kinda hacky – PositiveGuy Aug 25 '17 at 08:33
we didn't have to do this before and we had exactly what you have..we didn't need to do that... – PositiveGuy Aug 25 '17 at 08:33
1

My bad again. You don't need to get to do that extra step because `map` already has arrow function. – An Nguyen Aug 25 '17 at 08:41
added more info also, check the post – PositiveGuy Aug 25 '17 at 08:42
looks like `onClick={() => this.tabChanged(idx, headline)}` is passing the function. But when I click a TabItem, I get no error, and nothing happens – PositiveGuy Aug 25 '17 at 08:56
let me see if tabChanged is getting hit hold on – PositiveGuy Aug 25 '17 at 08:56
1

I added some more explanation. From my point of view, arrow function should work. Have you really tried? – An Nguyen Aug 25 '17 at 08:58

Passing handler results in undefined

1 Answers1