Skip to content

Commit 73d0e33

Browse files
made sure traverseHooks does not double-push snapshot for each hooks update
1 parent cf1461f commit 73d0e33

File tree

6 files changed

+52
-25
lines changed

6 files changed

+52
-25
lines changed

package.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,12 @@
2222
"contributors": [
2323
"Andy Wong",
2424
"Bryan Lee",
25+
"Chris Flannery",
2526
"David Chai",
2627
"Josh Kim",
28+
"Prasanna Malla",
29+
"Rajeeb Banstola",
30+
"Rocky Lin",
2731
"Ruth Anam",
2832
"Ryan Dang",
2933
"Sierra Swaby",

package/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const mode = {
99
const linkFiber = require('./linkFiber')(snapShot, mode);
1010
const timeJump = require('./timeJump')(snapShot, mode);
1111

12-
window.addEventListener('message', ({ data: { action, payload } }) => {
12+
window.addEventListener('message', ({ data: { action, payload } }) => { //runs automatically twice per second with inspectedElement
1313
switch (action) {
1414
case 'jumpToSnap':
1515
timeJump(payload);

package/linkFiber.js

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ const { saveState } = require('./masterState'); // saves AST state as array for
1111

1212
module.exports = (snap, mode) => {
1313
// snap is the current tree
14-
// mode is {jumping: bool, locked: bool, paused: bool}
14+
// mode is {jumping: bool, locked: bool, paused: bool
1515

1616
let fiberRoot = null;
1717
let astHooks;
@@ -34,7 +34,7 @@ module.exports = (snap, mode) => {
3434
if (component.setState.linkFiberChanged) {
3535
// console.log("setState has already been changed for", component);
3636
return;
37-
};
37+
}
3838
// make a copy of setState
3939
const oldSetState = component.setState.bind(component);
4040
// replace component's setState so developer doesn't change syntax
@@ -46,27 +46,31 @@ module.exports = (snap, mode) => {
4646
// continue normal setState functionality, except add sending message (to chrome extension) middleware
4747
oldSetState(state, () => {
4848
updateSnapShotTree(); // this doubles the actions in reactime for star wars app, also invokes changeSetState twice, also invokes changeSetState with Route and Characters
49-
sendSnapshot(); //runs once on page load, after event listener: message (line 145)
49+
sendSnapshot(); // runs once on page load, after event listener: message (line 145)
5050
callback.bind(component)(); // WHY DO WE NEED THIS ?
5151
});
5252
};
5353
component.setState.linkFiberChanged = true; // we changed setState.
5454
}
5555

5656
function changeUseState(component) { // if invoked, change useState dispatch functionality so that it also updates our snapshot
57-
//check that changeUseState hasn't been changed yet
57+
// check that changeUseState hasn't been changed yet
5858
if (component.queue.dispatch.linkFiberChanged) return;
5959
// store the original dispatch function definition
60-
const oldDispatch = component.queue.dispatch.bind(component.queue);
60+
61+
// not sure why we need the bind, seems to work without it
62+
// const oldDispatch = component.queue.dispatch.bind(component.queue);
63+
const oldDispatch = component.queue.dispatch;
64+
6165
// redefine the dispatch function so we can inject our code
6266
component.queue.dispatch = (fiber, queue, action) => {
6367
// don't do anything if state is locked, UNLESS we are currently jumping through time
6468
if (mode.locked && !mode.jumping) return;
65-
oldDispatch(fiber, queue, action);
66-
setTimeout(() => {
67-
updateSnapShotTree();
68-
sendSnapshot();
69-
}, 100);
69+
oldDispatch(fiber, queue, action); // hooks sees this and thinks its a side effect, that's why it's throwing an error
70+
// setTimeout(() => {
71+
updateSnapShotTree();
72+
sendSnapshot();
73+
// }, 100);
7074
};
7175
component.queue.dispatch.linkFiberChanged = true;
7276
}
@@ -79,8 +83,13 @@ module.exports = (snap, mode) => {
7983
let index = 0;
8084
astHooks = Object.values(astHooks);
8185
// while memoizedState is truthy, save the value to the object
82-
while (memoizedState) {
83-
changeUseState(memoizedState);
86+
while (memoizedState && memoizedState.queue !== null) {
87+
// we only want to changeUseState (which updates and sends the snapshot)
88+
// on the last item in the memoizedState chain. This makes sure it doesn't double-push
89+
// values to the timeline.
90+
if (memoizedState.next === null) {
91+
changeUseState(memoizedState);
92+
}
8493
// memoized[astHooks[index]] = memoizedState.memoizedState;
8594
memoized[astHooks[index]] = memoizedState.memoizedState;
8695
// Reassign memoizedState to its next value
@@ -103,7 +112,7 @@ module.exports = (snap, mode) => {
103112
elementType,
104113
} = currentFiber; // extract properties of current fiber
105114

106-
let childTree = tree; // initialize child fiber tree as current fiber tree
115+
let childTree = tree; // initialize child fiber tree as current fiber tree
107116
// check if stateful component
108117
if (stateNode && stateNode.state) {
109118
// add component to tree
@@ -112,6 +121,7 @@ module.exports = (snap, mode) => {
112121
changeSetState(stateNode);
113122
}
114123
// Check if the component uses hooks
124+
115125
if (memoizedState && Object.hasOwnProperty.call(memoizedState, 'baseState')) {
116126
// Traverse through the currentFiber and extract the getters/setters
117127
astHooks = astParser(elementType);
@@ -133,7 +143,7 @@ module.exports = (snap, mode) => {
133143
// but skips 1st hook click
134144
function updateSnapShotTree() {
135145
const { current } = fiberRoot; // on initial page load, current - fiberNode is tag type HostRoot (entire fiber tree)
136-
console.log("current", current);
146+
console.log('current', current);
137147
snap.tree = createTree(current);
138148
}
139149

@@ -154,8 +164,9 @@ module.exports = (snap, mode) => {
154164
window.addEventListener('message', ({ data: { action } }) => {
155165
if (action === 'contentScriptStarted') { // runs once on initial page load
156166
// console.log("in window.addEL")
157-
sendSnapshot()
158-
};
167+
console.log('page running');
168+
sendSnapshot();
169+
}
159170
});
160171
};
161172
};

src/extension/background.js

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
11
/* eslint-disable max-len */
22
/* eslint-disable no-param-reassign */
3+
34
// store ports in an array
45
const portsArr = [];
56
const reloaded = {};
67
const firstSnapshotReceived = {};
78
// there will be the same number of objects in here as there are reactime tabs open for each user application being worked on
89
const tabsObj = {};
910

11+
console.log("background scripts");
12+
1013
function createTabObj(title) {
14+
1115
// update tabsObj
1216
return {
1317
title,
@@ -65,7 +69,8 @@ function changeCurrLocation(tabObj, rootNode, index) {
6569

6670

6771
// establishing connection with devtools
68-
chrome.runtime.onConnect.addListener(port => {
72+
chrome.runtime.onConnect.addListener(port => { // port is one end of the connection - an object
73+
6974
// push every port connected to the ports array
7075
portsArr.push(port);
7176

@@ -87,8 +92,9 @@ chrome.runtime.onConnect.addListener(port => {
8792
}
8893
});
8994

90-
// receive snapshot from devtools and send it to contentScript
91-
port.onMessage.addListener(msg => {
95+
// receive snapshot from devtools and send it to contentScript -
96+
port.onMessage.addListener(msg => { // msg is action denoting a time jump in devtools
97+
9298
// ---------------------------------------------------------------
9399
// message incoming from devTools should look like this:
94100
// {
@@ -99,7 +105,7 @@ chrome.runtime.onConnect.addListener(port => {
99105
// ---------------------------------------------------------------
100106
const { action, payload, tabId } = msg;
101107
switch (action) {
102-
case 'import':
108+
case 'import': // create a snapshot property on tabId and set equal to tabs object
103109
tabsObj[tabId].snapshots = payload;
104110
return;
105111
case 'emptySnap':
@@ -247,6 +253,7 @@ chrome.runtime.onInstalled.addListener(() => {
247253
// when context menu is clicked, listen for the menuItemId,
248254
// if user clicked on reactime, open the devtools window
249255
chrome.contextMenus.onClicked.addListener(({ menuItemId }) => {
256+
console.log("clicked menu");
250257
const options = {
251258
type: 'panel',
252259
left: 0,

src/extension/contentScript.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
let firstMessage = true;
22

33
// listening for messages from npm package
4-
window.addEventListener('message', msg => {
4+
window.addEventListener('message', msg => { // runs automatically every second
55
// window listener picks up the message it sends, so we should filter
66
// messages sent by contentscript
77
if (msg.data.action !== 'contentScriptStarted' && firstMessage) {
@@ -13,11 +13,15 @@ window.addEventListener('message', msg => {
1313

1414
// post initial Message to background.js
1515
const { action } = msg.data;
16-
if (action === 'recordSnap') chrome.runtime.sendMessage(msg.data);
16+
17+
if (action === 'recordSnap') { // this is firing on page load
18+
chrome.runtime.sendMessage(msg.data);
19+
}
1720
});
1821

1922
// listening for messages from the UI
20-
chrome.runtime.onMessage.addListener(request => {
23+
chrome.runtime.onMessage.addListener(request => { // seems to never fire
24+
2125
// send the message to npm package
2226
const { action } = request;
2327
switch (action) {

webpack.config.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
const path = require('path');
2-
const ChromeExtensionReloader = require('webpack-chrome-extension-reloader');
2+
const ChromeExtensionReloader = require('webpack-chrome-extension-reloader'); //enable hot reloading while developing a chrome extension
33

44
const config = {
5+
// use a "multi-main entry" to inject multiple dependent files together and graph their dependencies into one "chunk"
56
entry: {
67
app: './src/app/index.js',
78
background: './src/extension/background.js',

0 commit comments

Comments
 (0)