Merge pull request #4992 from kobotoolbox/react-18-upgrade

Upgrade to React 18

.swcrc

@@ -1,4 +1,5 @@
 {
+  "$schema": "https://swc.rs/schema.json",
   "sourceMaps": true,
   "jsc": {
     "parser": {
@@ -7,7 +8,7 @@
     },
     "transform": {
       "react": {
-        "runtime": "classic",
+        "runtime": "automatic",
         "refresh": true
       }
     }

jsapp/js/account/organizations/requireOrgOwner.component.tsx

@@ -1,11 +1,11 @@
-import React, {ReactElement, Suspense, useContext, useEffect} from 'react';
-import {RouteObject, useNavigate} from 'react-router-dom';
+import React, {Suspense, useContext, useEffect} from 'react';
+import {useNavigate} from 'react-router-dom';
 import {OrganizationContext} from 'js/account/organizations/useOrganization.hook';
 import LoadingSpinner from 'js/components/common/loadingSpinner';
 import {ACCOUNT_ROUTES} from 'js/account/routes.constants';
 
 interface Props {
-  children: RouteObject[] | undefined | ReactElement;
+  children: React.ReactNode;
   redirect?: boolean;
 }
 

jsapp/js/account/plans/billingButton.component.tsx

@@ -2,7 +2,7 @@ import cx from 'classnames';
 import type {ButtonProps} from 'js/components/common/button';
 import Button from 'js/components/common/button';
 import React from 'react';
-import {button} from './billingButton.module.scss';
+import styles from './billingButton.module.scss';
 
 /**
  * The base button component that's used on the Plans/Add-ons pages.
@@ -16,7 +16,7 @@ export default function BillingButton(props: Partial<ButtonProps>) {
       color='blue'
       size='l'
       {...props}
-      className={cx([button, props.className])}
+      className={cx([styles.button, props.className])}
       isFullWidth
     />
   );

jsapp/js/account/plans/plan.component.tsx

@@ -341,7 +341,7 @@ export default function Plan(props: PlanProps) {
   const getSubscribedProduct = useCallback(getSubscriptionsForProductId, []);
 
   const isSubscribedProduct = useCallback(
-    (product: SinglePricedProduct, quantity = null) => {
+    (product: SinglePricedProduct, quantity: number | undefined) => {
       if (!product.price?.unit_amount && !hasActiveSubscription) {
         return true;
       }
@@ -356,7 +356,7 @@ export default function Plan(props: PlanProps) {
           (subscription: SubscriptionInfo) =>
             subscription.items[0].price.id === product.price.id &&
             hasManageableStatus(subscription) &&
-            quantity &&
+            quantity !== undefined &&
             quantity === subscription.quantity
         );
       }

jsapp/js/account/security/email/emailSection.component.tsx

@@ -170,6 +170,7 @@ export default function EmailSection() {
           value={email.newEmail}
           placeholder={t('Type new email address')}
           onChange={onTextFieldChange.bind(onTextFieldChange)}
+          type='email'
         />
 
         <Button
@@ -178,6 +179,9 @@ export default function EmailSection() {
           color='blue'
           type='frame'
           onClick={setNewUserEmail.bind(setNewUserEmail, email.newEmail)}
+          // quick simple subtle email validation to avoid complete accidents
+          // a toast showing any API error feedback would be nicer
+          isDisabled={!/[^@]+@[^@]+\.[^@]+/.test(email.newEmail)}
         />
       </form>
     </div>

jsapp/js/account/usage/usageContainer.tsx

@@ -47,11 +47,11 @@ const UsageContainer = ({
   /**
    * Render a limit amount, usage amount, or total balance as readable text
    * @param {number|'unlimited'} amount - The limit/usage amount
-   * @param {number|'unlimited'|null} [available=null] - If we're showing a balance,
+   * @param {number|'unlimited'} [available] - If we're showing a balance,
    * `amount` takes the usage amount and this takes the limit amount
    */
   const limitDisplay = useCallback(
-    (amount, available = null) => {
+    (amount: LimitAmount, available?: LimitAmount) => {
       if (amount === Limits.unlimited || available === Limits.unlimited) {
         return t('Unlimited');
       }

jsapp/js/components/assetsTable/assetActionButtons.tsx

@@ -228,8 +228,7 @@ class AssetActionButtons extends React.Component<
   }
 
   getFormBuilderLink() {
-    let link =
-      '#' + ROUTES.EDIT_LIBRARY_ITEM.replace(':uid', this.props.asset.uid);
+    let link = ROUTES.EDIT_LIBRARY_ITEM.replace(':uid', this.props.asset.uid);
 
     // when editing a child from within a collection page
     // make sure the "Return to list" button goes back to collection

jsapp/js/components/common/tooltip.tsx

@@ -9,6 +9,7 @@ export interface TooltipProps {
   ariaLabel: string;
   /** Position of the tooltip (centered as default) */
   alignment?: TooltipAlignment;
+  children?: React.ReactNode;
 }
 
 /**

jsapp/js/components/reports/reportTable.component.tsx

@@ -128,7 +128,10 @@ export default class ReportTable extends React.Component<ReportTableProps> {
           {rows.map((row) => (
             <tr key={row[0]}>
               {row.map((r, i: number) => (
-                <td key={i}>{r}</td>
+                // Note: See TODO above. ReportTableProps probably deserves a
+                // second look, and TypeScript is trying to tell us something
+                // about the value of 'r' here.
+                <td key={i}>{r as React.ReactNode}</td>
               ))}
             </tr>
           ))}

jsapp/js/components/submissions/submissionUtils.tests.es6

@@ -42,6 +42,15 @@ import {
 import chaiExclude from 'chai-exclude';
 chai.use(chaiExclude);
 
+// getSubmissionDisplayData might return objects with declared, undefined key:
+//    {... "label": "hi", "listName": undefined, "name": "hi" ...}
+// Assuming this is correct, test fixtures like this are equivalent enough:
+//    {... "label": "hi", "name": "hi" ...}
+// After a recent chai / deep-eql update, tests relying on this behavior would
+// fail. Hence, use this looser comparison function.
+import chaiDeepEqualIgnoreUndefined from 'chai-deep-equal-ignore-undefined'
+chai.use(chaiDeepEqualIgnoreUndefined);
+
 describe('getSubmissionDisplayData', () => {
   it('should return a valid data for a survey with a group', () => {
       const test = getSubmissionDisplayData(
@@ -55,7 +64,7 @@ describe('getSubmissionDisplayData', () => {
       const target = simpleSurveyDisplayData;
       expect(test)
         .excludingEvery(['__proto__', 'xpathNodes'])
-        .to.deep.equal(target);
+        .to.deepEqualIgnoreUndefined(target);
   });
 
   it('should return a null data entries for a survey with no answers', () => {
@@ -70,7 +79,7 @@ describe('getSubmissionDisplayData', () => {
       const target = simpleSurveyDisplayDataEmpty;
       expect(test)
         .excludingEvery(['__proto__', 'xpathNodes'])
-        .to.deep.equal(target);
+        .to.deepEqualIgnoreUndefined(target);
   });
 
   it('should return a valid data for a survey with a repeat group', () => {
@@ -85,7 +94,7 @@ describe('getSubmissionDisplayData', () => {
       const target = repeatSurveyDisplayData;
       expect(test)
         .excludingEvery(['__proto__', 'xpathNodes'])
-        .to.deep.equal(target);
+        .to.deepEqualIgnoreUndefined(target);
   });
 
   it('should return a valid data for a survey with nested repeat groups', () => {
@@ -98,7 +107,9 @@ describe('getSubmissionDisplayData', () => {
           },
         }, 0, nestedRepeatSurveySubmission).children;
       const target = nestedRepeatSurveyDisplayData;
-      expect(test).excludingEvery(['__proto__', 'xpathNodes']).to.deep.equal(target);
+      expect(test)
+        .excludingEvery(['__proto__', 'xpathNodes'])
+        .to.deepEqualIgnoreUndefined(target);
   });
 
   it('should return a valid data for a survey with a matrix', () => {
@@ -111,7 +122,9 @@ describe('getSubmissionDisplayData', () => {
           },
         }, 0, matrixSurveySubmission).children;
       const target = matrixSurveyDisplayData;
-      expect(test).excludingEvery(['__proto__', 'xpathNodes']).to.deep.equal(target);
+      expect(test)
+        .excludingEvery(['__proto__', 'xpathNodes'])
+        .to.deepEqualIgnoreUndefined(target);
   });
 
   it('should return a valid data for a survey with all kinds of groups', () => {
@@ -124,7 +137,9 @@ describe('getSubmissionDisplayData', () => {
           },
         }, 0, groupsSurveySubmission).children;
       const target = groupsSurveyDisplayData;
-      expect(test).excludingEvery(['__proto__', 'xpathNodes']).to.deep.equal(target);
+      expect(test)
+        .excludingEvery(['__proto__', 'xpathNodes'])
+        .to.deepEqualIgnoreUndefined(target);
   });
 
   it('should return a valid data for every possible question type', () => {
@@ -137,7 +152,9 @@ describe('getSubmissionDisplayData', () => {
           },
         }, 0, everythingSurveySubmission).children;
       const target = everythingSurveyDisplayData;
-      expect(test).excludingEvery(['__proto__', 'xpathNodes']).to.deep.equal(target);
+      expect(test)
+        .excludingEvery(['__proto__', 'xpathNodes'])
+        .to.deepEqualIgnoreUndefined(target);
   });
 
   it('should return a valid data for a matrix group inside repeat group', () => {
@@ -150,7 +167,9 @@ describe('getSubmissionDisplayData', () => {
           },
         }, 0, matrixRepeatSurveySubmission).children;
       const target = matrixRepeatSurveyDisplayData;
-      expect(test).excludingEvery(['__proto__', 'xpathNodes']).to.deep.equal(target);
+      expect(test)
+        .excludingEvery(['__proto__', 'xpathNodes'])
+        .to.deepEqualIgnoreUndefined(target);
   });
 });
 

jsapp/js/components/submissions/table.tsx

@@ -328,8 +328,9 @@ export class DataTable extends React.Component<DataTableProps, DataTableState> {
         submissions: results,
         submissionPager: false,
         resultsTotal: response.count,
+      }, () => {
+        this._prepColumns(results);
       });
-      this._prepColumns(results);
     } else if (options.filter?.length) {
       // if there are no results, but there is some filtering applied, we don't
       // want to display the "no data" message
@@ -1171,8 +1172,9 @@ export class DataTable extends React.Component<DataTableProps, DataTableState> {
       if (typeof subIndex !== 'undefined' && this.state.submissions[subIndex]) {
         const newData = this.state.submissions;
         newData[subIndex]._validation_status = result || {};
-        this.setState({submissions: newData});
-        this._prepColumns(newData);
+        this.setState({submissions: newData}, () => {
+          this._prepColumns(newData);
+        });
       }
     }
   }

jsapp/js/main.es6

@@ -3,27 +3,26 @@
  * plus it is the file that is handling the root rendering.
  */
 
-require('jquery-ui/ui/widgets/sortable');
+import 'jquery-ui/ui/widgets/sortable';
 import moment from 'moment';
 import AllRoutes from 'js/router/allRoutes';
 import RegistrationPasswordApp from './registrationPasswordApp';
-import {AppContainer} from 'react-hot-loader';
 import React from 'react';
 import {Cookies} from 'react-cookie';
-import {render} from 'react-dom';
+import {createRoot} from 'react-dom/client';
 import * as Sentry from '@sentry/react';
 import {csrfSafeMethod, currentLang} from 'utils';
-require('../scss/main.scss');
+import '../scss/main.scss';
 import Modal from 'react-modal';
 
-let sentryDsnEl = document.head.querySelector('meta[name=sentry-dsn]');
+const sentryDsnEl = document.head.querySelector('meta[name=sentry-dsn]');
 if (sentryDsnEl !== null) {
   Sentry.init({
     dsn: sentryDsnEl.content,
     tracesSampleRate: 0.0,
     sendClientReports: false,
     autoSessionTracking: false,
-  })
+  });
   window.Raven = Sentry; // Legacy use (formbuilder)
   /*
     In TS files, it's safe to do
@@ -86,22 +85,10 @@ if (document.head.querySelector('meta[name=kpi-root-path]')) {
     Modal.setAppElement('#kpiapp');
     return $d.get(0);
   })();
-
-  render(<AllRoutes />, el);
-
-  if (module.hot) {
-    module.hot.accept('js/app', () => {
-      const AllRoutes = require('js/app').default;
-      render(
-        <AppContainer>
-          <AllRoutes />
-        </AppContainer>,
-        el
-      );
-    });
-  }
+  const root = createRoot(el);
+  root.render(<AllRoutes />);
 } else {
-  console.error('no kpi-root-path meta tag set. skipping react-router init');
+  console.warn('no kpi-root-path meta tag set. skipping react-router init');
 }
 
 // Handles rendering a small app in the registration form
@@ -110,11 +97,9 @@ document.addEventListener('DOMContentLoaded', () => {
     'registration-password-app'
   );
   if (registrationPasswordAppEl) {
-    render(
-      <AppContainer>
+    const root = createRoot(registrationPasswordAppEl);
+    root.render(
         <RegistrationPasswordApp />
-      </AppContainer>,
-      registrationPasswordAppEl
     );
   }
 });

jsapp/js/router/requireAuth.tsx

@@ -1,11 +1,10 @@
-import React, {ReactElement, Suspense, useEffect, useState} from 'react';
-import {RouteObject} from 'react-router-dom';
+import React, {Suspense, useEffect, useState} from 'react';
 import sessionStore from 'js/stores/session';
 import LoadingSpinner from '../components/common/loadingSpinner';
 import {redirectToLogin} from './routerUtils';
 
 interface Props {
-  children: RouteObject[] | undefined | ReactElement;
+  children: React.ReactNode;
   redirect?: boolean;
 }