react-onclickoutside - npm Package Compare versions

Comparing version 5.7.1 to 5.8.0

index.js

@@ -32,2 +32,25 @@ /**
   /**
+   * Try to find our node in a hierarchy of nodes, returning the document
+   * node as highest noode if our node is not found in the path up.
+   */
+  var findHighest = function(current, componentNode, ignoreClass) {
+    if (current === componentNode) {
+      return true;
+    }
+
+    // If source=local then this event came from 'somewhere'
+    // inside and should be ignored. We could handle this with
+    // a layered approach, too, but that requires going back to
+    // thinking in terms of Dom node nesting, running counter
+    // to React's 'you shouldn't care about the DOM' philosophy.
+    while(current.parentNode) {
+      if (isNodeFound(current, componentNode, ignoreClass)) {
+        return true;
+      }
+      current = current.parentNode;
+    }
+    return current;
+  };
+
+  /**
    * Generate the event handler that checks whether a clicked DOM node
@@ -45,16 +68,5 @@ * is inside of, or lives outside of, our Component's node tree.
       var current = evt.target;
-      var found = false;
-      // If source=local then this event came from 'somewhere'
-      // inside and should be ignored. We could handle this with
-      // a layered approach, too, but that requires going back to
-      // thinking in terms of Dom node nesting, running counter
-      // to React's 'you shouldn't care about the DOM' philosophy.
-      while(current.parentNode) {
-        found = isNodeFound(current, componentNode, ignoreClass);
-        if(found) return;
-        current = current.parentNode;
+      if(findHighest(current, componentNode, ignoreClass) !== document) {
+        return;
       }
-      // If element is in a detached DOM, consider it 'not clicked
-      // outside', as it cannot be known whether it was outside.
-      if(current !== document) return;
       eventHandler(evt);
@@ -64,3 +76,2 @@ };
 
-
   /**
@@ -105,4 +116,4 @@ * This function generates the HOC function that you'll use
         componentDidMount: function() {
-          // If we are in an environment without a DOM such 
-          // as shallow rendering or snapshots then we exit 
+          // If we are in an environment without a DOM such
+          // as shallow rendering or snapshots then we exit
           // early to prevent any unhandled errors being thrown.
@@ -133,4 +144,24 @@ if (typeof document === 'undefined' || !document.createElement){
 
+          var componentNode = ReactDOM.findDOMNode(instance);
+          if (componentNode === null) {
+            console.warn('Antipattern warning: there was no DOM node associated with the component that is being wrapped by outsideClick.');
+            console.warn([
+              'This is typically caused by having a component that starts life with a render function that',
+              'returns `null` (due to a state or props value), so that the component \'exist\' in the React',
+              'chain of components, but not in the DOM.\n\nInstead, you need to refactor your code so that the',
+              'decision of whether or not to show your component is handled by the parent, in their render()',
+              'function.\n\nIn code, rather than:\n\n  A{render(){return check? <.../> : null;}\n  B{render(){<A check=... />}\n\nmake sure that you',
+              'use:\n\n  A{render(){return <.../>}\n  B{render(){return <...>{ check ? <A/> : null }<...>}}\n\nThat is:',
+              'the parent is always responsible for deciding whether or not to render any of its children.',
+              'It is not the child\'s responsibility to decide whether a render instruction from above should',
+              'get ignored or not by returning `null`.\n\nWhen any component gets its render() function called,',
+              'that is the signal that it should be rendering its part of the UI. It may in turn decide not to',
+              'render all of *its* children, but it should never return `null` for itself. It is not responsible',
+              'for that decision.'
+            ].join(' '));
+            throw new Error('onClickOutside was NOT applied to', instance.constructor.name);
+          }
+
           var fn = this.__outsideClickHandler = generateOutsideCheck(
-            ReactDOM.findDOMNode(instance),
+            componentNode,
             instance,
@@ -137,0 +168,0 @@ clickOutsideHandler,

package.json

 {
   "name": "react-onclickoutside",
-  "version": "5.7.1",
+  "version": "5.8.0",
   "description": "An onClickOutside mixin for React components",
   "main": "index.js",
+  "files": [
+    "index.js"
+  ],
   "homepage": "https://github.com/Pomax/react-onclickoutside",
@@ -7,0 +10,0 @@ "authors": [

README.md

@@ -57,2 +57,95 @@ # An onClickOutside wrapper for React components
 
+### IMPORTANT: Make sure there are DOM nodes to work with.
+
+If you are using this HOC to toggle visibility of UI elements, make sure you understand how responsibility for this works in React. While in a traditional web setting you would simply call something like `.show()` and `.hide()` on a part of the UI you want to toggle visibility for, using CSS properties, React instead is about *simply not showing UI unless it should be visible*.
+
+As such, doing **the following is a guaranteed error** for onClickOutside:
+```
+class InitiallyHidden extends React.Component {
+  constructor(props) {
+    super(props);
+  }
+  render() {
+    return this.props.hidden ? null : <div>...loads of content...</div>;
+  }
+  handleClickOutside() {
+    this.props.hide();
+  }
+}
+
+const A = onClickOutside(InitiallyHidden);
+
+class UI extends React.Component {
+  constructor(props) {
+    super(props);
+    this.state = {
+      hideThing: true
+    }
+  }
+  render() {
+    return <div>
+      <button onClick={e => this.showContent() }>click to show content</button>
+      <A hidden={this.state.hideThing} hide={e => this.hideContent() }/>
+    </div>;
+  }
+  showContent() {
+    this.setState({ hideThing: false });
+  }
+  hideContent() {
+    this.setState({ hideThing: true });
+  }
+}
+```
+
+Runnign this code will result in a console log that looks like this:
+
+![](warning.png)
+
+The reason this code will fail is that this component can mount *without* a DOM node backing it. Writing a `render()` function like this is somewhat of an antipattern: a component should assume that *if* its render function is called, it should render. It should *not* potentially render nothing.
+
+Instead, the parent should decide whether some child component should render at all, and any component should assume that when its `render()` function is called, it should render itself.
+
+A refactor is typically trivially effected, and **the following code will work fine**: 
+
+```
+class InitiallyHidden extends React.Component {
+  constructor(props) {
+    super(props);
+  }
+  render() {
+    return <div>...loads of content...</div>;
+  }
+  handleClickOutside() {
+    this.props.hide();
+  }
+}
+
+const A = onClickOutside(InitiallyHidden);
+
+class UI extends React.Component {
+  constructor(props) {
+    super(props);
+    this.state = {
+      hideThing: true
+    }
+  }
+  render() {
+    return <div>
+      <button onClick={e => this.showContent() }>click to show content</button>
+      { this.state.hideThing ? null : <A hide={e => this.hideContent() }/> }
+    </div>;
+  }
+  showContent() {
+    this.setState({ hideThing: false });
+  }
+  hideContent() {
+    this.setState({ hideThing: true });
+  }
+}
+```
+
+Here we have code where each component trusts that its `render()` will only get called when there is in fact something to render, and the `UI` component does this by making sure to check what *it* needs to render.
+
+The onOutsideClick HOC will work just fine with this kind of code. 
+
 ## Regulate which events to listen for
@@ -103,3 +196,3 @@
 
-Technically this HOC lets you pass in `preventDefault={true/false}` and `preventDefault={true/false}` to regulate what happens to the event when it hits your `handleClickOutside(evt)` function, but beware: `stopPropagation` may not do what you expect it to do.
+Technically this HOC lets you pass in `preventDefault={true/false}` and `stopPropagation={true/false}` to regulate what happens to the event when it hits your `handleClickOutside(evt)` function, but beware: `stopPropagation` may not do what you expect it to do.
 
@@ -106,0 +199,0 @@ Each component adds new event listeners to the document, which may or may not cause as many event triggers as there are event listening bindings. In the test file found in `./test/browser/index.html`, the coded uses `stopPropagation={true}` but sibling events still make it to "parents".

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Major refactor

Supply chain risk

Package has recently undergone a major refactor. It may be unstable or indicate significant internal changes. Use caution when updating to versions that include significant changes.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Improved metrics

Number of lines in readme file

260

increased by55.69%

Worsened metrics

Total package byte prevSize

24640

decreased by-27.87%

Number of package files

3

decreased by-72.73%

Lines of code

266

decreased by-57.51%

Number of medium supply chain risk alerts

1

increased byInfinity%

No dependency changes