performance optimizations

Author: aholachek

__tests__/makeProxyTest.js

@@ -0,0 +1,57 @@
+import { createMakeProxyFunction } from "../src/trackObjectUse"
+
+describe("basic proxy behavior", () => {
+  it("if a proxied object is proxied again, both proxies will be active, rather than the second proxy simply overriding the first", () => {
+    const handler1 = {
+      get(target, propKey) {
+        return "proxied get"
+      }
+    }
+
+    const handler2 = {
+      get(target, propKey) {
+        return `${target[propKey]} is wrapped with an outer proxy`
+      }
+    }
+    const proxy = new Proxy({ test: 1 }, handler1)
+    const wrappedProxy = new Proxy(proxy, handler2)
+    expect(proxy.test).toBe("proxied get")
+    expect(wrappedProxy.test).toBe("proxied get is wrapped with an outer proxy")
+  })
+  it("there can be a special hidden key that just returns the unproxied target to avoid this proxy nesting", () => {
+    const handler1 = {
+      get(target, propKey) {
+        if (propKey === "__initialVal") return target
+        if (target.__initialVal !== undefined) return target.__initialVal[propKey]
+        return "proxied get"
+      }
+    }
+
+    const handler2 = {
+      get(target, propKey) {
+        const value =
+          target.__initialVal !== undefined ? target.__initialVal[propKey] : target[propKey]
+        return `${value} is wrapped with an outer proxy`
+      }
+    }
+    const proxy = new Proxy({ test: 1 }, handler1)
+    const wrappedProxy = new Proxy(proxy, handler2)
+    expect(proxy.test).toBe("proxied get")
+    expect(proxy.__initialVal.test).toBe(1)
+    expect(wrappedProxy.test).toBe("1 is wrapped with an outer proxy")
+  })
+  it("JSON.parse(JSON.stringify(obj)) will return a non-proxied object from a proxied object, after calling the proxy's get methods", () => {
+    const handler1 = {
+      get: jest.fn(function(target, propKey) {
+        debugger
+        return "proxied get"
+      })
+    }
+
+    const proxy = new Proxy({ test: 1 }, handler1)
+    const copied = JSON.parse(JSON.stringify(proxy))
+    expect(handler1.get.mock.calls.length).toBe(2)
+    expect(copied.test).toBe("proxied get")
+    expect(handler1.get.mock.calls.length).toBe(2)
+  })
+})

package.json

@@ -4,7 +4,7 @@
   "description": "",
   "main": "dist/index.js",
   "scripts": {
-    "test": "jest",
+    "test": "NODE_ENV=test jest",
     "build": "NODE_ENV=production ./node_modules/.bin/babel src -d dist --ignore node_modules,src/lib; cp -r src/lib dist/",
     "prepublish": "npm run build"
   },
@@ -14,9 +14,8 @@
     "deep-object-diff": "^1.0.4",
     "fs": "^0.0.1-security",
     "install": "^0.10.4",
-    "lodash.clone": "^4.5.0",
+    "lodash.debounce": "^4.0.8",
     "lodash.isequal": "^4.5.0",
-    "lodash.throttle": "^4.1.1",
     "prop-types": "^15.6.0",
     "react": "^16.2.0",
     "react-json-tree": "^0.11.0",

src/generateReduxReport.js

@@ -2,18 +2,16 @@ import { diff } from "deep-object-diff"
 import StackTrace from "stacktrace-js"
 import { isObjectOrArray } from "./utility"
 import { createMakeProxyFunction } from "./trackObjectUse"
-import cloneDeep from "lodash.clone"
-import throttle from "lodash.throttle"
-import "./lib/browser-source-map-support"
+import debounce from "lodash.debounce"
 
 // we need source maps for the stack traces
 // or else we won't know whether to ignore object access
 // from non-local code (e.g node_modules, browser extensions...)
 // this takes the stack trace file name from e.g.  fileName: "http://localhost:3001/static/js/bundle.js",
 // to "http://localhost:3000/Users/alexholachek/Desktop/work/redux-usage-report/todomvc-example/src/containers/App.js
 // this raises an error during jest tests so limit to development
-//
 if (process.env.NODE_ENV === "development") {
+  require("./lib/browser-source-map-support")
   sourceMapSupport.install() // eslint-disable-line
 }
 
@@ -34,7 +32,9 @@ function replaceUndefinedWithNull(obj) {
 
 let globalObjectCache
 
-const shouldSkipProxy = (target, propKey) => {
+const shouldSkipProxy = () => {
+  if (global.reduxReport.__inProgress || global.reduxReport.__reducerInProgress) return true
+
   // this is kind of hacky, but webpack dev server serves non-local files
   // that look like this: `webpack:///./~/react-redux/lib/components/connect.js `
   // whereas local files look like this: webpack:///./containers/TodoApp.js
@@ -46,17 +46,12 @@ const shouldSkipProxy = (target, propKey) => {
 
   const initiatingFuncNotLocal =
     !!initiatingFunc &&
+    initiatingFunc.fileName &&
     (initiatingFunc.fileName.match(/\.\/~\/|\/node_modules\//) ||
       initiatingFunc.fileName.match(/extension:\/\//))
 
-  if (
-    !!initiatingFuncNotLocal ||
-    !target.hasOwnProperty(propKey) ||
-    global.reduxReport.__inProgress ||
-    global.reduxReport.__reducerInProgress
-  ) {
-    return true
-  }
+  if (!!initiatingFuncNotLocal) return true
+
   return false
 }
 
@@ -66,7 +61,7 @@ function generateReduxReport(global, rootReducer) {
     accessedState: {},
     state: {},
     setOnChangeCallback(cb) {
-      global.reduxReport.onChangeCallback = throttle(cb, 1000)
+      global.reduxReport.onChangeCallback = debounce(cb, 25)
     },
     removeOnChangeCallback() {
       global.reduxReport.onChangeCallback = undefined
@@ -81,8 +76,8 @@ function generateReduxReport(global, rootReducer) {
     },
     generate() {
       global.reduxReport.__inProgress = true
-      const used = cloneDeep(this.accessedState)
-      const stateCopy = cloneDeep(this.state)
+      const used = JSON.parse(JSON.stringify(this.accessedState))
+      const stateCopy = JSON.parse(JSON.stringify(this.state))
       const unused = diff(stateCopy, used)
       replaceUndefinedWithNull(unused)
       const report = {
@@ -99,7 +94,8 @@ function generateReduxReport(global, rootReducer) {
     shouldSkipProxy,
     accessedProperties: global.reduxReport.accessedState,
     getBreakpoint: () => global.localStorage && global.localStorage.getItem(localStorageKey),
-    onChange: (...args) => global.reduxReport.onChangeCallback && global.reduxReport.onChangeCallback(...args)
+    onChange: stateLocation =>
+      global.reduxReport.onChangeCallback && global.reduxReport.onChangeCallback(stateLocation)
   })
 
   return (prevState, action) => {

src/monitor/index.js

@@ -85,7 +85,8 @@ class ReduxUsageMonitor extends Component {
 
   componentDidMount() {
     this.updateReport()
-    window.reduxReport.setOnChangeCallback(this.updateReport)
+    // not sure why this bind is necessary
+    window.reduxReport.setOnChangeCallback(this.updateReport.bind(this))
   }
 
   componentWillUnmount() {
@@ -94,22 +95,12 @@ class ReduxUsageMonitor extends Component {
 
   updateReport = () => {
     const report = window.reduxReport.generate()
-    if (!isEqual(this.state.used, report.used)) {
-      this.setState(() => ({
-        used: report.used,
-        stateCopy: report.stateCopy
-      }))
-    }
-  }
-
-  componentDidUpdate = (prevProps, prevState) => {
-    if (prevProps.computedStates.length !== this.props.computedStates.length) {
-      const report = window.reduxReport.generate()
-      this.setState(() => ({
-        used: report.used,
-        stateCopy: report.stateCopy
-      }))
-    }
+    if (isEqual(report.used, this.state.used) && isEqual(report.stateCopy, this.state.stateCopy))
+      return
+    this.setState({
+      used: report.used,
+      stateCopy: report.stateCopy
+    })
   }
 
   setBreakpoint = breakpointPath => {

src/trackObjectUse.js

@@ -1,17 +1,27 @@
 import { isObjectOrArray, isUndefined } from "./utility"
 
+const UNPROXIED_OBJ_KEY = "__initialValue"
+
+const getUnproxiedObject = target =>
+  target[UNPROXIED_OBJ_KEY] !== undefined ? target[UNPROXIED_OBJ_KEY] : target
+
 export const createMakeProxyFunction = ({
   keepOriginalValues = false,
-  shouldSkipProxy,
   accessedProperties,
+  shouldSkipProxy = () => false,
   getBreakpoint = () => {},
   onChange = () => {}
 }) => {
   return function makeProxy(obj, stateLocation = "") {
     const handler = {
       get(target, propKey) {
+        // need to be able to actually get the value without triggering another get cycle
+        if (propKey === UNPROXIED_OBJ_KEY) return target
         const value = target[propKey]
-        if (!isUndefined(shouldSkipProxy) && shouldSkipProxy(target, propKey)) return value
+
+        if (!Object.hasOwnProperty.call(target, propKey)) return value
+
+        if (shouldSkipProxy()) return JSON.parse(JSON.stringify(value))
 
         const accessedPropertiesPointer = !stateLocation
           ? accessedProperties
@@ -20,27 +30,33 @@ export const createMakeProxyFunction = ({
             }, accessedProperties)
 
         const newStateLocation = stateLocation ? stateLocation + "." + propKey : propKey
-        onChange(newStateLocation)
 
         // allow people to examine the stack at certain access points
         if (getBreakpoint() === newStateLocation) {
           // explore the callstack to see when your app accesses a value
           debugger
         }
         if (isObjectOrArray(value)) {
-          if (!accessedPropertiesPointer[propKey]) {
+          if (isUndefined(accessedPropertiesPointer[propKey])) {
             accessedPropertiesPointer[propKey] = Array.isArray(value) ? [] : {}
+            onChange(newStateLocation)
           }
           return makeProxy(value, newStateLocation)
         } else {
-          if (isUndefined(accessedPropertiesPointer[propKey]) || !keepOriginalValues) {
+          if (
+            isUndefined(accessedPropertiesPointer[propKey]) ||
+            (!keepOriginalValues && value !== accessedPropertiesPointer[propKey])
+          ) {
             accessedPropertiesPointer[propKey] = value
+            onChange(newStateLocation)
           }
           return value
         }
       }
     }
-    return new Proxy(obj, handler)
+    // prevent double-wrapping proxies
+    const unproxiedObj = getUnproxiedObject(obj)
+    return new Proxy(unproxiedObj, handler)
   }
 }
 

todomvc-example/config/webpack.config.dev.js

@@ -28,7 +28,7 @@ const env = getClientEnvironment(publicUrl);
 module.exports = {
   // You may want 'eval' instead if you prefer to see the compiled output in DevTools.
   // See the discussion in https://github.com/facebookincubator/create-react-app/issues/343.
-  devtool: 'cheap-module-source-map',
+  devtool: 'source-map',
   // These are the "entry points" to our application.
   // This means they will be the "root" imports that are included in JS bundle.
   // The first two entry points enable "hot" CSS and auto-refreshes for JS.

todomvc-example/src/components/TodoItem.js

@@ -30,7 +30,6 @@ export default class TodoItem extends Component {
 
   render() {
     const { todo, completeTodo, deleteTodo } = this.props
-
     let element
     if (this.state.editing) {
       element = (
@@ -48,7 +47,7 @@ export default class TodoItem extends Component {
             type="checkbox"
             checked={todo.completed}
             onChange={() => {
-              console.log(this.props.todo.someExtraData.thisObj2)
+              console.log(`todo.someExtraData.thisObj2: ${todo.someExtraData.thisObj2}`)
               completeTodo(todo.id)
             }}
           />

todomvc-example/src/index.js

@@ -10,7 +10,6 @@ import "todomvc-app-css/index.css"
 import generateReduxReport from "./redux-usage-report"
 import DevTools from "./containers/DevTools"
 
-debugger
 const enhancer = compose(
   DevTools.instrument(),
   generateReduxReport(),

todomvc-example/src/reducers/todos.js

@@ -14,17 +14,15 @@ const initialState = [
     id: 0,
     someExtraData: {
       thisObj: "isUnused",
-      thisObj2: 'isAccessedWhenCompletedIsToggled'
-
+      thisObj2: "isAccessedWhenCompletedIsToggled"
     }
   },
   {
     text: "Did gyre and gimbel in the wabe",
     completed: false,
     someExtraData: {
       thisObj: "isUnused",
-      thisObj2: 'isAccessedWhenCompletedIsToggled'
-
+      thisObj2: "isAccessedWhenCompletedIsToggled"
     },
     id: 1
   },
@@ -33,7 +31,7 @@ const initialState = [
     completed: false,
     someExtraData: {
       thisObj: "isUnused",
-      thisObj2: 'isAccessedWhenCompletedIsToggled'
+      thisObj2: "isAccessedWhenCompletedIsToggled"
     },
     id: 3
   }
@@ -52,9 +50,10 @@ export default function todos(state = initialState, action) {
             a: Math.random().toFixed(3),
             b: Math.random().toFixed(4)
           },
-          thisValueIsntAccessed: "foo",
-          thisObj2: 'isAccessedWhenCompletedIsToggled'
-
+          someExtraData: {
+            thisObj: "isUnused",
+            thisObj2: "isAccessedWhenCompletedIsToggled"
+          }
         }
       ]