Add typechecks, typescript coverage GH action, fix many type errors (#…

…3136)

.github/workflows/js.yml

@@ -29,10 +29,56 @@ jobs:
         run: yarn run format-check
         working-directory: ./js
 
+  typecheck:
+    name: Typecheck
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Check out code
+        uses: actions/checkout@v2
+
+      - name: Set up Node
+        uses: actions/setup-node@v2
+        with:
+          node-version: ${{ env.NODE_VERSION }}
+          cache: "yarn"
+          cache-dependency-path: js/yarn.lock
+
+      - name: Install JS dependencies
+        run: yarn --frozen-lockfile
+        working-directory: ./js
+
+      - name: Typecheck
+        run: yarn run check-typings || true  # REMOVE THIS ONCE TYPE SAFETY REACHED
+        working-directory: ./js
+
+  type-coverage:
+    name: Type Coverage
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Check out code
+        uses: actions/checkout@v2
+
+      - name: Set up Node
+        uses: actions/setup-node@v2
+        with:
+          node-version: ${{ env.NODE_VERSION }}
+          cache: "yarn"
+          cache-dependency-path: js/yarn.lock
+
+      - name: Install JS dependencies
+        run: yarn --frozen-lockfile
+        working-directory: ./js
+
+      - name: Check type coverage
+        run: yarn run check-typings-coverage
+        working-directory: ./js
+
   build-prod:
     name: Build and commit
     runs-on: ubuntu-latest
-    needs: [prettier]
+    needs: [prettier, typecheck, type-coverage]
 
     # Only commit JS on push to master branch
     # Remember to change in `build-test` job too
@@ -62,7 +108,7 @@ jobs:
   build-test:
     name: Test build
     runs-on: ubuntu-latest
-    needs: [prettier]
+    needs: [prettier, typecheck, type-coverage]
 
     # Inverse check of `build-prod`
     # Remember to change in `build-prod` job too

.gitignore

@@ -8,3 +8,4 @@ tests/.phpunit.result.cache
 .vagrant
 .idea/*
 .vscode
+js/coverage-ts

js/package.json

@@ -30,6 +30,7 @@
     "flarum-webpack-config": "^2.0.0",
     "prettier": "^2.4.1",
     "typescript": "^4.4.4",
+    "typescript-coverage-report": "^0.6.1",
     "webpack": "^5.61.0",
     "webpack-cli": "^4.9.1",
     "webpack-merge": "^5.8.0"
@@ -41,7 +42,9 @@
     "format": "prettier --write src",
     "format-check": "prettier --check src",
     "clean-typings": "npx rimraf dist-typings && mkdir dist-typings",
-    "build-typings": "npm run clean-typings && cp -r src/@types dist-typings/@types && tsc"
+    "build-typings": "npm run clean-typings && cp -r src/@types dist-typings/@types && tsc",
+    "check-typings": "tsc --noEmit --emitDeclarationOnly false",
+    "check-typings-coverage": "typescript-coverage-report"
   },
   "packageManager": "yarn@3.1.0"
 }

js/src/@types/global.d.ts

@@ -98,3 +98,10 @@ interface JSX {
     attrs: Record<string, unknown>;
   };
 }
+
+interface Event {
+  /**
+   * Whether this event should trigger a Mithril redraw.
+   */
+  redraw: boolean;
+}

js/src/admin/AdminApplication.ts

@@ -6,6 +6,32 @@ import Navigation from '../common/components/Navigation';
 import AdminNav from './components/AdminNav';
 import ExtensionData from './utils/ExtensionData';
 
+export type Extension = {
+  id: string;
+  version: string;
+  description?: string;
+  icon?: {
+    name: string;
+  };
+  links: {
+    authors?: {
+      name?: string;
+      link?: string;
+    }[];
+    discuss?: string;
+    documentation?: string;
+    support?: string;
+    website?: string;
+    donate?: string;
+    source?: string;
+  };
+  extra: {
+    'flarum-extension': {
+      title: string;
+    };
+  };
+};
+
 export default class AdminApplication extends Application {
   extensionData = new ExtensionData();
 
@@ -24,6 +50,20 @@ export default class AdminApplication extends Application {
     },
   };
 
+  /**
+   * Settings are serialized to the admin dashboard as strings.
+   * Additional encoding/decoding is possible, but must take
+   * place on the client side.
+   *
+   * @inheritdoc
+   */
+
+  data!: Application['data'] & {
+    extensions: Record<string, Extension>;
+    settings: Record<string, string>;
+    modelStatistics: Record<string, { total: number }>;
+  };
+
   constructor() {
     super();
 
@@ -41,20 +81,20 @@ export default class AdminApplication extends Application {
     m.route.prefix = '#';
     super.mount();
 
-    m.mount(document.getElementById('app-navigation'), {
+    m.mount(document.getElementById('app-navigation')!, {
       view: () =>
         Navigation.component({
           className: 'App-backControl',
           drawer: true,
         }),
     });
-    m.mount(document.getElementById('header-navigation'), Navigation);
-    m.mount(document.getElementById('header-primary'), HeaderPrimary);
-    m.mount(document.getElementById('header-secondary'), HeaderSecondary);
-    m.mount(document.getElementById('admin-navigation'), AdminNav);
+    m.mount(document.getElementById('header-navigation')!, Navigation);
+    m.mount(document.getElementById('header-primary')!, HeaderPrimary);
+    m.mount(document.getElementById('header-secondary')!, HeaderSecondary);
+    m.mount(document.getElementById('admin-navigation')!, AdminNav);
   }
 
-  getRequiredPermissions(permission) {
+  getRequiredPermissions(permission: string) {
     const required = [];
 
     if (permission === 'startDiscussion' || permission.indexOf('discussion.') === 0) {

js/src/admin/app.ts

@@ -2,7 +2,7 @@ import Admin from './AdminApplication';
 
 const app = new Admin();
 
-// @ts-ignore
+// @ts-expect-error We need to do this for backwards compatibility purposes.
 window.app = app;
 
 export default app;

js/src/admin/components/ExtensionPage.js → js/src/admin/components/ExtensionPage.tsx

@@ -12,26 +12,39 @@ import ExtensionPermissionGrid from './ExtensionPermissionGrid';
 import isExtensionEnabled from '../utils/isExtensionEnabled';
 import AdminPage from './AdminPage';
 import ReadmeModal from './ReadmeModal';
+import RequestError from '../../common/utils/RequestError';
+import { Extension } from '../AdminApplication';
+import { IPageAttrs } from '../../common/components/Page';
+import type Mithril from 'mithril';
 
-export default class ExtensionPage extends AdminPage {
-  oninit(vnode) {
-    super.oninit(vnode);
+export interface ExtensionPageAttrs extends IPageAttrs {
+  id: string;
+}
 
-    this.extension = app.data.extensions[this.attrs.id];
-    this.changingState = false;
+export default class ExtensionPage<Attrs extends ExtensionPageAttrs = ExtensionPageAttrs> extends AdminPage<Attrs> {
+  extension!: Extension;
+
+  changingState = false;
 
-    this.infoFields = {
-      discuss: 'fas fa-comment-alt',
-      documentation: 'fas fa-book',
-      support: 'fas fa-life-ring',
-      website: 'fas fa-link',
-      donate: 'fas fa-donate',
-      source: 'fas fa-code',
-    };
+  infoFields = {
+    discuss: 'fas fa-comment-alt',
+    documentation: 'fas fa-book',
+    support: 'fas fa-life-ring',
+    website: 'fas fa-link',
+    donate: 'fas fa-donate',
+    source: 'fas fa-code',
+  };
 
-    if (!this.extension) {
+  oninit(vnode: Mithril.Vnode<Attrs, this>) {
+    super.oninit(vnode);
+
+    const extension = app.data.extensions[this.attrs.id];
+
+    if (!extension) {
       return m.route.set(app.route('dashboard'));
     }
+
+    this.extension = extension;
   }
 
   className() {
@@ -40,7 +53,7 @@ export default class ExtensionPage extends AdminPage {
     return this.extension.id + '-Page';
   }
 
-  view() {
+  view(vnode: Mithril.VnodeDOM<Attrs, this>) {
     if (!this.extension) return null;
 
     return (
@@ -51,7 +64,7 @@ export default class ExtensionPage extends AdminPage {
             <h3 className="ExtensionPage-subHeader">{app.translator.trans('core.admin.extension.enable_to_see')}</h3>
           </div>
         ) : (
-          <div className="ExtensionPage-body">{this.sections().toArray()}</div>
+          <div className="ExtensionPage-body">{this.sections(vnode).toArray()}</div>
         )}
       </div>
     );
@@ -92,10 +105,10 @@ export default class ExtensionPage extends AdminPage {
     ];
   }
 
-  sections() {
+  sections(vnode: Mithril.VnodeDOM<Attrs, this>) {
     const items = new ItemList();
 
-    items.add('content', this.content());
+    items.add('content', this.content(vnode));
 
     items.add('permissions', [
       <div className="ExtensionPage-permissions">
@@ -117,7 +130,7 @@ export default class ExtensionPage extends AdminPage {
     return items;
   }
 
-  content() {
+  content(vnode: Mithril.VnodeDOM<Attrs, this>) {
     const settings = app.extensionData.getSettings(this.extension.id);
 
     return (
@@ -126,7 +139,7 @@ export default class ExtensionPage extends AdminPage {
           {settings ? (
             <div className="Form">
               {settings.map(this.buildSettingComponent.bind(this))}
-              <div className="Form-group">{this.submitButton()}</div>
+              <div className="Form-group">{this.submitButton(vnode)}</div>
             </div>
           ) : (
             <h3 className="ExtensionPage-subHeader">{app.translator.trans('core.admin.extension.no_settings')}</h3>
@@ -172,21 +185,17 @@ export default class ExtensionPage extends AdminPage {
 
     const links = this.extension.links;
 
-    if (links.authors.length) {
-      let authors = [];
-
-      links.authors.map((author) => {
-        authors.push(
-          <Link href={author.link} external={true} target="_blank">
-            {author.name}
-          </Link>
-        );
-      });
+    if (links.authors?.length) {
+      const authors = links.authors.map((author) => (
+        <Link href={author.link} external={true} target="_blank">
+          {author.name}
+        </Link>
+      ));
 
       items.add('authors', [icon('fas fa-user'), <span>{punctuateSeries(authors)}</span>]);
     }
 
-    Object.keys(this.infoFields).map((field) => {
+    (Object.keys(this.infoFields) as (keyof ExtensionPage['infoFields'])[]).map((field) => {
       if (links[field]) {
         items.add(
           field,
@@ -240,7 +249,7 @@ export default class ExtensionPage extends AdminPage {
     return isExtensionEnabled(this.extension.id);
   }
 
-  onerror(e) {
+  onerror(e: RequestError) {
     // We need to give the modal animation time to start; if we close the modal too early,
     // it breaks the bootstrap modal library.
     // TODO: This workaround should be removed when we move away from bootstrap JS for modals.
@@ -254,14 +263,16 @@ export default class ExtensionPage extends AdminPage {
       throw e;
     }
 
-    const error = e.response.errors[0];
+    const error = e.response?.errors?.[0];
 
-    app.alerts.show(
-      { type: 'error' },
-      app.translator.trans(`core.lib.error.${error.code}_message`, {
-        extension: error.extension,
-        extensions: error.extensions.join(', '),
-      })
-    );
+    if (error) {
+      app.alerts.show(
+        { type: 'error' },
+        app.translator.trans(`core.lib.error.${error.code}_message`, {
+          extension: error.extension,
+          extensions: (error.extensions as string[]).join(', '),
+        })
+      );
+    }
   }
 }

js/src/admin/components/LoadingModal.js → js/src/admin/components/LoadingModal.tsx

@@ -1,11 +1,11 @@
 import app from '../../admin/app';
 import Modal from '../../common/components/Modal';
 
-export default class LoadingModal extends Modal {
+export default class LoadingModal<ModalAttrs = {}> extends Modal<ModalAttrs> {
   /**
    * @inheritdoc
    */
-  static isDismissible = false;
+  static readonly isDismissible: boolean = false;
 
   className() {
     return 'LoadingModal Modal--small';
@@ -18,4 +18,8 @@ export default class LoadingModal extends Modal {
   content() {
     return '';
   }
+
+  onsubmit(e: Event): void {
+    throw new Error('LoadingModal should not throw errors.');
+  }
 }

js/src/admin/index.ts

@@ -8,6 +8,7 @@ export { app };
 import compatObj from './compat';
 import proxifyCompat from '../common/utils/proxifyCompat';
 
+// @ts-expect-error The `app` instance needs to be available on compat.
 compatObj.app = app;
 
 export const compat = proxifyCompat(compatObj, 'admin');