Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
523b27d
Simplified profiler actualDuration timing
Aug 2, 2018
6f996da
Changed bubbling logic after chatting out of band with Andrew
Aug 3, 2018
8df2067
Added interaction-tracking package
Aug 4, 2018
974d25e
Incorporated feedback from Web Speed team
Aug 4, 2018
14dffc7
Overhalled continuations to work with React's scheduler
Aug 5, 2018
ac6887d
Fixed production tests
Aug 5, 2018
4418e72
Replaced __PROFILE__ with enableProfilerTimer feature flag
Aug 5, 2018
68bc9dd
Removed dep on shared/warning package
Aug 5, 2018
f277c09
Run interaction-tracking tests with profiling enabled in both DEV and…
Aug 6, 2018
112f2ec
Replaced console.error with warningWithouStack
Aug 6, 2018
5380b5f
Made InteractionTracking test internal since it depends on feature flags
Aug 6, 2018
ae7e4fa
Replaced __PROFILE__ with feature-flag conditionals in test
Aug 6, 2018
d98e713
Merge branch 'profiler-timer-bugfix' into interaction-tracking
Aug 6, 2018
ccc4f31
Updated test comment
Aug 6, 2018
7c88a28
Merge branch 'profiler-timer-bugfix' into interaction-tracking
Aug 6, 2018
2892738
Merge branch 'master' into interaction-tracking
Aug 8, 2018
332d87d
Redesigned interaction-tracking API to reduce runtime overhead
Aug 8, 2018
d47164a
Tidying up
Aug 8, 2018
75d9c9a
Naming nits
Aug 8, 2018
6b467ee
Moved interaction observer behind its own feature flag
Aug 9, 2018
d50a03c
Merge branch 'master' into interaction-tracking
Aug 10, 2018
8c0cefb
Refactored interaction-tracking API
Aug 10, 2018
d2148f5
Small naming and comment nits
Aug 10, 2018
d8e4665
More naming nits
Aug 10, 2018
1dc899a
Export more FLow types
Aug 10, 2018
dc98726
Prettier
Aug 10, 2018
e2ca33e
Removed async work count in favor of a __count attribute on Interaction
Aug 10, 2018
1fd5420
Deleted unused Flow type
Aug 11, 2018
56dba65
Added more Jest matchers for Interaction testing
Aug 11, 2018
dd7d78a
Added clear() method to reset interaction stack
Aug 13, 2018
cea2995
Fixed small innacurate comment
Aug 14, 2018
a511be1
cancelled -> canceled
Aug 14, 2018
83ac9fd
Removed 'interaction-tracking' dep in bundles
Aug 14, 2018
b8287cf
Improved subscriber error handling and added tests
Aug 14, 2018
012a9e6
Only support a single subscriber
Aug 14, 2018
0509a9f
let -> const
Aug 14, 2018
d5ebc31
Handle 'throw null'
Aug 14, 2018
4150607
Misc Seb feedback:
Aug 17, 2018
21248f8
callback(...args) -> callback.apply(undefined, arguments)
Aug 17, 2018
43a97f1
Added tests to ensure the first error was thrown
Aug 17, 2018
d88cbb9
Pass timestamp into track() method; delete now()
Aug 17, 2018
69fd285
Operator assignment for brevity
Aug 17, 2018
cc45d8e
Replaced multi try/catch with nested try/finally
Aug 17, 2018
d41bd61
Added some details to package.json
Aug 17, 2018
6e31ec3
Removed subscribe/unsubscribe methods from main export
Aug 17, 2018
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
Empty file.
12 changes: 12 additions & 0 deletions packages/interaction-tracking/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* Copyright (c) 2013-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

'use strict';

export * from './src/InteractionTracking';
7 changes: 7 additions & 0 deletions packages/interaction-tracking/npm/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use strict';

if (process.env.NODE_ENV === 'production') {
module.exports = require('./cjs/interaction-tracking.production.min.js');
} else {
module.exports = require('./cjs/interaction-tracking.development.js');
}
23 changes: 23 additions & 0 deletions packages/interaction-tracking/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"name": "interaction-tracking",
"description": "utility for tracking interaction events",
"version": "0.0.1",
"license": "MIT",
"files": [
"LICENSE",
"README.md",
"index.js",
"cjs/",
"umd/"
],
"keywords": [
"interaction",
"tracking",
"react"
],
"repository": "https://github.com/facebook/react",
"bugs": {
"url": "https://github.com/facebook/react/issues"
},
"homepage": "https://reactjs.org/"
}
289 changes: 289 additions & 0 deletions packages/interaction-tracking/src/InteractionTracking.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,289 @@
/**
* Copyright (c) 2013-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

import {
enableInteractionTracking,
enableInteractionTrackingObserver,
} from 'shared/ReactFeatureFlags';

export type Interaction = {|
__count: number,
id: number,
name: string,
timestamp: number,
|};

export type Subscriber = {
// A new interaction has been created via the track() method.
onInteractionTracked: (interaction: Interaction) => void,

// All scheduled async work for an interaction has finished.
onInteractionScheduledWorkCompleted: (interaction: Interaction) => void,

// New async work has been scheduled for a set of interactions.
// When this work is later run, onWorkStarted/onWorkStopped will be called.
// A batch of async/yieldy work may be scheduled multiple times before completing.
// In that case, onWorkScheduled may be called more than once before onWorkStopped.
// Work is scheduled by a "thread" which is identified by a unique ID.
onWorkScheduled: (interactions: Set<Interaction>, threadID: number) => void,

// A batch of scheduled work has been canceled.
// Work is done by a "thread" which is identified by a unique ID.
onWorkCanceled: (interactions: Set<Interaction>, threadID: number) => void,

// A batch of work has started for a set of interactions.
// When this work is complete, onWorkStopped will be called.
// Work is not always completed synchronously; yielding may occur in between.
// A batch of async/yieldy work may also be re-started before completing.
// In that case, onWorkStarted may be called more than once before onWorkStopped.
// Work is done by a "thread" which is identified by a unique ID.
onWorkStarted: (interactions: Set<Interaction>, threadID: number) => void,

// A batch of work has completed for a set of interactions.
// Work is done by a "thread" which is identified by a unique ID.
onWorkStopped: (interactions: Set<Interaction>, threadID: number) => void,
};

export type InteractionsRef = {
current: Set<Interaction>,
};

export type SubscriberRef = {
current: Subscriber | null,
};

const DEFAULT_THREAD_ID = 0;

// Counters used to generate unique IDs.
let interactionIDCounter: number = 0;
let threadIDCounter: number = 0;

// Set of currently tracked interactions.
// Interactions "stack"–
// Meaning that newly tracked interactions are appended to the previously active set.
// When an interaction goes out of scope, the previous set (if any) is restored.
let interactionsRef: InteractionsRef = (null: any);

// Listener(s) to notify when interactions begin and end.
// Note that subscribers are only supported when enableInteractionTrackingObserver is enabled.
let subscriberRef: SubscriberRef = (null: any);

if (enableInteractionTracking) {
interactionsRef = {
current: new Set(),
};
if (enableInteractionTrackingObserver) {
subscriberRef = {
current: null,
};
}
}

// These values are exported for libraries with advanced use cases (i.e. React).
// They should not typically be accessed directly.
export {interactionsRef as __interactionsRef, subscriberRef as __subscriberRef};

export function clear(callback: Function): any {
if (!enableInteractionTracking) {
return callback();
}

const prevInteractions = interactionsRef.current;
interactionsRef.current = new Set();

try {
return callback();
} finally {
interactionsRef.current = prevInteractions;
}
}

export function getCurrent(): Set<Interaction> | null {
if (!enableInteractionTracking) {
return null;
} else {
return interactionsRef.current;
}
}

export function getThreadID(): number {
return ++threadIDCounter;
}

export function track(
name: string,
timestamp: number,
callback: Function,
threadID: number = DEFAULT_THREAD_ID,
): any {
if (!enableInteractionTracking) {
return callback();
}

const interaction: Interaction = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will provide a hint to the VM that we only want 3 inline fields but they you add an expando later.

We should always provide the four fields in initialization. If you want to exclude on when the feature flag is not used you can initialize the object in a conditional.

In the three field case you can cast it to any and then back to Interaction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good call. Thanks!

__count: 0,
id: interactionIDCounter++,
name,
timestamp,
};

const prevInteractions = interactionsRef.current;

// Tracked interactions should stack/accumulate.
// To do that, clone the current interactions.
// The previous set will be restored upon completion.
const interactions = new Set(prevInteractions);
interactions.add(interaction);
interactionsRef.current = interactions;

if (enableInteractionTrackingObserver) {
// Update before calling callback in case it schedules follow-up work.
interaction.__count = 1;

let returnValue;
const subscriber = subscriberRef.current;

try {
if (subscriber !== null) {
subscriber.onInteractionTracked(interaction);
}
} finally {
try {
if (subscriber !== null) {
subscriber.onWorkStarted(interactions, threadID);
}
} finally {
try {
returnValue = callback();
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can actually just return here. The finally will still execute before returning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Returning in finally masks thrown errors.

} finally {
interactionsRef.current = prevInteractions;

try {
if (subscriber !== null) {
subscriber.onWorkStopped(interactions, threadID);
}
} finally {
interaction.__count--;

// If no async work was scheduled for this interaction,
// Notify subscribers that it's completed.
if (subscriber !== null && interaction.__count === 0) {
subscriber.onInteractionScheduledWorkCompleted(interaction);
}
}
}
}
}

return returnValue;
} else {
try {
return callback();
} finally {
interactionsRef.current = prevInteractions;
}
}
}

export function wrap(
callback: Function,
threadID: number = DEFAULT_THREAD_ID,
): Function {
Copy link
Collaborator

@acdlite acdlite Aug 17, 2018

Choose a reason for hiding this comment

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

I don't think I understand why wrap is so complicated. I was expecting it to be essentially this:

function wrap(callback) {
  const continuedInteractions = interactions.current;
  retain();
  return (...args) => {
    track(continuedInteractions, () => callback(...args));
    release();
  }
}

where most of the code is reused from track

Copy link
Collaborator

@acdlite acdlite Aug 17, 2018

Choose a reason for hiding this comment

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

I suppose avoiding the extra function call is worthwhile, but I believe semantically they should be identical. See other comment: #13234 (comment)

Copy link
Contributor Author

@bvaughn bvaughn Aug 17, 2018

Choose a reason for hiding this comment

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

I don't think wrap is that complicated. This approach avoids an extra function call and additional wrapper function.

Also the behavior of wrap/track are different. track is additive (stacking on top of what the current interactions are). wrap temporarily restores interactions at a previous point in time.

Copy link
Collaborator

Choose a reason for hiding this comment

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

But... why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because that's kind of fundamental to how track and wrap work? I'm not sure I understand the question.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It doesn’t modify the current set of interactions because it will reset them back to the previous set after it exits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I meant that it would modify the original set. So if I wrapped [foo,bar] I would expect my async work to be attributed to [foo,bar]– not [foo,bar,...whatever-else-was-also-active]

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 @acdlite's misunderstanding is a good opportunity to rename this function to something that makes it clearer. I could see how if I see track and wrap I would think that wrap is just a convenience for wrapTracked since it is highlighting the wrapping mechanism and nothing else.

In reality the purpose of wrap isn't to wrap a function. The primary purpose is to continue where you left off.

wrapContinuation? Too long. This is can be used a lot if you don't have auto-wrapping.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I suppose there's precedence for calling it wrap in Zones.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the brevity of "track" and "wrap" but I'm not bullish on them if others feel there are more meaningful names.

if (!enableInteractionTracking) {
return callback;
}

const wrappedInteractions = interactionsRef.current;

if (enableInteractionTrackingObserver) {
const subscriber = subscriberRef.current;
if (subscriber !== null) {
subscriber.onWorkScheduled(wrappedInteractions, threadID);
}

// Update the pending async work count for the current interactions.
// Update after calling subscribers in case of error.
wrappedInteractions.forEach(interaction => {
interaction.__count++;
});
}

const wrapped = () => {
const prevInteractions = interactionsRef.current;
interactionsRef.current = wrappedInteractions;

if (enableInteractionTrackingObserver) {
const subscriber = subscriberRef.current;

try {
let returnValue;

try {
if (subscriber !== null) {
subscriber.onWorkStarted(wrappedInteractions, threadID);
}
} finally {
try {
returnValue = callback.apply(undefined, arguments);
} finally {
interactionsRef.current = prevInteractions;

if (subscriber !== null) {
subscriber.onWorkStopped(wrappedInteractions, threadID);
}
}
}

return returnValue;
} finally {
// Update pending async counts for all wrapped interactions.
// If this was the last scheduled async work for any of them,
// Mark them as completed.
wrappedInteractions.forEach(interaction => {
interaction.__count--;

if (subscriber !== null && interaction.__count === 0) {
subscriber.onInteractionScheduledWorkCompleted(interaction);
}
});
}
} else {
try {
return callback.apply(undefined, arguments);
} finally {
interactionsRef.current = prevInteractions;
}
}
};

if (enableInteractionTrackingObserver) {
wrapped.cancel = () => {
const subscriber = subscriberRef.current;

try {
if (subscriber !== null) {
subscriber.onWorkCanceled(wrappedInteractions, threadID);
}
} finally {
// Update pending async counts for all wrapped interactions.
// If this was the last scheduled async work for any of them,
// Mark them as completed.
wrappedInteractions.forEach(interaction => {
interaction.__count--;

if (subscriber && interaction.__count === 0) {
subscriber.onInteractionScheduledWorkCompleted(interaction);
}
});
}
};
}

return wrapped;
}
Loading