- more code review

- also make VS Code use the repo TS version
paranext · Aug 29, 2024 · adcc586 · adcc586
1 parent 5075672
commit adcc586
Show file tree

Hide file tree

Showing 26 changed files with 91 additions and 168 deletions.
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -23,6 +23,7 @@
   "javascript.validate.enable": false,
   "javascript.format.enable": false,
   "typescript.format.enable": false,
+  "typescript.tsdk": "node_modules/typescript/lib",
 
   "search.exclude": {
     ".git": true,

diff --git a/extensions/lib/add-remotes.ts b/extensions/lib/add-remotes.ts
@@ -10,42 +10,29 @@ import {
 
 (async () => {
   let exitCode = 0;
-
-  // Helper function to handle remote addition
-  async function addRemote(name: string, url: string, errorString: string) {
-    try {
-      await execGitCommand(`git remote add ${name} ${url}`);
-    } catch (error: unknown) {
-      if (error instanceof Error) {
-        const errorMessage = error.message.toLowerCase();
-        if (includes(errorMessage, errorString.toLowerCase())) {
-          console.log(`Remote ${name} already exists. This is likely not a problem.`);
-        } else {
-          console.error(`Error on adding remote ${name}: ${error.message}`);
-          return 1;
-        }
-      } else {
-        console.error(`An unknown error occurred while adding remote ${name}`);
-        return 1;
-      }
+  // Try adding MULTI_TEMPLATE_REMOTE_NAME
+  try {
+    await execGitCommand(`git remote add ${MULTI_TEMPLATE_NAME} ${MULTI_TEMPLATE_URL}`);
+  } catch (e) {
+    if (includes(e.toString().toLowerCase(), ERROR_STRINGS.multiRemoteExists.toLowerCase()))
+      console.log(`Remote ${MULTI_TEMPLATE_NAME} already exists. This is likely not a problem.`);
+    else {
+      console.error(`Error on adding remote ${MULTI_TEMPLATE_NAME}: ${e}`);
+      exitCode = 1;
     }
-    return 0;
   }
 
-  // Try adding MULTI_TEMPLATE_REMOTE_NAME
-  exitCode = await addRemote(
-    MULTI_TEMPLATE_NAME,
-    MULTI_TEMPLATE_URL,
-    ERROR_STRINGS.multiRemoteExists,
-  );
-  if (exitCode !== 0) return exitCode;
-
   // Try adding SINGLE_TEMPLATE_REMOTE_NAME
-  exitCode = await addRemote(
-    SINGLE_TEMPLATE_NAME,
-    SINGLE_TEMPLATE_URL,
-    ERROR_STRINGS.singleRemoteExists,
-  );
+  try {
+    await execGitCommand(`git remote add ${SINGLE_TEMPLATE_NAME} ${SINGLE_TEMPLATE_URL}`);
+  } catch (e) {
+    if (includes(e.toString().toLowerCase(), ERROR_STRINGS.singleRemoteExists.toLowerCase()))
+      console.log(`Remote ${SINGLE_TEMPLATE_NAME} already exists. This is likely not a problem.`);
+    else {
+      console.error(`Error on adding remote ${SINGLE_TEMPLATE_NAME}: ${e}`);
+      exitCode = 1;
+    }
+  }
 
   return exitCode;
 })();
diff --git a/extensions/lib/git.util.ts b/extensions/lib/git.util.ts
@@ -1,8 +1,7 @@
-import { exec, ExecException, ExecOptions } from 'child_process';
+import { exec, ExecOptions } from 'child_process';
 import { promisify } from 'util';
 import path from 'path';
 import replaceInFile from 'replace-in-file';
-import { subtreeRootFolder } from '../webpack/webpack.util';
 
 const execAsync = promisify(exec);
 
@@ -43,19 +42,19 @@ const GIT_CONSTANTS = Object.freeze({
   SINGLE_TEMPLATE_BRANCH,
 });
 
-type GitConstantKeys = keyof typeof GIT_CONSTANTS;
-
 /**
  * Formats a string, replacing `GIT_CONSTANTS` values in brackets like `{MULTI_TEMPLATE_NAME}` and
  * such with their equivalent actual values
  *
  * @param str String to format
+ * @param args Rest args of strings to replace `{x}` with, where x is the arg index - 1
  * @returns Formatted string
  */
 function formatGitErrorTemplate(str: string): string {
-  return str.replace(/{([^}]+)}/g, (match, key: GitConstantKeys) =>
-    key in GIT_CONSTANTS ? GIT_CONSTANTS[key] : match,
-  );
+  return str.replace(/{([^}]+)}/g, function replaceTemplateNumber(match) {
+    const matchingGitConstant = GIT_CONSTANTS[match.slice(1, -1)];
+    return matchingGitConstant !== undefined ? matchingGitConstant : match;
+  });
 }
 
 /** Error strings to be checked for in git output for various reasons */
@@ -84,25 +83,16 @@ export async function execGitCommand(
   if (!quiet) console.log(`\n> ${command}`);
   try {
     const result = await execAsync(command, {
-      cwd: path.resolve(path.join(__dirname, '..', '..')),
+      cwd: path.resolve(path.join(__dirname, '..')),
       ...execOptions,
     });
     if (!quiet && result.stdout) console.log(result.stdout);
     if (!quiet && result.stderr) console.log(result.stderr);
     return result;
-  } catch (error: unknown) {
-    if (error instanceof Error) {
-      // Use the more specific type for `exec`.
-      // eslint-disable-next-line no-type-assertion/no-type-assertion
-      const execError = error as ExecException;
-      throw new Error(
-        `code ${execError.code}!${execError.stderr ? `\n${execError.stderr}` : ''}${
-          execError.stdout ? `\n${execError.stdout}` : ''
-        }`,
-      );
-    } else {
-      throw new Error('An unknown error occurred while executing git command');
-    }
+  } catch (e) {
+    throw new Error(
+      `code ${e.code}!${e.stderr ? `\n${e.stderr}` : ''}${e.stdout ? `\n${e.stdout}` : ''}`,
+    );
   }
 }
 
@@ -139,31 +129,13 @@ export async function fetchFromSingleTemplate() {
   // Fetch latest SINGLE_TEMPLATE_REMOTE_NAME branch
   try {
     await execGitCommand(`git fetch ${SINGLE_TEMPLATE_NAME} ${SINGLE_TEMPLATE_BRANCH}`);
-  } catch (error: unknown) {
-    if (error instanceof Error) {
-      console.error(`Error on git fetch on ${SINGLE_TEMPLATE_NAME}: ${error.message}`);
-    } else {
-      console.error(`An unknown error occurred while fetching from ${SINGLE_TEMPLATE_NAME}`);
-    }
+  } catch (e) {
+    console.error(`Error on git fetch on ${SINGLE_TEMPLATE_NAME}: ${e}`);
     return false;
   }
   return true;
 }
 
-/** Globs to ignore when replacing stuff while formatting extensions */
-const replaceInFileIgnoreGlobs = [
-  '**/node_modules/**/*',
-  '**/temp-build/**/*',
-  '**/logs/**/*',
-  '**/*.log',
-  '**/.eslintcache',
-  '**/dist/**/*',
-  '**/release/**/*',
-  // With npm workspaces, child workspace package-lock.json files are not used. Let's not format
-  // them so they can stay the same as how they were in the template to avoid merge conflicts
-  '**/package-lock.json',
-];
-
 /**
  * Format an extension folder to make the extension template folder work as a subfolder of this repo
  *
@@ -173,41 +145,26 @@ const replaceInFileIgnoreGlobs = [
  * @param extensionFolderPath Path to the extension to format relative to root
  */
 export async function formatExtensionFolder(extensionFolderPath: string) {
-  /**
-   * Path to the extension to format relative to the extensions folder (where the npm script is
-   * running).
-   *
-   * We need to take out the path from root to where npm is running its script because replaceInFile
-   * works relative to the npm folder. Setting `cwd` and `glob.cwd` did not work because
-   * replaceInFile was not properly offsetting the path before passing to fs
-   */
-  const extensionFolderPathFromExtensions = extensionFolderPath.replace(
-    `${subtreeRootFolder}/`,
-    '',
-  );
-  const results =
-    // Replace ../paranext-core with ../../../paranext-core to fix ts-config and package.json and such
-    (
-      await replaceInFile({
-        files: `${extensionFolderPathFromExtensions}/**/*`,
-        ignore: replaceInFileIgnoreGlobs,
-        from: /([^/])\.\.\/paranext-core/g,
-        to: '$1../../..',
-        countMatches: true,
-        allowEmptyPaths: true,
-      })
-    ).concat(
-      // Remove the type reference to external extensions since bundled extensions shouldn't use them
-      await replaceInFile({
-        files: `${extensionFolderPathFromExtensions}/tsconfig.json`,
-        ignore: replaceInFileIgnoreGlobs,
-        from: /("src\/types"),\n\n[\w\W]+dev-appdata\/cache\/extension-types"/g,
-        to: '$1',
-        countMatches: true,
-        allowEmptyPaths: true,
-      }),
-    );
-
+  // Replace ../paranext-core with ../../../paranext-core to fix ts-config and package.json and such
+  const results = await replaceInFile({
+    files: `${extensionFolderPath}/**/*`,
+    ignore: [
+      '**/node_modules/**/*',
+      '**/temp-build/**/*',
+      '**/logs/**/*',
+      '**/*.log',
+      '**/.eslintcache',
+      '**/dist/**/*',
+      '**/release/**/*',
+      // With npm workspaces, child workspace package-lock.json files are not used. Let's not format
+      // them so they can stay the same as how they were in the template to avoid merge conflicts
+      '**/package-lock.json',
+    ],
+    from: /([^/])\.\.\/paranext-core/g,
+    to: '$1../../../paranext-core',
+    countMatches: true,
+    allowEmptyPaths: true,
+  });
   const replaceStats = results.reduce(
     (replacements, replaceResult) => ({
       totalReplacements: replacements.totalReplacements + (replaceResult.numReplacements ?? 0),

diff --git a/extensions/lib/update-from-templates.ts b/extensions/lib/update-from-templates.ts
@@ -1,4 +1,3 @@
-import { includes } from 'platform-bible-utils';
 import {
   ERROR_STRINGS,
   MULTI_TEMPLATE_BRANCH,
@@ -10,7 +9,7 @@ import {
   fetchFromSingleTemplate,
   formatExtensionFolder,
 } from './git.util';
-import { ExtensionInfo, getExtensions, subtreeRootFolder } from '../webpack/webpack.util';
+import { ExtensionInfo, getExtensions } from '../webpack/webpack.util';
 
 (async () => {
   // Make sure there are not working changes as this will not work with working changes
@@ -27,7 +26,7 @@ import { ExtensionInfo, getExtensions, subtreeRootFolder } from '../webpack/webp
   // Merge changes from MULTI_TEMPLATE_REMOTE_NAME into this repo
   try {
     await execGitCommand(
-      `git subtree pull --prefix ${subtreeRootFolder} ${MULTI_TEMPLATE_NAME} ${MULTI_TEMPLATE_BRANCH} --squash`,
+      `git merge ${MULTI_TEMPLATE_NAME}/${MULTI_TEMPLATE_BRANCH} --allow-unrelated-histories`,
     );
   } catch (e) {
     console.error(`Error merging from ${MULTI_TEMPLATE_NAME}: ${e}`);
@@ -61,31 +60,23 @@ import { ExtensionInfo, getExtensions, subtreeRootFolder } from '../webpack/webp
 
       // We successfully pulled this subtree, so it is based on the template
       extensionsBasedOnTemplate.push(ext);
-    } catch (error: unknown) {
-      if (error instanceof Error) {
-        const errorMessage = error.message.toLowerCase();
-        if (
-          includes(
-            errorMessage,
+    } catch (e) {
+      if (
+        e
+          .toString()
+          .toLowerCase()
+          .includes(
             ERROR_STRINGS.subtreeNeverAdded.replace('{0}', ext.dirPathOSIndependent).toLowerCase(),
           )
-        ) {
-          // If this folder isn't a subtree, it may be intentionally not based on the template. Continue
-          console.warn(
-            `${ext.dirName} was never added as a subtree of ${SINGLE_TEMPLATE_NAME}. Feel free to ignore this if this folder is not supposed to be based on ${SINGLE_TEMPLATE_NAME}.\nIf this folder is supposed to be based on ${SINGLE_TEMPLATE_NAME}, move the folder elsewhere, run \`npm run create-extension -- ${ext.dirName}\`, drop the folder back in, and evaluate all working changes before committing.\n`,
-          );
-        } else {
-          console.error(
-            `Error pulling from ${SINGLE_TEMPLATE_NAME} to ${ext.dirName}: ${error.message}`,
-          );
-          // You can only fix merge conflicts on one subtree at a time, so stop
-          // if we hit an error like merge conflicts
-          return 1;
-        }
-      } else {
-        console.error(
-          `An unknown error occurred while pulling from ${SINGLE_TEMPLATE_NAME} to ${ext.dirName}`,
+      )
+        // If this folder isn't a subtree, it may be intentionally not based on the template. Continue
+        console.warn(
+          `${ext.dirName} was never added as a subtree of ${SINGLE_TEMPLATE_NAME}. Feel free to ignore this if this folder is not supposed to be based on ${SINGLE_TEMPLATE_NAME}.\nIf this folder is supposed to be based on ${SINGLE_TEMPLATE_NAME}, move the folder elsewhere, run \`npm run create-extension -- ${ext.dirName}\`, drop the folder back in, and evaluate all working changes before committing.\n`,
         );
+      else {
+        console.error(`Error pulling from ${SINGLE_TEMPLATE_NAME} to ${ext.dirName}: ${e}`);
+        // You can only fix merge conflicts on one subtree at a time, so stop
+        // if we hit an error like merge conflicts
         return 1;
       }
     }

diff --git a/extensions/package.json b/extensions/package.json
@@ -25,7 +25,6 @@
     "lint:staged": "lint-staged -q",
     "postinstall": "tsx ./lib/add-remotes.ts",
     "create-extension": "tsx ./lib/create-extension.ts",
-    "typecheck": "tsc -p ./tsconfig.lint.json",
     "update-from-templates": "tsx ./lib/update-from-templates.ts"
   },
   "lint-staged": {

diff --git a/extensions/src/c-sharp-provider-test/index.d.ts b/extensions/src/c-sharp-provider-test/index.d.ts
@@ -1,4 +1,5 @@
 declare module 'c-sharp-provider-test' {
+  // @ts-ignore: TS2307 - Cannot find module '@papi/core' or its corresponding type declarations
   import { IDataProvider, DataProviderDataType } from '@papi/core';
 
   type TimeDataTypes = {

diff --git a/extensions/src/hello-someone/package.json b/extensions/src/hello-someone/package.json
@@ -23,7 +23,8 @@
     "lint:scripts": "eslint --ext .cjs,.js,.jsx,.ts,.tsx --cache .",
     "lint:styles": "stylelint **/*.{css,scss}",
     "lint-fix": "npm run lint-fix:scripts && npm run lint:styles -- --fix",
-    "lint-fix:scripts": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint:scripts"
+    "lint-fix:scripts": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint:scripts",
+    "typecheck": "tsc -p ./tsconfig.json"
   },
   "browserslist": [],
   "peerDependencies": {

diff --git a/extensions/src/hello-someone/src/types/hello-someone.d.ts b/extensions/src/hello-someone/src/types/hello-someone.d.ts
@@ -1,4 +1,5 @@
 declare module 'hello-someone' {
+  // @ts-ignore: TS2307 - Cannot find module '@papi/core' or its corresponding type declarations
   import { IDataProvider, DataProviderDataType } from '@papi/core';
 
   export type Person = {

diff --git a/extensions/src/hello-world/package.json b/extensions/src/hello-world/package.json
@@ -23,7 +23,8 @@
     "lint:scripts": "eslint --ext .cjs,.js,.jsx,.ts,.tsx --cache .",
     "lint:styles": "stylelint **/*.{css,scss}",
     "lint-fix": "npm run lint-fix:scripts && npm run lint:styles -- --fix",
-    "lint-fix:scripts": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint:scripts"
+    "lint-fix:scripts": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint:scripts",
+    "typecheck": "tsc -p ./tsconfig.json"
   },
   "browserslist": [],
   "peerDependencies": {

diff --git a/extensions/src/hello-world/src/types/hello-world.d.ts b/extensions/src/hello-world/src/types/hello-world.d.ts
@@ -1,4 +1,5 @@
 declare module 'hello-world' {
+  // @ts-ignore: TS2307 - Cannot find module '@papi/core' or its corresponding type declarations
   import type { DataProviderDataType, MandatoryProjectDataTypes } from '@papi/core';
   import type { IBaseProjectDataProvider } from 'papi-shared-types';
 

diff --git a/extensions/src/platform-scripture/package.json b/extensions/src/platform-scripture/package.json
@@ -23,7 +23,8 @@
     "lint:scripts": "eslint --ext .cjs,.js,.jsx,.ts,.tsx --cache .",
     "lint:styles": "stylelint **/*.{css,scss}",
     "lint-fix": "npm run lint-fix:scripts && npm run lint:styles -- --fix",
-    "lint-fix:scripts": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint:scripts"
+    "lint-fix:scripts": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint:scripts",
+    "typecheck": "tsc -p ./tsconfig.json"
   },
   "browserslist": [],
   "peerDependencies": {

diff --git a/extensions/src/platform-scripture/src/types/platform-scripture.d.ts b/extensions/src/platform-scripture/src/types/platform-scripture.d.ts
@@ -6,6 +6,7 @@ declare module 'platform-scripture' {
     DataProviderUpdateInstructions,
     ExtensionDataScope,
     IDataProvider,
+    // @ts-ignore: TS2307 - Cannot find module '@papi/core' or its corresponding type declarations
   } from '@papi/core';
   import type { IProjectDataProvider } from 'papi-shared-types';
   import { Dispose, LocalizeKey, UnsubscriberAsync } from 'platform-bible-utils';

diff --git a/extensions/src/project-notes-data-provider/index.d.ts b/extensions/src/project-notes-data-provider/index.d.ts
@@ -3,6 +3,7 @@ import type {
   DataProviderDataType,
   DataProviderSubscriberOptions,
   IDataProvider,
+  // @ts-ignore: TS2307 - Cannot find module '@papi/core' or its corresponding type declarations
 } from '@papi/core';
 import { PlatformEvent, Unsubscriber } from 'platform-bible-utils';
 

diff --git a/extensions/src/quick-verse/package.json b/extensions/src/quick-verse/package.json
@@ -24,7 +24,8 @@
     "lint:scripts": "eslint --ext .cjs,.js,.jsx,.ts,.tsx --cache .",
     "lint:styles": "stylelint **/*.{css,scss}",
     "lint-fix": "npm run lint-fix:scripts && npm run lint:styles -- --fix",
-    "lint-fix:scripts": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint:scripts"
+    "lint-fix:scripts": "prettier --write \"**/*.{ts,tsx,js,jsx,cjs}\" && npm run lint:scripts",
+    "typecheck": "tsc -p ./tsconfig.json"
   },
   "browserslist": [],
   "peerDependencies": {

diff --git a/extensions/src/quick-verse/src/types/quick-verse.d.ts b/extensions/src/quick-verse/src/types/quick-verse.d.ts
@@ -1,4 +1,5 @@
 declare module 'quick-verse' {
+  // @ts-ignore: TS2307 - Cannot find module '@papi/core' or its corresponding type declarations
   import { DataProviderDataType, IDataProvider } from '@papi/core';
 
   export type QuickVerseSetData = string | { text: string; isHeresy: boolean };

diff --git a/extensions/tsconfig.lint.json b/extensions/tsconfig.lint.json
@@ -1,7 +1,4 @@
 {
   "extends": "./tsconfig",
-  "compilerOptions": {
-    "skipLibCheck": true
-  },
   "include": [".eslintrc.cjs", "*.cjs", "*.js", "*.ts", "lib", "src", "webpack"]
 }