This repository has been archived by the owner on Mar 27, 2021. It is now read-only.
add information about usage with react-redux to the readme #77
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cweiss-stripe
force-pushed
the
cw-redux
branch
from
1010645 to
b2d46f5
Compare
michelle
reviewed
Sep 13, 2017
README.md
Outdated
| @@ -270,6 +270,30 @@ type FactoryProps = { | |||
| }; | |||
| ``` | |||
|
|
|||
| ## Troubleshooting | |||
|
|
|||
| `react-stripe-elements` may not work properly when being used with components that implement `shouldComponentUpdate`. `react-stripe-elements` relies heavily on React's `context` feature and `shouldComponentUpdate` does not provide a way to take context updates into account when deciding whether to allow a re-render and hence can block context updates from reaching components. | |||
There was a problem hiding this comment.
nit: extra word, when being used -> when used
michelle
reviewed
Sep 13, 2017
README.md
Outdated
| @@ -270,6 +270,30 @@ type FactoryProps = { | |||
| }; | |||
| ``` | |||
|
|
|||
| ## Troubleshooting | |||
|
|
|||
| `react-stripe-elements` may not work properly when being used with components that implement `shouldComponentUpdate`. `react-stripe-elements` relies heavily on React's `context` feature and `shouldComponentUpdate` does not provide a way to take context updates into account when deciding whether to allow a re-render and hence can block context updates from reaching components. | |||
There was a problem hiding this comment.
nit: run-on -> "...deciding whether to allow a re-render. These components can block context updates from reaching react-stripe-element components in the tree."
michelle
reviewed
Sep 13, 2017
README.md
Outdated
|
|
||
| `react-stripe-elements` may not work properly when being used with components that implement `shouldComponentUpdate`. `react-stripe-elements` relies heavily on React's `context` feature and `shouldComponentUpdate` does not provide a way to take context updates into account when deciding whether to allow a re-render and hence can block context updates from reaching components. | ||
|
|
||
| For example when using `react-stripe-elements` together with [`react-redux`](https://github.com/reactjs/react-redux) doing the following will not work: |
There was a problem hiding this comment.
nit: For example -> For example,
michelle
reviewed
Sep 13, 2017
README.md
Outdated
| ```js | ||
| const Component = connect()(injectStripe(_Component)); | ||
| ``` | ||
| In this case, the context updates originating from the `StripeProvider` are not reaching the components wrapped inside the `connect` function and therefore the behavior of `react-stripe-elements` breaks. The reason is that the `connect` function of `react-redux` [implements `shouldComponentUpdate`](https://github.com/reactjs/react-redux/blob/master/docs/troubleshooting.md#my-views-arent-updating-when-something-changes-outside-of-redux) and blocks re-renders that are triggered by context changes outside of the connected component. |
There was a problem hiding this comment.
nit: run-on -> 'inside the connect function. Therefore, react-stripe-elements components deeper in the tree break.'
|
wording nits! otherwise lgtm :) |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
r? @fred-stripe
cc @michelle
This PR adds some information about the usage of
react-stripe-elementstogether withreact-reduxto the readme.This addresses #55
Link to the pretty version: https://github.com/stripe/react-stripe-elements/blob/b2d46f532b48636f8e6c74db26fb15f030096b8b/README.md#troubleshooting