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 option to turn connect refs on and off #151

Closed
wants to merge 1 commit into from
Closed

Add option to turn connect refs on and off #151

wants to merge 1 commit into from

Conversation

troch
Copy link

@troch troch commented Oct 11, 2015

An alternative way to deal with #141

Add prop connectOptions to Provider (put in context). Default connect options are {useRefs: true}, and options can be overwritten on each connect instance (so refs can be turned on or off globally and locally). If refs are used for testing purposes only, then it gives the ability to not have them in production.

useRefs is by default true to avoid introducing a breaking change, but IMO it would be best if it is false by default (and turned on for testing and debugging purposes).

@gaearon
Copy link
Contributor

gaearon commented Oct 11, 2015

This still doesn't feel like a right solution.

@troch
Copy link
Author

troch commented Oct 11, 2015

I cannot think of another way right now if detecting stateless functional components is not bullet proof 😣

@gaearon
Copy link
Contributor

gaearon commented Oct 11, 2015

I don't think we need to detect them. Maybe we should instead offer some other API for accessing refs. For example, we could accept instanceRef prop that would act as a callback ref. If user doesn't specify it, it would be undefined.

@gaearon
Copy link
Contributor

gaearon commented Oct 15, 2015

Thanks for the PR.
In the end I decided on a simpler approach: refs are now opt-in at the connect() call site.

Closed via 2d3d0be.

@gaearon gaearon closed this Oct 15, 2015
@troch troch deleted the optional-refs branch October 15, 2015 11:22
@troch
Copy link
Author

troch commented Oct 15, 2015

👍

foiseworth pushed a commit to foiseworth/react-redux that referenced this pull request Jul 30, 2016
Designed by Matthew Johnston
Closes reduxjs#151
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants