Upgrade typescript to ^4.2.0 and fix lint issues (#1156)

* Upgrade typescript to ^4.2.0 and fix lint issues

* Upgrade react-scripts to ^4.0.3

* Add craco and set url: false in css-loader

* Fix lint issues

* Fix part of unit tests

* Fix another part of unit tests

* Handle ServiceWorker in a new way

* Fix ts errors

* Fix new lint errors

* revert this lint fix due to compilation errors - it should be fixed after updating @opesntax/types

* Move also service-worker.js.map to books folder

* Fix one more lint error and test

* change AnyRoute signature and add assertNonNullableArray util

* small fix for test

* Do not use any for animationEvent function

* update loadFont helper

* Update serviceWorker.ts

* remove unused workbox plugins

* Revert "remove unused workbox plugins"

This reverts commit d85a8fd.

* Use craco to move service-worker.js to build/books/

* Remove check for process.env.PUBLIC_URL

* Default value for process.env.PUBLIC_URL

* remove unused workbox plugins

* Check if PUBLIC_URL is undefined instead of falsy

* requested changes

* remove assertNonNullable

* Update @openstax/types

* Do not use deprecated matchMedia.add/removeListener methods

* add global KeyboardEventConstructor

* fix lint and tests

* smaller fixes

* No need to mock navigation state in Home.spec.tsx

* Add @babel/plugin-proposal-optional-chaining to babel-config.js

* Update node_modules cache

* requested changes

Co-authored-by: staxly[bot] <35789409+staxly[bot]@users.noreply.github.com>
Co-authored-by: jarosik10 <bartosz.jaros96@gmail.com>

.github/workflows/ci.yml

@@ -21,7 +21,7 @@ jobs:
       id: node_modules
       with:
         path: node_modules
-        key: node_modules5
+        key: node_modules6
     - run: yarn install
     - name: Find Deployment
       id: find-deployment
@@ -62,7 +62,7 @@ jobs:
       id: node_modules
       with:
         path: node_modules
-        key: node_modules5
+        key: node_modules6
     - run: yarn install
     - run: ./script/validate-modified-books.bash
       env:
@@ -86,7 +86,7 @@ jobs:
       id: node_modules
       with:
         path: node_modules
-        key: node_modules5
+        key: node_modules6
     - run: yarn install
     - run: yarn ci:test:${{ matrix.suite }}
     - uses: actions/upload-artifact@v2
@@ -107,6 +107,6 @@ jobs:
       id: node_modules
       with:
         path: node_modules
-        key: node_modules5
+        key: node_modules6
     - run: yarn install
     - run: yarn lint

craco.config.js

@@ -0,0 +1,39 @@
+const path = require('path');
+
+module.exports = {
+    plugins: [{
+        plugin: {
+            // Based on https://github.com/kevinsperrine/craco-workbox/blob/master/lib/index.js
+            overrideWebpackConfig: ({ webpackConfig, context: { env, paths } }) => {
+                if (env === "production") {
+                  try {
+                    const workboxConfig = require(path.join(
+                      paths.appPath,
+                      "workbox.config.js"
+                    ));
+
+                    webpackConfig.plugins.forEach(plugin => {
+                      if (plugin.constructor.name === "InjectManifest") {
+                        plugin.config = workboxConfig(plugin.config);
+                      }
+                    });
+                  } catch (error) {
+                    console.log("[craco.config.js - overrideWebpackConfig]");
+                    console.log(error.stack);
+                    process.exit(1);
+                  }
+                }
+
+                return webpackConfig;
+              }
+        },
+    }],
+    style: {
+        css: {
+            loaderOptions: {
+                // https://github.com/openstax/unified/issues/1469
+                url: false,
+            },
+        },
+    },
+};

package.json

@@ -3,6 +3,7 @@
   "version": "0.1.0",
   "private": true,
   "dependencies": {
+    "@craco/craco": "^6.1.2",
     "@formatjs/intl-pluralrules": "^4.0.6",
     "@openstax/highlighter": "^1.9.0",
     "@openstax/open-search-client": "0.1.0",
@@ -27,7 +28,7 @@
     "react-intl": "^5.12.0",
     "react-loadable": "^5.5.0",
     "react-redux": "^7.1.0",
-    "react-scripts": "3.4.4",
+    "react-scripts": "^4.0.3",
     "redux": "^4.0.1",
     "redux-sentry-middleware": "^0.2.2",
     "reselect": "^4.0.0",
@@ -36,10 +37,15 @@
     "styled-components": "^4.3.2",
     "styled-icons": "^8.1.0",
     "typesafe-actions": "^4.4.2",
-    "typescript": "3.9.6",
+    "typescript": "^4.2.0",
     "url": "^0.11.0",
     "uuid": "^3.4.0",
-    "weak-map": "^1.0.5"
+    "weak-map": "^1.0.5",
+    "workbox-core": "^5.1.3",
+    "workbox-expiration": "^5.1.3",
+    "workbox-precaching": "^5.1.3",
+    "workbox-routing": "^5.1.3",
+    "workbox-strategies": "^5.1.3"
   },
   "scripts": {
     "trust-localhost": "./script/trust-localhost.bash",
@@ -49,12 +55,12 @@
     "lint:css": "stylelint 'src/**/*.tsx'",
     "lint:bash": "shellcheck $(find . -type f \\( -iname '*\\.sh' -or -iname '*\\.bash' \\) | grep -v 'node_modules' )",
     "prestart": "npm run-script build:css",
-    "start": "HTTPS=${HTTPS:-true} react-scripts start",
+    "start": "HTTPS=${HTTPS:-true} craco start",
     "start:static": "export REACT_APP_ENV=${REACT_APP_ENV:-test} && npm run-script build && npm run-script prerender && npm run-script server",
     "clean": "rm -rf ./build",
-    "build": "npm run-script build:css && npm run-script build:js && ./script/doctor-workbox.bash",
+    "build": "npm run-script build:css && npm run-script build:js",
     "build:css": "lessc --source-map --source-map-include-source ./generic-styles/index.less ./src/content.css",
-    "build:js": "export REACT_APP_ENV=${REACT_APP_ENV:-production} && GENERATE_SOURCEMAP=${GENERATE_SOURCEMAP:-true} react-scripts build",
+    "build:js": "export REACT_APP_ENV=${REACT_APP_ENV:-production} && GENERATE_SOURCEMAP=${GENERATE_SOURCEMAP:-true} craco build",
     "build:clean": "npm run-script clean && npm run-script build",
     "prerender": "REACT_APP_ENV=${REACT_APP_ENV:-production} node ./script/entry prerender/index",
     "server": "REACT_APP_ENV=${REACT_APP_ENV:-development} node ./script/entry server/cli",
@@ -76,7 +82,7 @@
     "test:prerender:browser": "REACT_APP_ENV=test SERVER_MODE=built jest --testPathPattern=\"(\\.|/)browserspec\\.tsx?\" --config jest-puppeteer.config.json -i",
     "test:prerender:screenshots": "REACT_APP_ENV=test SERVER_MODE=built jest --testPathPattern=\"(\\.|/)screenshotspec\\.tsx?\" --config jest-puppeteer.config.json",
     "test:lighthouse-manual": "REACT_APP_ENV=test lighthouse --view --config-path=./src/test/audits/index.js",
-    "analyze:bundle": "react-scripts build && source-map-explorer 'build/static/js/*.js' -m",
+    "analyze:bundle": "craco build && source-map-explorer 'build/static/js/*.js' -m",
     "analyze:dom": "node ./script/entry.js domVisitor",
     "heroku-postbuild": "npm run-script build:clean"
   },
@@ -93,12 +99,13 @@
     "@babel/core": "^7.0.0-0",
     "@babel/plugin-proposal-class-properties": "^7.1.0",
     "@babel/plugin-proposal-object-rest-spread": "^7.0.0",
+    "@babel/plugin-proposal-optional-chaining": "^7.13.12",
     "@babel/plugin-transform-runtime": "^7.1.0",
     "@babel/preset-env": "^7.1.6",
     "@babel/preset-react": "^7.0.0",
     "@babel/preset-typescript": "^7.1.0",
     "@babel/register": "^7.0.0",
-    "@openstax/types": "^1.0.3",
+    "@openstax/types": "^2.0.0",
     "@types/color": "^3.0.0",
     "@types/express": "^4.16.0",
     "@types/flat": "^0.0.28",

src/app/components/Dropdown.tsx

@@ -75,7 +75,7 @@ const TabHiddenDropDown = styled((
   const container = React.useRef<HTMLElement>(null);
   const toggleElement = React.useRef<HTMLElement>(null);
 
-  useFocusLost(container, isOpen, () => setOpen(false));
+  useFocusLost(container, isOpen, React.useCallback(() => setOpen(false), [setOpen]));
   useOnEsc(container, isOpen, () => {
     setOpen(false);
     if (toggleElement.current) { toggleElement.current.focus(); }

src/app/content/components/BookBanner.spec.tsx

@@ -24,7 +24,7 @@ describe('BookBanner', () => {
     resetModules();
 
     window = assertWindow();
-    delete window.location;
+    delete (window as any).location;
 
     window.location = {
       assign: jest.fn(),

src/app/content/components/NudgeStudyTools/index.tsx

@@ -48,7 +48,7 @@ const NudgeStudyTools = () => {
     if (positions) {
       document.body.style.overflow = 'hidden';
     }
-    return () => { document.body.style.overflow = null; };
+    return () => { document.body.style.overflow = ''; };
     // document will not change
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [positions]);

src/app/content/components/NudgeStudyTools/utils.spec.tsx

@@ -1,4 +1,4 @@
-import { ClientRect, HTMLElement } from '@openstax/types/lib.dom';
+import { DOMRect, HTMLElement } from '@openstax/types/lib.dom';
 import * as Cookies from 'js-cookie';
 import React from 'react';
 import { Provider } from 'react-redux';
@@ -17,7 +17,7 @@ describe('usePositions', () => {
   let store: Store;
   // tslint:disable-next-line: variable-name
   let Component: (props: { isMobile: boolean }) => JSX.Element;
-  const mockRect = { bottom: 228, height: 25, left: 951, right: 1233, top: 203, width: 282 } as any as ClientRect;
+  const mockRect = { bottom: 228, height: 25, left: 951, right: 1233, top: 203, width: 282 } as any as DOMRect;
 
   beforeEach(() => {
     const document = assertDocument();

src/app/content/components/Page.spec.tsx

@@ -701,7 +701,7 @@ describe('Page', () => {
       event.initMouseEvent('click',
         event.cancelBubble,
         event.cancelable,
-        event.view,
+        assertWindow(),
         event.detail,
         event.screenX,
         event.screenY,

src/app/content/components/Page/contentLinkHandler.ts

@@ -88,7 +88,7 @@ export const contentLinkHandler = (anchor: HTMLAnchorElement, getProps: () => Co
       return;
     }
 
-    const base = new URL(assertWindow().location);
+    const base = new URL(assertWindow().location.href);
     base.hash = '';
     base.search = '';
 

src/app/content/components/Page/highlightManager.spec.tsx

@@ -70,7 +70,7 @@ describe('highlightManager', () => {
   });
 
   afterEach(() => {
-    delete window.document.getSelection;
+    delete (window as any).document.getSelection;
   });
 
   it('CardList is rendered initially', () => {

src/app/content/components/Page/highlightManager.ts

@@ -89,7 +89,7 @@ const onSelectHighlight = (
   }
 
   if (services.getProp().hasUnsavedHighlight && !await showConfirmation()) {
-    assertWindow().getSelection().removeAllRanges();
+    assertWindow().getSelection()?.removeAllRanges();
     return;
   }
 

src/app/content/components/utils/allImagesLoaded.spec.ts

@@ -14,6 +14,8 @@ describe('allImagesLoaded', () => {
     finished = false;
     element = assertDocument().createElement('div');
     img = assertDocument().createElement('img');
+
+    Object.defineProperty(img, 'complete', { value: false, writable: true });
   });
 
   it('resolves when passed an element with no images', async() => {

src/app/content/content.prerenderspec.ts

@@ -35,6 +35,8 @@ describe('content', () => {
         ['[data-testid="toc"]', 'style'],
         ['[data-testid="search-results-sidebar"]', 'style'],
         ['[data-testid="loader"] path', 'style'],
+        // img src is changed from data:image/svg+xml;base64... to static path
+        ['[data-testid="navbar"] img', 'src'],
       ].forEach(([selector, attribute]) => {
         root.querySelectorAll(selector).forEach((element) =>
           element.removeAttribute(attribute)

src/app/content/highlights/components/CardWrapper.spec.tsx

@@ -21,8 +21,7 @@ const dispatchKeyDownEvent = (
   key: string,
   target?: HTMLElement
 ) => {
-  const keyboardEvent = window.document.createEvent('KeyboardEvent');
-  keyboardEvent.initKeyboardEvent('keydown', true, true, window, key, 0, '', false, '');
+  const keyboardEvent = new KeyboardEvent('keydown', { bubbles: true, cancelable: true, key, view: window });
   if (target) {
     Object.defineProperty(keyboardEvent, 'target', { value: target });
   }
@@ -178,23 +177,23 @@ describe('CardWrapper', () => {
 
     // Update state with a height
     renderer.act(() => {
-      card1.props.onHeightChange({ current: { offsetHeight: 100 }});
-      card2.props.onHeightChange({ current: { offsetHeight: 100 }});
+      card1.props.onHeightChange({ current: { offsetHeight: 100 } });
+      card2.props.onHeightChange({ current: { offsetHeight: 100 } });
     });
     // We are starting at 100 because of getHighlightTopOffset mock
     expect(card1.props.topOffset).toEqual(100);
     expect(card2.props.topOffset).toEqual(100 + 100 + remsToPx(cardMarginBottom));
 
     // Noops when height is the same
     renderer.act(() => {
-      card1.props.onHeightChange({ current: { offsetHeight: 100 }});
+      card1.props.onHeightChange({ current: { offsetHeight: 100 } });
     });
     expect(card1.props.topOffset).toEqual(100);
     expect(card2.props.topOffset).toEqual(100 + 100 + remsToPx(cardMarginBottom));
 
     // Handle null value
     renderer.act(() => {
-      card1.props.onHeightChange({ current: { offsetHeight: null }});
+      card1.props.onHeightChange({ current: { offsetHeight: null } });
     });
     // First card have null height so secondcard starts at the highlight top offset
     expect(card1.props.topOffset).toEqual(100);
@@ -228,10 +227,10 @@ describe('CardWrapper', () => {
 
     // Update state with a height
     renderer.act(() => {
-      card1.props.onHeightChange({ current: { offsetHeight: 50 }});
-      card2.props.onHeightChange({ current: { offsetHeight: 50 }});
-      card3.props.onHeightChange({ current: { offsetHeight: 50 }});
-      card4.props.onHeightChange({ current: { offsetHeight: 50 }});
+      card1.props.onHeightChange({ current: { offsetHeight: 50 } });
+      card2.props.onHeightChange({ current: { offsetHeight: 50 } });
+      card3.props.onHeightChange({ current: { offsetHeight: 50 } });
+      card4.props.onHeightChange({ current: { offsetHeight: 50 } });
     });
     expect(card1.props.topOffset).toEqual(0); // first card have only 50px height + 20px margin bottom
     expect(card2.props.topOffset).toEqual(100); // so the second cards starts and the highlight level which is 100

src/app/content/highlights/components/CardWrapper.tsx

@@ -58,7 +58,7 @@ const Wrapper = ({highlights, className, container, highlighter}: WrapperProps)
 
   // Clear shouldFocusCard when focus is lost from the CardWrapper.
   // If we don't do this then card related for the focused highlight will be focused automatically.
-  useFocusLost(element, shouldFocusCard, () => setShouldFocusCard(false));
+  useFocusLost(element, shouldFocusCard, React.useCallback(() => setShouldFocusCard(false), [setShouldFocusCard]));
 
   const onHeightChange = React.useCallback((id: string, ref: React.RefObject<HTMLElement>) => {
     const height = ref.current && ref.current.offsetHeight;