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

Update docs for test utils add ons change #9331

Closed

Conversation

flarnie
Copy link
Contributor

@flarnie flarnie commented Apr 4, 2017

Wanted to get this up so I can get any corrections. Continuing to work on the rest of the document updates, which I can either push on separate branches as they are finished or on one branch/PR that gets updated as I go. I assume we will hold off merging until the actual 15.5 release happens.


Updating all references and docs on the React.addons.TestUtils and the
shallow renderer to refer to the correct targets.

Instead of:

const React = require('react');

// ...
React.addons.Testutils
// or

const ReactTestUtils = require('react-addons-test-utils');

we now show:

const ReactTestUtils = require('react-dom/test-utils');

And for shallow renderer, instead of:

const shallowRenderer = TestUtils.createRenderer();

we now show:

const shallowRenderer = require('react-test-renderer/shallow');

Also did a commit to update the incorrect or missing 'prev' and 'next' attributes on the 'add-ons' doc pages.

Updating all references and docs on the `React.addons.TestUtils` and the
shallow renderer to refer to the correct targets.

Instead of:
```
const React = require('react');

// ...
React.addons.Testutils
// or

const ReactTestUtils = require('react-addons-test-utils');
```
we now show:
```
const ReactTestUtils = require('react-dom/test-utils');
```

And for shallow renderer, instead of:
```
const shallowRenderer = TestUtils.createRenderer();
```

we now show:
```
const shallowRenderer = require('react-test-renderer/shallow');
```
These flags are used to set arrow links to easily navigate through the
documents. They were wrong or missing in some of the 'add-ons' pages and
this bothered me when manually testing the updates from the previous
commit.
@gaearon
Copy link
Collaborator

gaearon commented Apr 4, 2017

What do you think it the right thing to do for other addons? Given they're 15-only. I'm thinking maybe move their docs to Gists and set up permanent redirects so they're gone from the website but keep working.


Shallow rendering lets you render a component "one level deep" and assert facts about what its render method returns, without worrying about the behavior of child components, which are not instantiated or rendered. This does not require a DOM.

- [`createRenderer()`](#createrenderer)
Copy link
Contributor

Choose a reason for hiding this comment

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

What's this refer to? Maybe accidentally left over from before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup - you are right, I had guessed that we were not using 'createRenderer' any more. Will fix.

Then you can assert:

```javascript
const renderer = ReactTestUtils.createRenderer();
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the old way of doing things. The new way should be:

const ReactShallowRenderer = require('react-test-renderer/shallow');
const renderer = new ReactShallowRenderer();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks - will fix.

@flarnie
Copy link
Contributor Author

flarnie commented Apr 4, 2017

"What do you think it the right thing to do for other addons? Given they're 15-only. I'm thinking maybe move their docs to Gists and set up permanent redirects so they're gone from the website but keep working."

That seems ok to me, and in that case we would want warnings that they are "legacy" and won't be supported past v15.5.

For now we could also just remove them from the side-nav to unblock the 15.5 release, and then move them to gists and set up the redirects.

Missed this when updating the docs for the changes to shallow-renderer
in React 15.5.
Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

I think I'm seeing a pointer problem.

shallow-compare next points to two-way-binding-helpers but two-way-binding-helpers previous points to pure-render-mixin. (It should point back to shallow-compare).

Similarly, two-way-binding-helpers next points to perf but perf previous points to create-fragment. (Should point back to two-way-binding-helpers.)


```javascript
import shallowRenderer from 'react-test-renderer/shallow'; // ES6
var shallowRenderer = require('react-test-renderer/shallow'); // ES5 with npm
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think we might want to rename these to ReactShallowRenderer to reduce ambiguity between what's returned by the import (the class) and the instance you render with.

import ReactShallowRenderer from 'react-test-renderer/shallow'; // ES6
var ReactShallowRenderer = require('react-test-renderer/shallow'); // ES5 with npm

I should have mentioned this before. Sorry.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's cool - will fix.

Thanks @bvaughn for catching this
We should show using the same variable names between code samples.
Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

👍


Shallow rendering lets you render a component "one level deep" and assert facts about what its render method returns, without worrying about the behavior of child components, which are not instantiated or rendered. This does not require a DOM.

- [`shallowRenderer.render()`](#shallowrenderer.render)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's change these to say ReactShallowRenderer too? Or maybe drop React there and just ShallowRenderer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The pattern we show first is

const ReactShallowRenderer = require('react-test-renderer/shallow');
const renderer = new ReactShallowRenderer();

It makes sense to reserve the UpperCamelCase for the class itself, and for instances of the class use lowerSnakeCase. What we need still is consistency between the naming of the instances. I would vote for shallowRenderer instead of renderer.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah I see. Thanks for explaining.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(I agree)

We should use the same variable name for the same thing across examples.
`renderer` -> `shallowRenderer`.
@flarnie flarnie mentioned this pull request Apr 7, 2017
2 tasks
@acdlite acdlite closed this Apr 7, 2017
flarnie added a commit to flarnie/react that referenced this pull request May 23, 2017
**what is the change?:**
- Remove outdated 'shallow renderer' docs on 'test utils' page, and point to the updated 'shallow renderer' docs.
- Re-add a link to the updated 'shallow renderer' docs on the main navigation.

**why make this change?:**
This was already approved in facebook#9331 which was then cherry-picked to https://github.com/facebook/react/pull/9359/commits and landed on master.

I'm not sure why some of these changes didn't persist. For now just adding back the changes we need.

**test plan:**
Manually inspected website - will insert screenshots.

**issue:**
flarnie added a commit to flarnie/react that referenced this pull request May 25, 2017
**what is the change?:**
- Remove outdated 'shallow renderer' docs on 'test utils' page, and point to the updated 'shallow renderer' docs.
- Re-add a link to the updated 'shallow renderer' docs on the main navigation.

**why make this change?:**
This was already approved in facebook#9331 which was then cherry-picked to https://github.com/facebook/react/pull/9359/commits and landed on master.

I'm not sure why some of these changes didn't persist. For now just adding back the changes we need.

**test plan:**
Manually inspected website - will insert screenshots.

**issue:**
flarnie added a commit that referenced this pull request May 26, 2017
* Add 'Test Utils' docs back to main navigation

**why make this change?:**
We accidentally removed this - still supporting the use of Test Utilities, so we should have them in the docs.

**test plan:**
Manually tested the website - will insert a screenshot.

**issue:**
#9651

* Move test-utils docs to reference section

**what is the change?:**
Moved from 'advanced guides' to 'reference'

**why make this change?:**
It makes more sense as a reference

**test plan:**
Visual inspection (flarnie may add a screenshot)

**issue:**

* Add back the shallow renderer docs and remove outdated docs

**what is the change?:**
- Remove outdated 'shallow renderer' docs on 'test utils' page, and point to the updated 'shallow renderer' docs.
- Re-add a link to the updated 'shallow renderer' docs on the main navigation.

**why make this change?:**
This was already approved in #9331 which was then cherry-picked to https://github.com/facebook/react/pull/9359/commits and landed on master.

I'm not sure why some of these changes didn't persist. For now just adding back the changes we need.

**test plan:**
Manually inspected website - will insert screenshots.

**issue:**

* Further improvements to 'shallow rendering' and 'test utils' docs

Thanks @gaearon for the improvements!

**what is the change?:**
- Remove <hr/> from end of 'shallow rendering' docs
- 'documents' -> 'documentation'
- Move 'shallow rendering' redirection section to top of 'test utils' docs
- Add intro sentence about testing to 'shallow rendering' docs

**why make this change?:**
Documentation helps people learn.

**test plan:**
Visual inspection
gaearon pushed a commit that referenced this pull request Jun 9, 2017
* Add 'Test Utils' docs back to main navigation

**why make this change?:**
We accidentally removed this - still supporting the use of Test Utilities, so we should have them in the docs.

**test plan:**
Manually tested the website - will insert a screenshot.

**issue:**
#9651

* Move test-utils docs to reference section

**what is the change?:**
Moved from 'advanced guides' to 'reference'

**why make this change?:**
It makes more sense as a reference

**test plan:**
Visual inspection (flarnie may add a screenshot)

**issue:**

* Add back the shallow renderer docs and remove outdated docs

**what is the change?:**
- Remove outdated 'shallow renderer' docs on 'test utils' page, and point to the updated 'shallow renderer' docs.
- Re-add a link to the updated 'shallow renderer' docs on the main navigation.

**why make this change?:**
This was already approved in #9331 which was then cherry-picked to https://github.com/facebook/react/pull/9359/commits and landed on master.

I'm not sure why some of these changes didn't persist. For now just adding back the changes we need.

**test plan:**
Manually inspected website - will insert screenshots.

**issue:**

* Further improvements to 'shallow rendering' and 'test utils' docs

Thanks @gaearon for the improvements!

**what is the change?:**
- Remove <hr/> from end of 'shallow rendering' docs
- 'documents' -> 'documentation'
- Move 'shallow rendering' redirection section to top of 'test utils' docs
- Add intro sentence about testing to 'shallow rendering' docs

**why make this change?:**
Documentation helps people learn.

**test plan:**
Visual inspection

(cherry picked from commit 114b9c5)
flarnie added a commit that referenced this pull request Jun 12, 2017
* Add 'Test Utils' docs back to main navigation

**why make this change?:**
We accidentally removed this - still supporting the use of Test Utilities, so we should have them in the docs.

**test plan:**
Manually tested the website - will insert a screenshot.

**issue:**
#9651

* Move test-utils docs to reference section

**what is the change?:**
Moved from 'advanced guides' to 'reference'

**why make this change?:**
It makes more sense as a reference

**test plan:**
Visual inspection (flarnie may add a screenshot)

**issue:**

* Add back the shallow renderer docs and remove outdated docs

**what is the change?:**
- Remove outdated 'shallow renderer' docs on 'test utils' page, and point to the updated 'shallow renderer' docs.
- Re-add a link to the updated 'shallow renderer' docs on the main navigation.

**why make this change?:**
This was already approved in #9331 which was then cherry-picked to https://github.com/facebook/react/pull/9359/commits and landed on master.

I'm not sure why some of these changes didn't persist. For now just adding back the changes we need.

**test plan:**
Manually inspected website - will insert screenshots.

**issue:**

* Further improvements to 'shallow rendering' and 'test utils' docs

Thanks @gaearon for the improvements!

**what is the change?:**
- Remove <hr/> from end of 'shallow rendering' docs
- 'documents' -> 'documentation'
- Move 'shallow rendering' redirection section to top of 'test utils' docs
- Add intro sentence about testing to 'shallow rendering' docs

**why make this change?:**
Documentation helps people learn.

**test plan:**
Visual inspection

(cherry picked from commit 114b9c5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants