From dbd921550a040be71845d0018b93b326a78f1e39 Mon Sep 17 00:00:00 2001 From: Yuri Pirola Date: Tue, 24 Oct 2017 11:00:02 +0200 Subject: [PATCH] Add the option `suppressRefError` to `getRootProps`. Fixes paypal/downshift#235. --- README.md | 15 +++++++- src/__tests__/downshift.get-root-props.js | 47 +++++++++++++++++++++++ src/downshift.js | 11 +++++- 3 files changed, 69 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 6f6722c23..e449438e6 100644 --- a/README.md +++ b/README.md @@ -266,7 +266,7 @@ but differ slightly. > `function(inputValue: string, stateAndHelpers: object)` | optional, no useful default -Called whenever the input value changes. Useful to use instead or in combination of `onStateChange` when `inputValue` is a controlled prop to [avoid issues with cursor positions](https://github.com/paypal/downshift/issues/217). +Called whenever the input value changes. Useful to use instead or in combination of `onStateChange` when `inputValue` is a controlled prop to [avoid issues with cursor positions](https://github.com/paypal/downshift/issues/217). - `inputValue`: The current value of the input - `stateAndHelpers`: This is the same thing your `children` prop @@ -400,7 +400,7 @@ being overridden (or overriding the props returned). For example: | `getInputProps` | `function({})` | returns the props you should apply to the `input` element that you render. | | `getItemProps` | `function({})` | returns the props you should apply to any menu item elements you render. | | `getLabelProps` | `function({})` | returns the props you should apply to the `label` element that you render. | -| `getRootProps` | `function({})` | returns the props you should apply to the root element that you render. It can be optional. | +| `getRootProps` | `function({},{})` | returns the props you should apply to the root element that you render. It can be optional. | #### `getRootProps` @@ -418,6 +418,17 @@ Required properties: and your composite component would forward like: `
` +If you're rendering a composite component, `Downshift` checks that +`getRootProps` is called and that `refKey` is a prop of the returned composite +component. +This is done to catch common causes of errors but, in some cases, the check +could fail even if the ref is correctly forwarded to the root DOM component. +In these cases, you can provide the object `{suppressRefError : true}` as the +second argument to `getRootProps` to completely bypass the check. +** Please use it with extreme care and only if you are absolutely sure that the +ref is correctly forwarded otherwise `Downshift` will unexpectedly fail. ** +See issue #235 for the discussion that lead to this. + #### `getInputProps` This method should be applied to the `input` you render. It is recommended that diff --git a/src/__tests__/downshift.get-root-props.js b/src/__tests__/downshift.get-root-props.js index 061bca090..4f1c5495a 100644 --- a/src/__tests__/downshift.get-root-props.js +++ b/src/__tests__/downshift.get-root-props.js @@ -66,4 +66,51 @@ test('renders fine when rendering a composite component and applying getRootProp expect(() => mount()).not.toThrow() }) +test('returning a composite component and calling getRootProps without a refKey does not result in an error if suppressRefError is true', () => { + const MyComponent = () => ( + + {({getRootProps}) => ( + + )} + + ) + expect(() => mount()).not.toThrow() +}) + +test('returning a DOM element and calling getRootProps with a refKey does not result in an error if suppressRefError is true', () => { + const MyComponent = () => ( + + {({getRootProps}) => ( +
+ )} + + ) + expect(() => mount()).not.toThrow() +}) + +test('not applying the ref prop results in an error does not result in an error if suppressRefError is true', () => { + const MyComponent = () => ( + + {({getRootProps}) => { + const {onClick} = getRootProps({}, {suppressRefError: true}) + return
+ }} + + ) + expect(() => mount()).not.toThrow() +}) + +test('renders fine when rendering a composite component and applying getRootProps properly even if suppressRefError is true', () => { + const MyComponent = () => ( + + {({getRootProps}) => ( + + )} + + ) + expect(() => mount()).not.toThrow() +}) + /* eslint no-console:0 */ diff --git a/src/downshift.js b/src/downshift.js index 866493372..7f13d1644 100644 --- a/src/downshift.js +++ b/src/downshift.js @@ -424,11 +424,15 @@ class Downshift extends Component { rootRef = node => (this._rootNode = node) - getRootProps = ({refKey = 'ref', ...rest} = {}) => { + getRootProps = ( + {refKey = 'ref', ...rest} = {}, + {suppressRefError = false} = {}, + ) => { // this is used in the render to know whether the user has called getRootProps. // It uses that to know whether to apply the props automatically this.getRootProps.called = true this.getRootProps.refKey = refKey + this.getRootProps.suppressRefError = suppressRefError return { [refKey]: this.rootRef, ...rest, @@ -767,6 +771,7 @@ class Downshift extends Component { // apply the props for them. this.getRootProps.called = false this.getRootProps.refKey = undefined + this.getRootProps.suppressRefError = undefined // we do something similar for getLabelProps this.getLabelProps.called = false // and something similar for getInputProps @@ -776,7 +781,9 @@ class Downshift extends Component { return null } if (this.getRootProps.called) { - validateGetRootPropsCalledCorrectly(element, this.getRootProps) + if (!this.getRootProps.suppressRefError) { + validateGetRootPropsCalledCorrectly(element, this.getRootProps) + } return element } else if (isDOMElement(element)) { // they didn't apply the root props, but we can clone