Fauntleroy / react-simple-dropdown

Non-prescriptive React.js dropdown toolkit
ISC License
225 stars 75 forks source link

Doesn't work with react hot loader #45

Closed thebedroomprogrammer closed 7 years ago

thebedroomprogrammer commented 7 years ago

I was using react simple dropdown but my code was returning error. I found out that when using react hot loader the condition check for classes like this with the child.type and the component class return a falsy value even though both have both are the same class. In order to fix that I came up with a solution where I declared a static method on each class and instead of directly checking the classes I called that static method to check if they return the same string. It worked in all cases and thus I think that it would be much better to declare a static method to check for the child type by checking the return value we receive when calling the method

Problem

  bindChildren(children) {
    return React.Children.map(children, child => {
      **if (child.type == DropdownTrigger) {
        child = React.cloneElement(child, {
          onClick: this.toggleDropdown.bind(this)
        });
      } else if (child.type == DropdownMenu) {
        child = React.cloneElement(child, {
          onClick: this.onOptionClick.bind(this)
        });**
      }
      return child;
    });
  }

Solution

class DropdownMenu extends Component {
  **static check() {
    return "dropdownmenu";**
  }

  bindChildren(children) {
    return React.Children.map(children, child => {
      if **(child.type.check() === DropdownTrigger.check())** {
        child = React.cloneElement(child, {
          onClick: this.toggleDropdown.bind(this)
        });
      } else if (child.type.check() === DropdownMenu.check()) {
        child = React.cloneElement(child, {
          onClick: this.onOptionClick.bind(this)
        });
      }
      return child;
    });
  }
Fauntleroy commented 7 years ago

What version of react are you using?

thebedroomprogrammer commented 7 years ago

15.5.0 is my React Version. I don't think that it's version is a problem. It works fine. I only run into error when using it with react hot loader.

Fauntleroy commented 7 years ago

I can take another look at this, but as far as I know I'm doing the comparison properly. Have you tried filing an issue with react-hot-loader?

thebedroomprogrammer commented 7 years ago

Yeah will do. I too think that this is a problem with the react hot loader where the classes are created on the fly when hot reloading which fails the check.

johnryan commented 6 years ago

@Arpit1294 did you figure out a solution to this issue? I just noticed it when starting to use react-hot-loader 4.0.0