Add Test and add myself to list of contributors #60
Conversation
Codecov Report
@@ Coverage Diff @@
## master #60 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 1 1
Lines 18 18
=====================================
Hits 18 18Continue to review full report at Codecov.
|
src/__tests__/react-context.js
Outdated
| test('Component renders with the correct message, correctly changes message after clicking butotn', () => { | ||
| const {getByTestId} = render(<Container />) | ||
| expect(getByTestId('message').textContent).toBe('Hidden') | ||
| Simulate.click(getByTestId('button')) |
There was a problem hiding this comment.
Can we use here getByText method instead?
src/__tests__/react-context.js
Outdated
| const {getByTestId} = render(<Container />) | ||
| expect(getByTestId('message').textContent).toBe('Hidden') | ||
| Simulate.click(getByTestId('button')) | ||
| expect(getByTestId('message').textContent).toBe('Secret Message') |
There was a problem hiding this comment.
There is a custom jest matcher called toHaveTextContent, may be we can use that here..
There was a problem hiding this comment.
Okay so I changed it to getByText but I am not sure how to use toHaveTextContent since it is a custom jest matcher. How can I pull that in? thank you!
src/__tests__/react-context.js
Outdated
| <StateContext.Provider | ||
| value={{ | ||
| toggleStatus: this.state.toggleStatus, | ||
| handleToggle: this.handleToggle, |
There was a problem hiding this comment.
TL;DR: if you could rewrite this so that handleToggle is in state (you'll have to move it above the state initialization) then pass this.state into value, that'd be great! Thanks!
This is a pretty common mistake with the context API. It's actually not a problem in this specific scenario because it's simple, but I'd prefer to avoid exacerbating the common mistake by having an example here :)
Anyway, the problem here is that you generally shouldn't have a value assigned to a value that is initialized in the render method. Every time the value is changed, React will re-render all consumers, even if the object is the same.
To solve this problem, the suggested solution is to put everything into state (including event handlers). That is, if you want to expose everything that's in state to consumers. If you don't, then the convention is to set a property on state called context and pass this.state.context, which actually makes things more complicated when updating the context, but avoids unnecessary re-renders.
src/__tests__/react-context.js
Outdated
| } | ||
| } | ||
|
|
||
| test('Component renders with the correct message, correctly changes message after clicking butotn', () => { |
|
Okay, I think it is ready to go. Please let me know if there is anything else that needs to be fixed 😄 |
|
🎉 This PR is included in version 2.2.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
A test using the new react context API was added.
I felt like it was a good example and I just wanted to contribute something to this great library.
They are all in one file react-context.js
This is my first Pull Request to an open source library. I hope that it might be helpful. Please let me know if there is something that needs to be changed and if it is not needed I totally understand.