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 bare minimum documentation #34

Merged
merged 2 commits into from
Jun 22, 2017

Conversation

vamsiampolu
Copy link
Contributor

@MicheleBertoli I took your comments from the issue and added it to the README #33

Copy link
Member

@MicheleBertoli MicheleBertoli left a comment

Choose a reason for hiding this comment

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

WOW, thanks for helping with the docs @vamsiampolu .
I just added a few comments, and I'm more than happy to merge this.

README.md Outdated
```
yarn add --dev jest-styled-components
```

This package must be used with either `react-test-renderer` or `enzyme` and `enzyme-to-json` to generate a serialized `tree` from your component.
Copy link
Member

Choose a reason for hiding this comment

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

This should be more generic, like "To render React components for testing you can use either [...]" because it doesn't apply to this package only.

Also, enzyme-to-json is only needed for snapshots testing.

README.md Outdated
import renderer from 'react-test-renderer'

// somewhere in your tests
it('matches the styled components snapshot', () => {
Copy link
Member

Choose a reason for hiding this comment

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

nit: test(

README.md Outdated
// somewhere in your tests
it('matches the styled components snapshot', () => {
const tree = renderer.create(<MyComponent />).toJSON()
expect(tree).toMatchStyledComponentsSnapshot()
Copy link
Member

Choose a reason for hiding this comment

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

nit: new line

README.md Outdated
})
```

For more info on using react-test-renderer with jest for snapshots see [this](https://facebook.github.io/jest/docs/snapshot-testing.html)
Copy link
Member

Choose a reason for hiding this comment

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

Can we rephrase this to "Lear more about Snapshot Testing with Jest." or something similar?

Also, I personally wrote the FAQs in that page :)

README.md Outdated

Usage:

This module is not compatible with shallow rendering, components must use the full DOM rendering with `mount`
Copy link
Member

Choose a reason for hiding this comment

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

This is not correct, the module works with both shallow and mount.

Copy link
Contributor Author

@vamsiampolu vamsiampolu Jun 22, 2017

Choose a reason for hiding this comment

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

I only added this because I saw this message:

Expected ${received} to be a component from react-test-renderer, or a mounted enzyme component.

here

Also, when is retrieving a child component asserting for its styles when it is within a larger component necessary and when is it to be avoided?

Copy link
Member

Choose a reason for hiding this comment

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

You are absolutely right, that message should be updated.
My idea was to make it similar to the toHaveStyleRule matcher for RN.

README.md Outdated
import toJSON from 'enzyme-to-json'

// inside your test
it('can use enzyme instead of react-test-renderer', () => {
Copy link
Member

Choose a reason for hiding this comment

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

nit: test(

README.md Outdated

```js
import {mount} from 'enzyme'
import toJSON from 'enzyme-to-json'
Copy link
Member

Choose a reason for hiding this comment

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

enzyme-to-json can be used like this in each test file, or it can be added globally (which is more convenient).

README.md Outdated
@@ -29,6 +83,9 @@ expect(tree).toMatchStyledComponentsSnapshot()

## toHaveStyleRule [React]

Only checks for the styles directly applied to the component it receives, to assert that a style has been applied to a child
Copy link
Member

Choose a reason for hiding this comment

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

to assert that a style has been applied to a child component

You can make assertion on a child component using Enzyme's find.
Snapshots should be used for testing complex selectors and/or the entire output in one go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you assign a style to a component from a library ( a component that you do not own), would you use toMatchSnapshot or would you try and find it, esp. if unstyled and styled elements of the same target elementt are in the subject component.

@terrencewwong
Copy link
Contributor

Thanks @vamsiampolu this has helped me too!

@MicheleBertoli
Copy link
Member

🙌 Thank you very much @vamsiampolu

@MicheleBertoli MicheleBertoli merged commit 31359df into styled-components:master Jun 22, 2017
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.

3 participants