Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the option suppressRefError to getRootProps. #241

Merged
merged 6 commits into from
Nov 9, 2017
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .all-contributorsrc
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,16 @@
"contributions": [
"example"
]
},
{
"login": "yp",
"name": "yp",
"avatar_url": "https://avatars1.githubusercontent.com/u/275483?v=4",
"profile": "http://algolab.eu/pirola",
"contributions": [
"bug",
"code"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could add "test" to this too if you want. Just update the file and commit. It'll update the readme in a commit hook 👍

]
}
]
}
19 changes: 15 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ autocomplete/dropdown/select/combobox components</p>
[![version][version-badge]][package]
[![MIT License][license-badge]][LICENSE]

[![All Contributors](https://img.shields.io/badge/all_contributors-38-orange.svg?style=flat-square)](#contributors)
[![All Contributors](https://img.shields.io/badge/all_contributors-39-orange.svg?style=flat-square)](#contributors)
[![PRs Welcome][prs-badge]][prs]
[![Chat][chat-badge]][chat]
[![Code of Conduct][coc-badge]][coc]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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`

Expand All @@ -418,6 +418,17 @@ Required properties:
and your composite component would forward like:
`<div ref={props.innerRef} />`

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 paypal/downshift#235 for the discussion that lead to this.

#### `getInputProps`

This method should be applied to the `input` you render. It is recommended that
Expand Down Expand Up @@ -649,7 +660,7 @@ Thanks goes to these people ([emoji key][emojis]):
| [<img src="https://avatars1.githubusercontent.com/u/7330124?v=4" width="100px;"/><br /><sub>Geoff Davis</sub>](https://geoffdavis.info)<br />[💡](#example-geoffdavis92 "Examples") | [<img src="https://avatars0.githubusercontent.com/u/3415488?v=4" width="100px;"/><br /><sub>Anup</sub>](https://github.com/reznord)<br />[📖](https://github.com/paypal/downshift/commits?author=reznord "Documentation") | [<img src="https://avatars0.githubusercontent.com/u/340520?v=4" width="100px;"/><br /><sub>Ferdinand Salis</sub>](http://ferdinandsalis.com)<br />[🐛](https://github.com/paypal/downshift/issues?q=author%3Aferdinandsalis "Bug reports") [💻](https://github.com/paypal/downshift/commits?author=ferdinandsalis "Code") | [<img src="https://avatars2.githubusercontent.com/u/662750?v=4" width="100px;"/><br /><sub>Kye Hohenberger</sub>](https://github.com/tkh44)<br />[🐛](https://github.com/paypal/downshift/issues?q=author%3Atkh44 "Bug reports") | [<img src="https://avatars0.githubusercontent.com/u/1443499?v=4" width="100px;"/><br /><sub>Julien Goux</sub>](https://github.com/jgoux)<br />[🐛](https://github.com/paypal/downshift/issues?q=author%3Ajgoux "Bug reports") [💻](https://github.com/paypal/downshift/commits?author=jgoux "Code") [⚠️](https://github.com/paypal/downshift/commits?author=jgoux "Tests") | [<img src="https://avatars2.githubusercontent.com/u/9586897?v=4" width="100px;"/><br /><sub>Joachim Seminck</sub>](https://github.com/jseminck)<br />[💻](https://github.com/paypal/downshift/commits?author=jseminck "Code") | [<img src="https://avatars3.githubusercontent.com/u/954596?v=4" width="100px;"/><br /><sub>Jesse Harlin</sub>](http://jesseharlin.net/)<br />[🐛](https://github.com/paypal/downshift/issues?q=author%3Athe-simian "Bug reports") [💡](#example-the-simian "Examples") |
| [<img src="https://avatars0.githubusercontent.com/u/1402095?v=4" width="100px;"/><br /><sub>Matt Parrish</sub>](https://github.com/pbomb)<br />[🔧](#tool-pbomb "Tools") [👀](#review-pbomb "Reviewed Pull Requests") | [<img src="https://avatars1.githubusercontent.com/u/11661846?v=4" width="100px;"/><br /><sub>thom</sub>](http://thom.kr)<br />[💻](https://github.com/paypal/downshift/commits?author=thomhos "Code") | [<img src="https://avatars2.githubusercontent.com/u/1088312?v=4" width="100px;"/><br /><sub>Vu Tran</sub>](http://twitter.com/tranvu)<br />[💻](https://github.com/paypal/downshift/commits?author=vutran "Code") | [<img src="https://avatars1.githubusercontent.com/u/74193?v=4" width="100px;"/><br /><sub>Codie Mullins</sub>](https://github.com/codiemullins)<br />[💻](https://github.com/paypal/downshift/commits?author=codiemullins "Code") [💡](#example-codiemullins "Examples") | [<img src="https://avatars3.githubusercontent.com/u/12202757?v=4" width="100px;"/><br /><sub>Mohammad Rajabifard</sub>](https://morajabi.me)<br />[📖](https://github.com/paypal/downshift/commits?author=morajabi "Documentation") [🤔](#ideas-morajabi "Ideas, Planning, & Feedback") | [<img src="https://avatars3.githubusercontent.com/u/9488719?v=4" width="100px;"/><br /><sub>Frank Tan</sub>](https://github.com/tansongyang)<br />[💻](https://github.com/paypal/downshift/commits?author=tansongyang "Code") | [<img src="https://avatars3.githubusercontent.com/u/5093058?v=4" width="100px;"/><br /><sub>Kier Borromeo</sub>](https://kierb.com)<br />[💡](#example-srph "Examples") |
| [<img src="https://avatars1.githubusercontent.com/u/8969456?v=4" width="100px;"/><br /><sub>Paul Veevers</sub>](https://github.com/paul-veevers)<br />[💻](https://github.com/paypal/downshift/commits?author=paul-veevers "Code") | [<img src="https://avatars2.githubusercontent.com/u/13622298?v=4" width="100px;"/><br /><sub>Ron Cruz</sub>](https://github.com/Ronolibert)<br />[📖](https://github.com/paypal/downshift/commits?author=Ronolibert "Documentation") | [<img src="https://avatars1.githubusercontent.com/u/13605633?v=4" width="100px;"/><br /><sub>Rick McGavin</sub>](http://rickmcgavin.github.io)<br />[📖](https://github.com/paypal/downshift/commits?author=rickMcGavin "Documentation") | [<img src="https://avatars0.githubusercontent.com/u/869669?v=4" width="100px;"/><br /><sub>Jelle Versele</sub>](http://twitter.com/vejersele)<br />[💡](#example-vejersele "Examples") | [<img src="https://avatars1.githubusercontent.com/u/202773?v=4" width="100px;"/><br /><sub>Brent Ertz</sub>](https://github.com/brentertz)<br />[🤔](#ideas-brentertz "Ideas, Planning, & Feedback") | [<img src="https://avatars3.githubusercontent.com/u/8015514?v=4" width="100px;"/><br /><sub>Justice Mba </sub>](https://github.com/Dajust)<br />[💻](https://github.com/paypal/downshift/commits?author=Dajust "Code") [📖](https://github.com/paypal/downshift/commits?author=Dajust "Documentation") [🤔](#ideas-Dajust "Ideas, Planning, & Feedback") | [<img src="https://avatars2.githubusercontent.com/u/3925281?v=4" width="100px;"/><br /><sub>Mark Ellis</sub>](http://mfellis.com)<br />[🤔](#ideas-ellismarkf "Ideas, Planning, & Feedback") |
| [<img src="https://avatars1.githubusercontent.com/u/3241922?v=4" width="100px;"/><br /><sub>us͡an̸df͘rien͜ds͠</sub>](http://ronak.io/)<br />[🐛](https://github.com/paypal/downshift/issues?q=author%3Ausandfriends "Bug reports") [💻](https://github.com/paypal/downshift/commits?author=usandfriends "Code") [⚠️](https://github.com/paypal/downshift/commits?author=usandfriends "Tests") | [<img src="https://avatars0.githubusercontent.com/u/474248?v=4" width="100px;"/><br /><sub>Robin Drexler</sub>](https://www.robin-drexler.com/)<br />[🐛](https://github.com/paypal/downshift/issues?q=author%3Arobin-drexler "Bug reports") [💻](https://github.com/paypal/downshift/commits?author=robin-drexler "Code") | [<img src="https://avatars0.githubusercontent.com/u/7406639?v=4" width="100px;"/><br /><sub>Arturo Romero</sub>](http://arturoromero.info/)<br />[💡](#example-arturoromeroslc "Examples") |
| [<img src="https://avatars1.githubusercontent.com/u/3241922?v=4" width="100px;"/><br /><sub>us͡an̸df͘rien͜ds͠</sub>](http://ronak.io/)<br />[🐛](https://github.com/paypal/downshift/issues?q=author%3Ausandfriends "Bug reports") [💻](https://github.com/paypal/downshift/commits?author=usandfriends "Code") [⚠️](https://github.com/paypal/downshift/commits?author=usandfriends "Tests") | [<img src="https://avatars0.githubusercontent.com/u/474248?v=4" width="100px;"/><br /><sub>Robin Drexler</sub>](https://www.robin-drexler.com/)<br />[🐛](https://github.com/paypal/downshift/issues?q=author%3Arobin-drexler "Bug reports") [💻](https://github.com/paypal/downshift/commits?author=robin-drexler "Code") | [<img src="https://avatars0.githubusercontent.com/u/7406639?v=4" width="100px;"/><br /><sub>Arturo Romero</sub>](http://arturoromero.info/)<br />[💡](#example-arturoromeroslc "Examples") | [<img src="https://avatars1.githubusercontent.com/u/275483?v=4" width="100px;"/><br /><sub>yp</sub>](http://algolab.eu/pirola)<br />[🐛](https://github.com/paypal/downshift/issues?q=author%3Ayp "Bug reports") [💻](https://github.com/paypal/downshift/commits?author=yp "Code") |
<!-- ALL-CONTRIBUTORS-LIST:END -->

This project follows the [all-contributors][all-contributors] specification.
Expand Down
47 changes: 47 additions & 0 deletions src/__tests__/downshift.get-root-props.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,51 @@ test('renders fine when rendering a composite component and applying getRootProp
expect(() => mount(<MyComponent />)).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 = () => (
<Downshift>
{({getRootProps}) => (
<MyDiv {...getRootProps({}, {suppressRefError: true})} />
)}
</Downshift>
)
expect(() => mount(<MyComponent />)).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 = () => (
<Downshift>
{({getRootProps}) => (
<div {...getRootProps({refKey: 'blah'}, {suppressRefError: true})} />
)}
</Downshift>
)
expect(() => mount(<MyComponent />)).not.toThrow()
})

test('not applying the ref prop results in an error does not result in an error if suppressRefError is true', () => {
const MyComponent = () => (
<Downshift>
{({getRootProps}) => {
const {onClick} = getRootProps({}, {suppressRefError: true})
return <div onClick={onClick} />
}}
</Downshift>
)
expect(() => mount(<MyComponent />)).not.toThrow()
})

test('renders fine when rendering a composite component and applying getRootProps properly even if suppressRefError is true', () => {
const MyComponent = () => (
<Downshift>
{({getRootProps}) => (
<MyDiv
{...getRootProps({refKey: 'innerRef'}, {suppressRefError: true})}
/>
)}
</Downshift>
)
expect(() => mount(<MyComponent />)).not.toThrow()
})

/* eslint no-console:0 */
11 changes: 9 additions & 2 deletions src/downshift.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down