React: [New Docs] Docs Bugs & Issues

[gaearon]

We just completely rewrote docs in #7864.
They're bound to cause some issues, have some mistakes, etc.
Let's keep this issue as an umbrella for all problems we're discovering after the initial rollout.

• [x] Old links shouldn't be 404! Major issue.
• [x] Topbar points to https://facebook.github.io/react/index.html which is ugly URL
• [x] We shouldn't use Number as an identifier https://github.com/facebook/react/issues/8034
• [x] Remove vars everywhere except ES5 page
• [x] JSX in depth has weird names like Story1, render2 @lacker
• [x] JSX in depth shows a failing example with <hello> tag but doesn't show how to fix it @lacker
• [ ] Localized pages are gone: need to set up a new streamlined localization effort (cc @thejameskyle)
• [ ] Mention in default index.html that it is not production-ready
• [ ] Mark mockComponent as legacy and unnecessary
  https://github.com/facebook/react/issues/2499#issuecomment-94044857
• [ ] shallowCompare on Addons page should recommend PureComponent instead
• [ ] Figure out what to do with Community section (https://github.com/facebook/react/pull/6099)
• [ ] Mention string refs
• [x] Forms doc has mistakes
• [ ] Forms doc uses setState for uncontrolled components which is confusing
• [ ] Installation is messy

[luqmaan]

Docs on Component API are 404ing: https://facebook.github.io/react/docs/component-specs.html#updating-componentwillreceiveprops

[gaearon]

Yea, something happened to all some redirects. @lacker

[gaearon]

@luqmaan Thanks, fixed. Let us know if you find more broken links.

[hzoo]

On Hello World, https://facebook.github.io/docs/installation.html should be https://facebook.github.io/react/docs/installation.html so the react/ isn't added and redirects to https://code.facebook.com/?

[Installation](/docs/installation.html) page

[johnbrett]

Great job on new docs all, love the live editor with error handling.

Just curious: Any reason why var is used on all the homepage tutorials? Curious whether there's a "best practice" on this.. would have thought const where possible, otherwise let?

[hzoo]

Another thing I noticed: this might of been an issue with the old docs but _syntax errors_ don't have correct formatting of newlines from babel-code-frame

Someone can make a pr to add to add style={{whiteSpace: "pre-wrap"}} as a style on the div at https://github.com/facebook/react/blob/eb118b70379ff01b30bb04dfce2c685c26d5453d/docs/_js/live_editor.js#L224 or edit the .playgroundError css class in https://github.com/facebook/react/blob/15-dev/docs/css/react.scss (<pre> seems to make it go over the width limit of the colored error box)

Width would need to be longer to be accurate but it would be better than before

[gaearon]

Any reason why var is used on all the homepage tutorials?

No reason, feel free to send a PR fixing this.

[pythononwheels]

If this is the "new style" I think you should point out the key differences to the "old style".
Just to take people with you.... knowing what key things change and why ...

[gaearon]

Does it matter at this point? It's not as much style as a lack of cohesion in previous docs.

They weren't really grouped by topic, there was no clear progression from simple to complex, their titles weren't descriptive, there were many random "tips" that didn't fit a cohesive narrative, they were using APIs that aren't used widely in the ecosystem anymore, etc.

New docs are in line with how people use React today, have descriptive titles, and a linear progression from simple to more complex topics.

That's the basic difference.

[p-adams]

Possible typo in docs: "Consider the ticking clock example from the one of the previous sections."

https://facebook.github.io/react/docs/state-and-lifecycle.html

[hnordt]

@gaearon the uncontrolled form example on https://facebook.github.io/react/docs/forms.html#uncontrolled-components is passing value, so it's actually a controlled example.

The correct example would be something like:

class Form extends React.Component {
  constructor(props) {
    super(props);
    this.handleSubmit = this.handleSubmit.bind(this);
  }

  handleSubmit(event) {
    alert("Text field value is: '" + this.textInput.value + "'");
  }

  render() {
    return (
      <div>
        <input
          type="text"
          placeholder="Hello!"
          ref={(input) => this.textInput = input}
        />
        <button onClick={this.handleSubmit}>Submit</button>
      </div>
    );
  }
}

ReactDOM.render(<Form />, document.getElementById('root'));

[giuseppeg]

A controlled component does not maintain its own internal state; the component renders purely based on props.

• [ ] is a bit confusing, probably better clarify that we are talking about DOM state instead of React state.

---

• [x] In thinking-in-react.html#step-4-identify-where-your-state-should-live the second last paragraph shouldn't mention getInitialState()

---

• [ ] In jsx-in-depth.html#javascript-expressions there are two headings with the same ID "JavaScript Expressions" - this makes it impossible to link to the second one.
• [x] The fourth example in the innermost one is missing props.:

// Calls the children callback numTimes to produce a repeated component
function Repeat(props) {
  var items = [];
  for (var i = 0; i < numTimes; i++) {
  // ... 
}

Here numTimes should be props.numTimes.

[Andreyco]

Why was info about latest stable removed from landing page?

[gaearon]

@Andreyco What do you mean? There was a "Download Starter Kit" link that mentioned version but most people didn't find that "starter kit" useful. You can always find the latest version on Releases GH page.

[gaearon]

Another feedback on "Controlled Components": https://github.com/facebook/react/issues/8052

[Andreyco]

@gaearon Yes, I mean link that mentioned version. I don't care about link, I am interested in version information. Anyway, no big deal, I will search for latest in GH releases. TY.

[gaearon]

Merged a bunch of PRs with fixes. Keep them coming please!

[gaearon]

https://facebook.github.io/react/blog/2016/07/22/create-apps-with-no-configuration.html looks very weird on mobile, with some sort of a broken top bar

[gaearon]

Feedback on tutorial https://twitter.com/iandavey773/status/789870405389475840
cc @spicyj

[gaearon]

Figure out if we do redirects correctly https://twitter.com/spiceee/status/789774523549745152

[giuseppeg]

@gaearon have you considered to replace redcarpet with kramdown?

[gaearon]

@giuseppeg No idea, want to send a PR so I better understand the difference?

[gaearon]

More feedback: people are used to seeing version number on homepage. Can we add it somewhere and also link to the changelog for good measure?

[simonsmith]

Just reading though the "React Without ES6" page:

In React components declared as ES6 classes, methods follow the same semantics as regular ES6 classes. This means that they don't automatically bind this to the instance. You'll have to explicitly use .bind(this) in the constructor:

Perhaps it's worth mentioning that this only applies in event handlers. Otherwise this works just fine. Happy to open a PR if so.

[gaearon]

@simonsmith Sounds good.

[aweary]

Perhaps it's worth mentioning that this only applies in event handlers. Otherwise this works just fine. Happy to open a PR if so.

@simonsmith @gaearon I don't think that's true if I'm understanding you correctly. You need to bind your method anytime it might be executed in a context where this does not refer to the component instance it was defined on.

class Bar extends React.Component {
  constructor(props) {
    super(props);
    // this will throw since it tries to access this.state.foo and
   // this.state is undefined on Bar
    this.props.logFoo();
  }
   render() {
    return <h1>Bar</h1>
  }

}

class Foo extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
        foo: "foo"
    };
  }
  logFoo() {
    console.log("Foo is: ", this.state.foo);
  }
  render() {
    return <Bar logFoo={this.logFoo} />
  }
}

An event handler is the most common case, but it's not the only case where you need to bind methods.

[gaearon]

The rule of thumb is that if you access method anywhere without () right after it, then you need to bind it.

[aweary]

The rule of thumb is that if you access method anywhere without () right after it, then you need to bind it.

Is that in the docs? If not, it should be 😄

[gaearon]

If it helps: https://twitter.com/dan_abramov/status/790612782471319553

[gaearon]

PRs welcome!

[simonsmith]

@Aweary Yeah, my point was the docs imply you need to use bind whenever you want to use an instance method which (as you've demonstrated) is only true some of the time.

I've seen code where every method was bound by hand even if it was just being called normally. I can see why people might think that was correct. I'll put together some changes and we can discuss in a PR 👍

[Shenlok]

No mention of the old string-based refs in the new docs. Should these be mentioned in a "deprecated" notice at least?

[gaearon]

@Shenlok They're technically not deprecated yet, just considered legacy.

Refs and the DOM does mention them but too briefly:

Using the ref callback just to set a property on the class is a common pattern for accessing DOM elements. If you are currently using this.refs.myRefName to access refs, we recommend using this pattern instead.

I agree we should add a small section at the bottom at least describing how string refs work, with a notice that we don't recommend using them. Could you send a PR for this?

[SMHFandA]

Forms docs has mistakes.
In "Forms" docs i found mistake in "Basic Radio Button" example. Method render() do not use this.state.value, but state has it. According "State and Lifecycle" docs it is bad:

While this.props is set up by React itself and this.state has a special meaning, you are free to add additional fields to the class manually if you need to store something that is not used for the visual output.
If you don't use something in render(), it shouldn't be in the state.

[julen]

_(Sorry if this has been discussed elsewhere — I haven't been able to find any references to it though)._

there were many random "tips" that didn't fit a cohesive narrative

I agree there was no narrative here, although at the same time many of these provided really good advice on anti-patterns and things to avoid. At least they have been very valuable to me on the way to learning React.

Now as I browse/search through the new docs, I am missing these and I am not sure whether they have been integrated under other sections or discarded by some reason. IMHO having a specific section on anti-patterns can be beneficial for newcomers to avoid making common mistakes (e.g. using props as initial state), as well as a reference to be able to point to others.

[gaearon]

@julen All valuable tips were integrated into the relevant content. There aren't many "antipatterns" in React despite what some people say. If something is "bad" we try to disallow it on the API level.

[crosscompile]

Tiny (possible?) typo: in reference-react.md, the code for React.Children.count omits React which could be slightly confusing to newcomers.

[joni-]

I was wondering about the example code in https://facebook.github.io/react/docs/thinking-in-react.html#step-4-identify-where-your-state-should-live.

I don't really understand why the filtering should be ProductTable's responsibility. Wouldn't it make more sense to do the filtering in FilterableProductTable and just pass the filtered products down to ProductTable?

[kalmanh]

@gaearon @aweary I know I am "beating a dead horse" on this (as a similar issue was discussed above in this thread), but after reading the following in the docs in handling events

You have to be careful about the meaning of this in JSX callbacks. In JavaScript, class methods are not bound by default. If you forget to bind this.handleClick and pass it to onClick, this will be undefined when the function is actually called.

This is not React-specific behavior; it is a part of how functions work in JavaScript.

you walk away thinking that you have to bind hi() and callAnother() methods in the following code

class Test {
    constructor(){
        this.someProperty = "this is a test";
    }

    hi(){
     console.log(`this in hi() =  ${JSON.stringify(this)}`);
     this.callAnother();
    }

    callAnother(){
     console.log(`this in callAnother() = ${JSON.stringify(this)}`);
    }
}

const c = new Test();
c.hi();

which is obviously not true.

So, what the paragraph is really trying to say is this, but it needs to do it better... :)

[gaearon]

@kalmanh Send a PR ;-)

[kkemple]

Tiny (possible?) typo: in reference-react.md, the code for React.Children.count omits React which could be slightly confusing to newcomers.

@jayjaycross did you want to fix that typo? If not I'll create a PR

[crosscompile]

@kkemple go for it!

[ivstas]

Not sure it is a proper place to post it, but "React documentation is Creative Commons licensed." in the end of README.md leads to 404 github page.

[monkindey]

yeah, I think the licence-doc was removed by this commit https://github.com/facebook/react/pull/11137/commits/cfaa072ea052350960b98aaba7de1e39ab0ca5ce , maybe we can remove the "React documentation is Creative Commons licensed." paragraph because of the migration of the document website.

[bvaughn]

The documentation and source code for reactjs.org now lives in a different repository: reactjs/reactjs.org. (For more info on why we made this move, see issue #11075.)

I've moved this issue to the new repo: reactjs/reactjs.org#84

Let's continue the discussion there!

[sag1v]

@bvaughn The reactjs/reactjs.org link you posted is broken i think you meant https://github.com/reactjs/reactjs.org/ ?