Fix: #16 False positive for "Better to just use Proxy state" (#17)

* add conditional check for reading of snapshot

* additional tests for allowing read on snapshots

.size-snapshot.json

@@ -1,7 +1,7 @@
 {
   "index.js": {
-    "bundled": 12750,
-    "minified": 7384,
-    "gzipped": 2144
+    "bundled": 13543,
+    "minified": 7698,
+    "gzipped": 2234
   }
 }

src/StateSnapshot.js

@@ -1,7 +1,10 @@
-import { isInSomething } from './lib/utils'
+import {
+  callExpressions,
+  functionTypes,
+  isInSomething,
+  isReadOnly,
+} from './lib/utils'
 
-const functionTypes = ['ArrowFunctionExpression', 'FunctionExpression']
-const callExpressions = ['JSXExpressionContainer', 'CallExpression']
 export const PROXY_RENDER_PHASE_MESSAGE =
   'Using proxies in the render phase would cause unexpected problems.'
 export const SNAPSHOT_CALLBACK_MESSAGE = 'Better to just use proxy state.'
@@ -84,11 +87,20 @@ export default {
             message: PROXY_RENDER_PHASE_MESSAGE,
           })
         }
-        if (kind === 'snapshot' && isInCallback(node)) {
-          return context.report({
-            node,
-            message: SNAPSHOT_CALLBACK_MESSAGE,
-          })
+
+        if (kind === 'snapshot') {
+          // ignore the error if the snapshot
+          // is just being read
+          if (isReadOnly(node)) {
+            return
+          }
+
+          if (isInCallback(node)) {
+            return context.report({
+              node,
+              message: SNAPSHOT_CALLBACK_MESSAGE,
+            })
+          }
         }
       },
     }

src/lib/utils.js

@@ -1,3 +1,7 @@
+export const callExpressions = ['JSXExpressionContainer', 'CallExpression']
+export const functionTypes = ['ArrowFunctionExpression', 'FunctionExpression']
+export const writingOpExpressionTypes = ['UpdateExpression']
+
 /**
  * @param {any} node ASTNode to check
  * @param {string} thing Check if the node is inside a particular type of ASTNode
@@ -87,3 +91,56 @@ export function getParentOfNodeType(node, nodeType) {
   }
   return null
 }
+
+/**
+ * @description check if the node is only being read
+ * and not being used to manipulate state, recursively checks in the node is
+ * in a member expression as well
+ * @param {*} node
+ */
+export function isReadOnly(node) {
+  if (writingOpExpressionTypes.indexOf(node.parent.type) > -1) {
+    return false
+  }
+
+  if (node.parent.type === 'AssignmentExpression' && isLeftOfAssignment(node)) {
+    return false
+  }
+
+  if (node.parent.type === 'MemberExpression') {
+    return isReadOnly(node.parent)
+  }
+
+  return true
+}
+
+/**
+ *
+ * @param {*} node
+ * @returns {boolean} true if the node is on the left
+ * of an assignment expression aka being modified
+ */
+export function isLeftOfAssignment(node) {
+  if (Object.is(node.parent.left, node)) {
+    return true
+  }
+
+  return false
+}
+
+/**
+ * @description get the closes call expression or function defintion
+ * @param {*} node
+ */
+export function returnFirstCallback(node) {
+  if (!node.parent || !node.parent.type) return false
+
+  if (
+    callExpressions.includes(node.parent.type) &&
+    functionTypes.includes(node.type)
+  ) {
+    return node
+  } else {
+    return returnFirstCallback(node.parent)
+  }
+}

tests/StateSnapshot.test.js

@@ -51,6 +51,25 @@ ruleTester.run('state-snapshot-rule', rule, {
     return <div>{snap.foo}</div>
   }
   `,
+    `
+function useExample2(s) {
+  const {b: {c} } = useSnapshot(s.a1);
+
+  useEffect(() => {
+    if (c === 'a1c') {
+      state.a1.b.c = 'example';
+    }
+  }, [c]);
+}`,
+
+    `
+function useProxyStateExample(a) {
+  const s = useSnapshot(a);
+
+  useEffect(() => {
+      a.a = 'example';
+  }, [s.a]);
+}`,
   ],
   invalid: [
     {
@@ -178,5 +197,30 @@ ruleTester.run('state-snapshot-rule', rule, {
   `,
       errors: [COMPUTED_DECLARATION_ORDER],
     },
+    {
+      code: `function Counter() {
+  const snapshot = useSnapshot(state)
+  useEffect(() => {
+    snapshot.count = randomValue + 1;
+  })
+  return (
+    <></>
+  )
+}`,
+      errors: [SNAPSHOT_CALLBACK_MESSAGE],
+    },
+    {
+      code: `
+      function useProxyStateExample(a) {
+        const s = useSnapshot(a);
+      
+        useEffect(() => {
+          if(s.a==="1"){
+            s.a = 'example';
+          }
+        }, [s.a]);
+      }`,
+      errors: [SNAPSHOT_CALLBACK_MESSAGE],
+    },
   ],
 })