From 1ad2be35b5984e4bcf2ba49943dc8c5f46f0e237 Mon Sep 17 00:00:00 2001 From: Alf Eaton Date: Tue, 26 Mar 2024 09:05:06 +0000 Subject: [PATCH] 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 | 81 +++++++++++-------- .../outline/components/outline-pane.spec.tsx | 11 +-- .../components/codemirror-editor.spec.tsx | 12 ++- 3 files changed, 57 insertions(+), 47 deletions(-) diff --git a/services/web/.eslintrc b/services/web/.eslintrc index 17ce9968ca..eb410d0ce3 100644 --- a/services/web/.eslintrc +++ b/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" + } ] } }, diff --git a/services/web/test/frontend/features/outline/components/outline-pane.spec.tsx b/services/web/test/frontend/features/outline/components/outline-pane.spec.tsx index 08dd2fcf13..06f536518e 100644 --- a/services/web/test/frontend/features/outline/components/outline-pane.spec.tsx +++ b/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('', function () { it('renders expanded outline', function () { @@ -38,7 +39,7 @@ describe('', 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('', 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('', 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) }) }) diff --git a/services/web/test/frontend/features/source-editor/components/codemirror-editor.spec.tsx b/services/web/test/frontend/features/source-editor/components/codemirror-editor.spec.tsx index 9e60444e77..c12b59285c 100644 --- a/services/web/test/frontend/features/source-editor/components/codemirror-editor.spec.tsx +++ b/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('', { scrollBehavior: false }, function () { beforeEach(function () { @@ -555,13 +556,10 @@ describe('', { 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(