helior / react-highlighter

:high_brightness: Highlight text using React
MIT License
157 stars 42 forks source link

The 'search' property should accept 'null' or 'undefined' value #54

Open olivierr91 opened 6 years ago

olivierr91 commented 6 years ago

When there is nothing to search, the 'search' property of the component should accept null or undefined value to signify that nothing should be searched and the component should return the child 'as is'.

Currently, React throws

Warning: Failed prop type: The prop `search` is marked as required in `Highlighter`, but its value is `null`.
warning.js:33
    in Highlighter

when null is passed to the 'search' prop.

kachkaev commented 6 years ago

I think you can pass false if there is nothing to search.

<Highlight search={highlightRegexp || false}>{text}</Highlight>
Gsiete commented 5 years ago

From looking at the code, you can indeed pass false or even '' and it would act the same as not having received the search prop

    search: PropTypes.oneOfType([
      PropTypes.string,
      PropTypes.number,
      PropTypes.bool,
      RegExpPropType
    ]).isRequired,
  hasSearch: function() {
    return (typeof this.props.search !== 'undefined') && this.props.search;
  },

However, the undefined case is explicitly being supported in the hasSearch function, but then made fail by propTypes.

Moreover (typeof this.props.search !== 'undefined') && this.props.search is the same as this.props.search for the purposes of this code.

Removing the isRequired from the propTypes would spare me from having to transform {...props} into {...props} search={props.search || false} Actually the very existence of the function hasSearch opposes the usage of isRequired