TypeScript cleanup (#1651)

* Include type-check in CI

* Clean up some TypeScript problems

* Fix Store/StoreGenerator confusion

* Update TypeScript

* Add comment

---------

Co-authored-by: Alexey Romanov <[email protected]>

.github/workflows/lint-js-and-ruby.yml

@@ -53,3 +53,5 @@ jobs:
       run: yarn start lint
     - name: Check formatting
       run: yarn start format.listDifferent
+    - name: Type-check TypeScript
+      run: yarn run type-check

.gitignore

@@ -32,3 +32,9 @@ npm-debug.*
 yalc.lock
 
 .byebug_history
+
+# IDE
+.idea/
+
+# TypeScript
+*.tsbuildinfo

CHANGELOG.md

@@ -18,6 +18,9 @@ Please follow the recommendations outlined at [keepachangelog.com](http://keepac
 ### [Unreleased]
 Changes since the last non-beta release.
 
+#### Fixed
+- Incorrect type and confusing name for `ReactOnRails.registerStore`, use `registerStoreGenerators` instead. [PR 1651](https://github.com/shakacode/react_on_rails/pull/1651) by [alexeyr-ci](https://github.com/alexeyr-ci).
+
 ### [14.0.5] - 2024-08-20
 #### Fixed
 - Should force load react-components which send over turbo-stream [PR #1620](https://github.com/shakacode/react_on_rails/pull/1620) by [theforestvn88](https://github.com/theforestvn88).

node_package/src/Authenticity.ts

@@ -3,7 +3,7 @@ import type { AuthenticityHeaders } from './types/index';
 export default {
   authenticityToken(): string | null {
     const token = document.querySelector('meta[name="csrf-token"]');
-    if (token && (token instanceof window.HTMLMetaElement)) {
+    if (token instanceof HTMLMetaElement) {
       return token.content;
     }
     return null;

node_package/src/ComponentRegistry.ts

@@ -1,7 +1,7 @@
 import type { RegisteredComponent, ReactComponentOrRenderFunction, RenderFunction } from './types/index';
 import isRenderFunction from './isRenderFunction';
 
-const registeredComponents = new Map();
+const registeredComponents = new Map<string, RegisteredComponent>();
 
 export default {
   /**
@@ -35,8 +35,9 @@ export default {
    * @returns { name, component, isRenderFunction, isRenderer }
    */
   get(name: string): RegisteredComponent {
-    if (registeredComponents.has(name)) {
-      return registeredComponents.get(name);
+    const registeredComponent = registeredComponents.get(name);
+    if (registeredComponent !== undefined) {
+      return registeredComponent;
     }
 
     const keys = Array.from(registeredComponents.keys()).join(', ');

node_package/src/ReactOnRails.ts

@@ -17,13 +17,11 @@ import type {
   ErrorOptions,
   ReactComponentOrRenderFunction,
   AuthenticityHeaders,
+  Store,
   StoreGenerator,
 } from './types';
 import reactHydrateOrRender from './reactHydrateOrRender';
 
-/* eslint-disable @typescript-eslint/no-explicit-any */
-type Store = any;
-
 const ctx = context();
 
 if (ctx === undefined) {
@@ -56,19 +54,23 @@ ctx.ReactOnRails = {
     ComponentRegistry.register(components);
   },
 
+  registerStore(stores: { [id: string]: StoreGenerator }): void {
+    this.registerStoreGenerators(stores);
+  },
+
   /**
-   * Allows registration of store generators to be used by multiple react components on one Rails
+   * Allows registration of store generators to be used by multiple React components on one Rails
    * view. store generators are functions that take one arg, props, and return a store. Note that
    * the setStore API is different in that it's the actual store hydrated with props.
-   * @param stores (keys are store names, values are the store generators)
+   * @param storeGenerators (keys are store names, values are the store generators)
    */
-  registerStore(stores: { [id: string]: Store }): void {
-    if (!stores) {
-      throw new Error('Called ReactOnRails.registerStores with a null or undefined, rather than ' +
+  registerStoreGenerators(storeGenerators: { [id: string]: StoreGenerator }): void {
+    if (!storeGenerators) {
+      throw new Error('Called ReactOnRails.registerStoreGenerators with a null or undefined, rather than ' +
         'an Object with keys being the store names and the values are the store generators.');
     }
 
-    StoreRegistry.register(stores);
+    StoreRegistry.register(storeGenerators);
   },
 
   /**
@@ -148,7 +150,7 @@ ctx.ReactOnRails = {
 
   /**
    * Returns header with csrf authenticity token and XMLHttpRequest
-   * @param {*} other headers
+   * @param otherHeaders Other headers
    * @returns {*} header
    */
 
@@ -171,7 +173,7 @@ ctx.ReactOnRails = {
 
   /**
    * Allows retrieval of the store generator by name. This is used internally by ReactOnRails after
-   * a rails form loads to prepare stores.
+   * a Rails form loads to prepare stores.
    * @param name
    * @returns Redux Store generator function
    */

node_package/src/StoreRegistry.ts

@@ -1,17 +1,14 @@
-import type { StoreGenerator } from './types';
+import type { Store, StoreGenerator } from './types';
 
-/* eslint-disable @typescript-eslint/no-explicit-any */
-type Store = any;
-
-const registeredStoreGenerators = new Map();
-const hydratedStores = new Map();
+const registeredStoreGenerators = new Map<string, StoreGenerator>();
+const hydratedStores = new Map<string, Store>();
 
 export default {
   /**
    * Register a store generator, a function that takes props and returns a store.
    * @param storeGenerators { name1: storeGenerator1, name2: storeGenerator2 }
    */
-  register(storeGenerators: { [id: string]: Store }): void {
+  register(storeGenerators: { [id: string]: StoreGenerator }): void {
     Object.keys(storeGenerators).forEach(name => {
       if (registeredStoreGenerators.has(name)) {
         console.warn('Called registerStore for store that is already registered', name);
@@ -66,8 +63,9 @@ This can happen if you are server rendering and either:
    * @returns storeCreator with given name
    */
   getStoreGenerator(name: string): StoreGenerator {
-    if (registeredStoreGenerators.has(name)) {
-      return registeredStoreGenerators.get(name);
+    const registeredStoreGenerator = registeredStoreGenerators.get(name);
+    if (registeredStoreGenerator) {
+      return registeredStoreGenerator;
     }
 
     const storeKeys = Array.from(registeredStoreGenerators.keys()).join(', ');

node_package/src/buildConsoleReplay.ts

@@ -1,8 +1,6 @@
 import RenderUtils from './RenderUtils';
 import scriptSanitizedVal from './scriptSanitizedVal';
 
-/* eslint-disable @typescript-eslint/no-explicit-any */
-
 declare global {
   interface Console {
     history?: {
@@ -13,24 +11,29 @@ declare global {
 
 export function consoleReplay(): string {
   // console.history is a global polyfill used in server rendering.
-  // $FlowFixMe
   if (!(console.history instanceof Array)) {
     return '';
   }
 
   const lines = console.history.map(msg => {
     const stringifiedList = msg.arguments.map(arg => {
-      let val;
+      let val: string;
       try {
-        val = (typeof arg === 'string' || arg instanceof String) ? arg : JSON.stringify(arg);
+        if (typeof arg === 'string') {
+          val = arg;
+        } else if (arg instanceof String) {
+          val = String(arg);
+        } else {
+          val = JSON.stringify(arg);
+        }
         if (val === undefined) {
           val = 'undefined';
         }
-      } catch (e: any) {
-        val = `${e.message}: ${arg}`;
+      } catch (e) {
+        val = `${(e as Error).message}: ${arg}`;
       }
 
-      return scriptSanitizedVal(val as string);
+      return scriptSanitizedVal(val);
     });
 
     return `console.${msg.level}.apply(console, ${JSON.stringify(stringifiedList)});`;

node_package/src/handleError.ts

@@ -2,7 +2,7 @@ import React from 'react';
 import ReactDOMServer from 'react-dom/server';
 import type { ErrorOptions } from './types/index';
 
-function handleRenderFunctionIssue(options: {e: Error; name?: string}): string {
+function handleRenderFunctionIssue(options: ErrorOptions): string {
   const { e, name } = options;
 
   let msg = '';

node_package/src/isRenderFunction.ts

@@ -8,11 +8,9 @@ import { ReactComponentOrRenderFunction, RenderFunction } from "./types/index";
  * @param component
  * @returns {boolean}
  */
-export default function isRenderFunction(component: ReactComponentOrRenderFunction): boolean {
+export default function isRenderFunction(component: ReactComponentOrRenderFunction): component is RenderFunction {
   // No for es5 or es6 React Component
-  if (
-    (component as RenderFunction).prototype &&
-    (component as RenderFunction).prototype.isReactComponent) {
+  if ((component as RenderFunction).prototype?.isReactComponent) {
     return false;
   }
 
@@ -22,7 +20,7 @@ export default function isRenderFunction(component: ReactComponentOrRenderFuncti
 
   // If zero or one args, then we know that this is a regular function that will
   // return a React component
-  if (component.length >= 2) {
+  if ((component as RenderFunction).length >= 2) {
     return true;
   }
 

node_package/src/serverRenderReactComponent.ts

@@ -3,11 +3,10 @@ import type { ReactElement } from 'react';
 
 import ComponentRegistry from './ComponentRegistry';
 import createReactOutput from './createReactOutput';
-import {isServerRenderHash, isPromise} from
-    './isServerRenderResult';
+import { isPromise, isServerRenderHash } from './isServerRenderResult';
 import buildConsoleReplay from './buildConsoleReplay';
 import handleError from './handleError';
-import type { RenderParams, RenderResult, RenderingError } from './types/index';
+import type { RenderParams, RenderResult, RenderingError, ServerRenderResult } from './types';
 
 /* eslint-disable @typescript-eslint/no-explicit-any */
 
@@ -35,37 +34,37 @@ See https://github.com/shakacode/react_on_rails#renderer-functions`);
     });
 
     const processServerRenderHash = () => {
-        // We let the client side handle any redirect
-        // Set hasErrors in case we want to throw a Rails exception
-        hasErrors = !!(reactRenderingResult as {routeError: Error}).routeError;
-
-        if (hasErrors) {
-          console.error(
-            `React Router ERROR: ${JSON.stringify((reactRenderingResult as {routeError: Error}).routeError)}`,
-          );
-        }
+      // We let the client side handle any redirect
+      // Set hasErrors in case we want to throw a Rails exception
+      const { redirectLocation, routeError } = reactRenderingResult as ServerRenderResult;
+      hasErrors = !!routeError;
+
+      if (hasErrors) {
+        console.error(
+          `React Router ERROR: ${JSON.stringify(routeError)}`,
+        );
+      }
 
-        if ((reactRenderingResult as {redirectLocation: {pathname: string; search: string}}).redirectLocation) {
-          if (trace) {
-            const { redirectLocation } = (reactRenderingResult as {redirectLocation: {pathname: string; search: string}});
-            const redirectPath = redirectLocation.pathname + redirectLocation.search;
-            console.log(`\
+      if (redirectLocation) {
+        if (trace) {
+          const redirectPath = redirectLocation.pathname + redirectLocation.search;
+          console.log(`\
   ROUTER REDIRECT: ${name} to dom node with id: ${domNodeId}, redirect to ${redirectPath}`,
-            );
-          }
-          // For redirects on server rendering, we can't stop Rails from returning the same result.
-          // Possibly, someday, we could have the rails server redirect.
-          return '';
+          );
         }
-        return (reactRenderingResult as { renderedHtml: string }).renderedHtml;
+        // For redirects on server rendering, we can't stop Rails from returning the same result.
+        // Possibly, someday, we could have the rails server redirect.
+        return '';
+      }
+      return (reactRenderingResult as ServerRenderResult).renderedHtml as string;
     };
 
     const processPromise = () => {
       if (!renderingReturnsPromises) {
-        console.error('Your render function returned a Promise, which is only supported by a node renderer, not ExecJS.')
+        console.error('Your render function returned a Promise, which is only supported by a node renderer, not ExecJS.');
       }
       return reactRenderingResult;
-    }
+    };
 
     const processReactElement = () => {
       try {
@@ -101,15 +100,16 @@ as a renderFunction and not a simple React Function Component.`);
 
   const consoleReplayScript = buildConsoleReplay();
   const addRenderingErrors = (resultObject: RenderResult, renderError: RenderingError) => {
+    // Do not use `resultObject.renderingError = renderError` because JSON.stringify will turn it into '{}'.
     resultObject.renderingError = { // eslint-disable-line no-param-reassign
       message: renderError.message,
       stack: renderError.stack,
     };
-  }
+  };
 
-  if(renderingReturnsPromises) {
+  if (renderingReturnsPromises) {
     const resolveRenderResult = async () => {
-      let promiseResult;
+      let promiseResult: RenderResult;
 
       try {
         promiseResult = {
@@ -129,7 +129,7 @@ as a renderFunction and not a simple React Function Component.`);
           }),
           consoleReplayScript,
           hasErrors: true,
-        }
+        };
         renderingError = e;
       }
 
@@ -143,11 +143,11 @@ as a renderFunction and not a simple React Function Component.`);
     return resolveRenderResult();
   }
 
-  const result = {
-    html: renderResult,
+  const result: RenderResult = {
+    html: renderResult as string,
     consoleReplayScript,
     hasErrors,
-  } as RenderResult;
+  };
 
   if (renderingError) {
     addRenderingErrors(result, renderingError);