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.

Sign up for GitHub

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

Jump to bottom

Warn about string refs within strict mode trees #12161

Merged
merged 2 commits into from
Feb 6, 2018
Merged

Warn about string refs within strict mode trees #12161

merged 2 commits into from
Feb 6, 2018

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Feb 6, 2018

Took an initial stab at warning about string refs within a strict tree.

There are a couple of things I feel uncertain about:

  • What's a better/more informative warning message than the one I wrote?
  • Is the approach of deduping based on the returnFiber display name reasonable?
  • For the time being, I've linked fb.me/react-strict-mode-string-ref to a little blurb in our current documentation. Where should I link it, long term?

Resolves #12158. Relates to #1373.

sebmarkbage
sebmarkbage approved these changes Feb 6, 2018
View reviewed changes
Copy link
Collaborator

@sebmarkbage sebmarkbage left a comment

Choose a reason for hiding this comment

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

👍The returnFiber approach seems fine.

@bvaughn
Copy link
Contributor Author

bvaughn commented Feb 6, 2018

Thanks for confirming!

Any suggestions on the wording of the warning? Okay for now?

trueadm
trueadm approved these changes Feb 6, 2018
View reviewed changes
Copy link
Contributor

@trueadm trueadm left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM. The wording is good too :)

@bvaughn
Copy link
Contributor Author

bvaughn commented Feb 6, 2018

Thanks folks

@bvaughn bvaughn merged commit 6f2f55e into facebook:master Feb 6, 2018
@bvaughn bvaughn deleted the 12158-strict-mode-warnings-for-string-refs branch February 6, 2018 15:43
@bvaughn bvaughn mentioned this pull request Feb 6, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trueadm trueadm trueadm approved these changes

@sebmarkbage sebmarkbage sebmarkbage approved these changes

@sophiebits sophiebits Awaiting requested review from sophiebits

@gaearon gaearon Awaiting requested review from gaearon

@flarnie flarnie Awaiting requested review from flarnie

@acdlite acdlite Awaiting requested review from acdlite

Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bvaughn @sebmarkbage @trueadm @facebook-github-bot