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

prevent spamming warnings related to performance measurement code #7299

Merged
merged 4 commits into from
Jul 19, 2016
Merged

prevent spamming warnings related to performance measurement code #7299

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2662a2d
prevent spamming warnings related to performance measurement code
sassanh Jul 18, 2016
5bfb08b
minor changes in names and such
sassanh Jul 18, 2016
2d51f65
-
sassanh Jul 18, 2016
c5d98dd
-
sassanh Jul 18, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 27 additions & 18 deletions src/renderers/shared/ReactDebugTool.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ var currentTimerStartTime = null;
var currentTimerNestedFlushDuration = null;
var currentTimerType = null;

var beginLifeCycleTimerHasWarned = false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it’s fine to have a single variable for this. If it warns once, something’s messed up already so it might warn more times.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just wanted to be safe about it. Thought maybe tests can produce situations that both functions generate warnings and then check the output of both. Changed it as you suggested in new commit.

var endLifeCycleTimerHasWarned = false;

function clearHistory() {
ReactComponentTreeDevtool.purgeUnmountedComponents();
ReactHostOperationHistoryDevtool.clearHistory();
Expand Down Expand Up @@ -109,15 +112,18 @@ function beginLifeCycleTimer(debugID, timerType) {
if (currentFlushNesting === 0) {
return;
}
warning(
!currentTimerType,
'There is an internal error in the React performance measurement code. ' +
'Did not expect %s timer to start while %s timer is still in ' +
'progress for %s instance.',
timerType,
currentTimerType || 'no',
(debugID === currentTimerDebugID) ? 'the same' : 'another'
);
if (!beginLifeCycleTimerHasWarned) {
Copy link
Collaborator

@gaearon gaearon Jul 18, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

You can move this inside:

warning(
  !currentTimerType || beginLifeCycleTimerHasWarned,
  ...
)

It only warns when the passed condition is false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great, didn't know how warning-'s api. Changed it as you suggested in new commit.

warning(
!currentTimerType,
'There is an internal error in the React performance measurement code. ' +
'Did not expect %s timer to start while %s timer is still in ' +
'progress for %s instance.',
timerType,
currentTimerType || 'no',
(debugID === currentTimerDebugID) ? 'the same' : 'another'
);
beginLifeCycleTimerHasWarned = true;
}
currentTimerStartTime = performanceNow();
currentTimerNestedFlushDuration = 0;
currentTimerDebugID = debugID;
Expand All @@ -128,15 +134,18 @@ function endLifeCycleTimer(debugID, timerType) {
if (currentFlushNesting === 0) {
return;
}
warning(
currentTimerType === timerType,
'There is an internal error in the React performance measurement code. ' +
'We did not expect %s timer to stop while %s timer is still in ' +
'progress for %s instance. Please report this as a bug in React.',
timerType,
currentTimerType || 'no',
(debugID === currentTimerDebugID) ? 'the same' : 'another'
);
if (!endLifeCycleTimerHasWarned) {
warning(
currentTimerType === timerType,
'There is an internal error in the React performance measurement code. ' +
'We did not expect %s timer to stop while %s timer is still in ' +
'progress for %s instance. Please report this as a bug in React.',
timerType,
currentTimerType || 'no',
(debugID === currentTimerDebugID) ? 'the same' : 'another'
);
endLifeCycleTimerHasWarned = true;
}
if (isProfiling) {
currentFlushMeasurements.push({
timerType,
Expand Down