Merge pull request #17626 from overleaf/ae-eslint-window-localstorage

Add ESLint rule that prevents direct use of window.localStorage

GitOrigin-RevId: 30abcd96ef31690a482c5368ff2a6d11024f5126

services/web/.eslintrc

@@ -131,40 +131,6 @@
         "plugin:cypress/recommended"
       ]
     },
-    {
-      // React component specific rules
-      //
-      "files": ["**/frontend/js/**/components/**/*.{js,jsx,ts,tsx}", "**/frontend/js/**/hooks/**/*.{js,jsx,ts,tsx}"],
-      "rules": {
-        "@overleaf/no-unnecessary-trans": "error",
-        "@overleaf/should-unescape-trans": "error",
-
-        // https://astexplorer.net/
-        "no-restricted-syntax": [
-          "error",
-          // prohibit direct calls to methods of window.location
-          {
-            "selector": "CallExpression[callee.object.object.name='window'][callee.object.property.name='location']",
-            "message": "Modify location via useLocation instead of calling window.location methods directly"
-          },
-          // prohibit assignment to window.location
-          {
-            "selector": "AssignmentExpression[left.object.name='window'][left.property.name='location']",
-            "message": "Modify location via useLocation instead of calling window.location methods directly"
-          },
-          // prohibit assignment to window.location.href
-          {
-            "selector": "AssignmentExpression[left.object.object.name='window'][left.object.property.name='location'][left.property.name='href']",
-            "message": "Modify location via useLocation instead of calling window.location methods directly"
-          },
-          // prohibit using lookbehinds due to incidents with Safari simply crashing when the script is parsed
-          {
-            "selector": "Literal[regex.pattern=/\\(\\?<[!=]/]",
-            "message": "Lookbehind is not supported in older Safari versions."
-          }
-        ]
-      }
-    },
     {
       // Frontend specific rules
       "files": ["**/frontend/js/**/*.{js,jsx,ts,tsx}", "**/frontend/stories/**/*.{js,jsx,ts,tsx}", "**/*.stories.{js,jsx,ts,tsx}", "**/test/frontend/**/*.{js,jsx,ts,tsx}", "**/test/frontend/components/**/*.spec.{js,jsx,ts,tsx}"],
@@ -274,8 +240,53 @@
           {
             "selector": "CallExpression[callee.object.name='App'][callee.property.name=/run|directive|config|controller/] > ArrayExpression[elements.length=0]",
             "message": "Array must not be empty. Add parameters and a function. E.g ['param1', function(param1) {}]"
-          }
+          },
           // End: Make sure angular can withstand minification
+          // prohibit direct calls to methods of window.localStorage
+          {
+            "selector": "CallExpression[callee.object.object.name='window'][callee.object.property.name='localStorage']",
+            "message": "Modify location via customLocalStorage instead of calling window.localStorage methods directly"
+          }
+        ]
+      }
+    },
+    {
+      // React component specific rules
+      //
+      "files": ["**/frontend/js/**/components/**/*.{js,jsx,ts,tsx}", "**/frontend/js/**/hooks/**/*.{js,jsx,ts,tsx}"],
+      "rules": {
+        "@overleaf/no-unnecessary-trans": "error",
+        "@overleaf/should-unescape-trans": "error",
+
+        // https://astexplorer.net/
+        "no-restricted-syntax": [
+          "error",
+          // prohibit direct calls to methods of window.location
+          {
+            "selector": "CallExpression[callee.object.object.name='window'][callee.object.property.name='location']",
+            "message": "Modify location via useLocation instead of calling window.location methods directly"
+          },
+          // prohibit assignment to window.location
+          {
+            "selector": "AssignmentExpression[left.object.name='window'][left.property.name='location']",
+            "message": "Modify location via useLocation instead of calling window.location methods directly"
+          },
+          // prohibit assignment to window.location.href
+          {
+            "selector": "AssignmentExpression[left.object.object.name='window'][left.object.property.name='location'][left.property.name='href']",
+            "message": "Modify location via useLocation instead of calling window.location methods directly"
+          },
+          // prohibit using lookbehinds due to incidents with Safari simply crashing when the script is parsed
+          {
+            "selector": "Literal[regex.pattern=/\\(\\?<[!=]/]",
+            "message": "Lookbehind is not supported in older Safari versions."
+          },
+          // prohibit direct calls to methods of window.localStorage
+          // NOTE: this rule is also defined for all frontend files, but those rules are overriden by the React component-specific config
+          {
+            "selector": "CallExpression[callee.object.object.name='window'][callee.object.property.name='localStorage']",
+            "message": "Modify location via customLocalStorage instead of calling window.localStorage methods directly"
+          }
         ]
       }
     },

services/web/test/frontend/features/outline/components/outline-pane.spec.tsx

@@ -1,6 +1,7 @@
 import OutlinePane from '@/features/outline/components/outline-pane'
 import { EditorProviders, PROJECT_ID } from '../../../helpers/editor-providers'
 import { useState } from 'react'
+import customLocalStorage from '@/infrastructure/local-storage'
 
 describe('<OutlinePane />', function () {
   it('renders expanded outline', function () {
@@ -38,7 +39,7 @@ describe('<OutlinePane />', function () {
   })
 
   it('expand outline and use local storage', function () {
-    window.localStorage.setItem(`file_outline.expanded.${PROJECT_ID}`, 'false')
+    customLocalStorage.setItem(`file_outline.expanded.${PROJECT_ID}`, false)
 
     const onToggle = cy.stub()
 
@@ -53,9 +54,9 @@ describe('<OutlinePane />', function () {
           onToggle={onToggle}
           expanded={expanded}
           toggleExpanded={() => {
-            window.localStorage.setItem(
+            customLocalStorage.setItem(
               `file_outline.expanded.${PROJECT_ID}`,
-              expanded ? 'false' : 'true'
+              !expanded
             )
             setExpanded(!expanded)
           }}
@@ -78,8 +79,8 @@ describe('<OutlinePane />', function () {
     cy.findByRole('tree').then(() => {
       expect(onToggle).to.be.calledTwice
       expect(
-        window.localStorage.getItem(`file_outline.expanded.${PROJECT_ID}`)
-      ).to.equal('true')
+        customLocalStorage.getItem(`file_outline.expanded.${PROJECT_ID}`)
+      ).to.equal(true)
     })
   })
 

services/web/test/frontend/features/source-editor/components/codemirror-editor.spec.tsx

@@ -5,6 +5,7 @@ import { metaKey } from '../helpers/meta-key'
 import { docId } from '../helpers/mock-doc'
 import { activeEditorLine } from '../helpers/active-editor-line'
 import { TestContainer } from '../helpers/test-container'
+import customLocalStorage from '@/infrastructure/local-storage'
 
 describe('<CodeMirrorEditor/>', { scrollBehavior: false }, function () {
   beforeEach(function () {
@@ -555,13 +556,10 @@ describe('<CodeMirrorEditor/>', { scrollBehavior: false }, function () {
   it('restores stored cursor and scroll position', function () {
     const scope = mockScope()
 
-    window.localStorage.setItem(
-      `doc.position.${docId}`,
-      JSON.stringify({
-        cursorPosition: { row: 50, column: 5 },
-        firstVisibleLine: 45,
-      })
-    )
+    customLocalStorage.setItem(`doc.position.${docId}`, {
+      cursorPosition: { row: 50, column: 5 },
+      firstVisibleLine: 45,
+    })
 
     cy.mount(
       <TestContainer>