React: Delimit dynamic part of the warning messages with newlines

[gaearon]

As proposed in https://github.com/facebook/react/pull/8495#issuecomment-271311978, I think we should find the warnings where we add a dynamic part to the end (like "Check the render method of <...>") and delimit it with two newlines. This way it’s much easier to recognize in the middle of a bunch of errors:

This is a good first issue to contribute. You would need to find warning() calls that include additional info like Check the render method of and add a couple of newlines. You'd also need to change the tests in case they fail.

[gaearon]

Assigning myself so I don’t forget about this issue.
But this is a good place to get your feet wet with the React codebase.

I don’t intend to work on this myself but will be happy to review your PR.

[richiethomas]

I'd like to work on this issue. :)

[gaearon]

👍 Please send a PR and ping me!

[richiethomas]

@gaearon - while browsing through 'codebase-overview.md', I noticed a potential typo in the markdown. On line 116, there appears to be one too many "`" marks, resulting in the rest of the file appearing as code, instead of regular text with code excerpts interspersed when relevant:

I'm happy to fix this as well, but should I create a separate issue for this and submit a PR, or just bundle the fix together with my work on issue #8719?

Here's what the markdown would look like with one less "`" symbol:

[gaearon]

Please create a separate PR.

[gaearon]

While unrelated, I generally recommend reading docs on the website:
https://facebook.github.io/react/contributing/codebase-overview.html

[AshikNesin]

@richiethomas You are working on this issue ?

[richiethomas]

@AshikNesin yes, I am. :)

@gaearon I'd like to make sure I have it right before submitting a PR. As an example of the change being requested, one of the files I'd change would be ReactElementValidator.js, line 36. Looks like I would add 2 newlines at the start of the string that's returned from the 'getDeclarationErrorAddendum' function. In other words, change:

' Check the render method of ' + name + '.';

to this:

'\n\n Check the render method of ' + name + '.';

If I have it right, I'll submit a PR for this fix later today.

[gaearon]

Something like this. But there's a few more similar places you'd need to change.

[richiethomas]

Yep, that's what I understood. Will submit a PR soon. Cheers.

[richiethomas]

I caught all dynamically-generated warnings which contain the phrase "Check", but I also found similar warnings with different verbage so I'm doing a more fine-grained search now. If it's OK, I'll submit a follow-up PR if I find anything.

[gaearon]

Best to put them all in one PR I think.

[richiethomas]

OK. So that I understand, the spirit of this bugfix is to only address the dynamically-generated warnings with large blocks of text, right? If a warning has dynamic content but is small, easily parseable, and clearly shows which file is problematic, there's no need for a newline, right?

[gaearon]

Yes. I want to fix those that are easy to miss and add info at the very end of a long message.

[richiethomas]

@gaearon as requested, here's a ping to say that this PR is ready for review. Cheers! :)