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

Can't compose Downshift with other 'render prop' components because of validateGetRootPropsCalledCorrectly #529

Closed
jf248 opened this issue Jul 26, 2018 · 5 comments

Comments

@jf248
Copy link
Contributor

jf248 commented Jul 26, 2018

  • downshift version: 2.0.19
  • node version:
  • npm (or yarn) version:

Reproduction repository:
https://codesandbox.io/s/jjk32o429y

Problem description:
(I've seen #235 and aware of the suppressRefError workaround but it doesn't work for this use case, as demonstrated.)

I'm trying to 'compose'/'chain' Downshift with another 'render prop' component. (My reproduction example is deliberately trivial but composability is, I think, a really useful property of the 'render prop' pattern.)

I've got the example working in this fork:
https://codesandbox.io/s/oo096p975y

I have had to hack the Downshift code to make it work:

  • In render(): this.getRootProps.called = true. (I do call it.)
  • In validateGetRootPropsCalledCorrectly(): change a throw new Error to console.warn
  • In input_handleBlur() add a console.log(this._rootNode) just to demonstrate it has been set correctly

Suggested solution:
Perhaps adding a suppressRefError prop to the Downshift component itself to allow this use case?

And thanks so much for Downshift!

@jf248 jf248 changed the title Can't compose Downshift with other 'renderProps' components because of validateGetRootPropsCalledCorrectly Can't compose Downshift with other 'render props' components because of validateGetRootPropsCalledCorrectly Jul 26, 2018
@jf248 jf248 changed the title Can't compose Downshift with other 'render props' components because of validateGetRootPropsCalledCorrectly Can't compose Downshift with other 'render prop' components because of validateGetRootPropsCalledCorrectly Jul 26, 2018
@kentcdodds
Copy link
Member

What if you did this: https://codesandbox.io/s/93kqpy8lvp

@jf248
Copy link
Contributor Author

jf248 commented Jul 27, 2018

Yes, that works, but it entangles the presentational root div inside the logic components (Downshift and Foo). It prevents us extracting a reusable pure logic component DownshiftWithFoo:

const DownshiftWithFoo = () => <Downshift>{() => <Foo/>}</Downshift>

@kentcdodds
Copy link
Member

Interesting... Ok, I'd be fine with an (undocumented) prop for this use case. PRs accepted. Thanks @jf248!

@jf248
Copy link
Contributor Author

jf248 commented Jul 27, 2018

Thanks, I'll send a PR

@martinpgaunt
Copy link

martinpgaunt commented Jul 30, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants