Correct flow types for isolateModules (#431)

## Summary: Issue: XXX-XXXX ## Test plan: `yarn flow` `yarn test` Author: somewhatabstract Reviewers: kevinbarabash, somewhatabstract Required Reviewers: Approved By: kevinbarabash Checks: ✅ codecov/project, ✅ Test (macOS-latest, 16.x), ✅ CodeQL, ✅ Lint, flow, and coverage check (ubuntu-latest, 16.x), ✅ Analyze (javascript), ✅ gerald, ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 16.x), ⏭ dependabot Pull Request URL: #431
Khan · Oct 13, 2022 · ddd526c · ddd526c
1 parent 29f1a1a
commit ddd526c
Show file tree

Hide file tree

Showing 8 changed files with 38 additions and 26 deletions.
diff --git a/.changeset/weak-dolls-reflect.md b/.changeset/weak-dolls-reflect.md
@@ -0,0 +1,7 @@
+---
+"@khanacademy/wonder-stuff-core": patch
+"@khanacademy/wonder-stuff-i18n": patch
+"@khanacademy/wonder-stuff-testing": patch
+---
+
+Fix typing of isolateModules
diff --git a/packages/wonder-stuff-core/src/__tests__/errors-from-error.test.js b/packages/wonder-stuff-core/src/__tests__/errors-from-error.test.js
@@ -2,6 +2,8 @@
 import {KindError} from "../kind-error.js";
 import {errorsFromError, Order} from "../errors-from-error.js";
 
+import type {Options as KindErrorOptions} from "../kind-error.js";
+
 describe("#errorsFromError", () => {
     it.each([null, undefined, "NOT_A_GOOD_VALUE", 42])(
         "should throw if the order is invalid",
@@ -49,7 +51,7 @@ describe("#errorsFromError", () => {
         it("should yield errors in the order of the root error down to the lowermost cause", () => {
             // Arrange
             class MyError extends KindError {
-                constructor(message, options) {
+                constructor(message: string, options: KindErrorOptions) {
                     super(message, "CustomKind", {...options});
                 }
             }
@@ -74,7 +76,7 @@ describe("#errorsFromError", () => {
         it("should yield errors in the order of the lowermost causal error up to the root consequence", () => {
             // Arrange
             class MyError extends KindError {
-                constructor(message, options) {
+                constructor(message: string, options: KindErrorOptions) {
                     super(message, "CustomKind", {...options});
                 }
             }

diff --git a/packages/wonder-stuff-core/src/kind-error.js b/packages/wonder-stuff-core/src/kind-error.js
@@ -8,7 +8,7 @@ import type {Metadata} from "./types.js";
 /**
  * Options for constructing a `KindError`.
  */
-type Options = {|
+export type Options = {|
     /**
      * An error responsible for the error being created.
      *

diff --git a/packages/wonder-stuff-i18n/src/plugins/__tests__/i18n-plugin.test.js b/packages/wonder-stuff-i18n/src/plugins/__tests__/i18n-plugin.test.js
@@ -657,7 +657,7 @@ describe("I18nPlugin", () => {
                     },
                 },
             };
-            const getLocalePath = (locale) => `/prod/${locale}`;
+            const getLocalePath = (locale: string) => `/prod/${locale}`;
             const locales = ["es", "pt"];
             const plugin = new I18nPlugin({
                 locales,
@@ -774,7 +774,7 @@ describe("I18nPlugin", () => {
                     },
                 },
             };
-            const getLocalePath = (locale) => `/prod/${locale}`;
+            const getLocalePath = (locale: string) => `/prod/${locale}`;
             const locales = ["es", "pt"];
             const plugin = new I18nPlugin({
                 locales,

diff --git a/packages/wonder-stuff-i18n/src/plugins/i18n-plugin.js b/packages/wonder-stuff-i18n/src/plugins/i18n-plugin.js
@@ -33,11 +33,11 @@ import type {TranslatedLocaleStrings} from "../utils/i18n-utils.js";
 /**
  * Add a new asset to the Webpack compilation assets.
  *
- * @param {Object} assets the compilation assets object from Webpack
+ * @param {Assets} assets the compilation assets object from Webpack
  * @param {string} relFilePath a file path relative to the Webpack output
  *   directory.
  */
-const addAsset = (assets, relFilePath, data) => {
+const addAsset = (assets: Assets, relFilePath: string, data: string) => {
     assets[relFilePath] = {
         source() {
             return data;
@@ -139,12 +139,15 @@ type InternalOptions = {
 type LocaleHashMap = {[oldHash: string]: string};
 type HashMaps = {[locale: string]: LocaleHashMap};
 
+type Asset = {|
+    source: () => string,
+    size: () => number,
+|};
+
 export type Assets = {
-    [assetName: string]: {|
-        source: () => string,
-        size: () => number,
-    |},
+    [assetName: string]: Asset,
 };
+
 export default class I18nPlugin {
     options: InternalOptions;
 

diff --git a/packages/wonder-stuff-i18n/src/utils/__tests__/localize-file.test.js b/packages/wonder-stuff-i18n/src/utils/__tests__/localize-file.test.js
@@ -71,7 +71,7 @@ describe("localizeString", () => {
         const locale = "es";
         const content = "A simple path: /genwebpack/prod/en/foo.js";
         const hashMap = {};
-        const getLocalePath = (locale) => `/prod/${locale}/`;
+        const getLocalePath = (locale: string) => `/prod/${locale}/`;
 
         // Act
         const newContent = localizeString({
@@ -90,7 +90,7 @@ describe("localizeString", () => {
         const locale = "es";
         const content = "/genwebpack/prod/en/a.js /genwebpack/prod/en/b.js";
         const hashMap = {};
-        const getLocalePath = (locale) => `/prod/${locale}/`;
+        const getLocalePath = (locale: string) => `/prod/${locale}/`;
 
         // Act
         const newContent = localizeString({
@@ -111,7 +111,7 @@ describe("localizeString", () => {
         const locale = "es";
         const content = "/genwebpack/prod/pt/a.js /genwebpack/prod/hu/b.js";
         const hashMap = {};
-        const getLocalePath = (locale) => `/prod/${locale}/`;
+        const getLocalePath = (locale: string) => `/prod/${locale}/`;
 
         // Act
         const newContent = localizeString({
@@ -134,7 +134,7 @@ describe("localizeString", () => {
         const hashMap = {
             "3e083afe95f447481cad": "593cff1d8e1e383f2471",
         };
-        const getLocalePath = (locale) => `/prod/${locale}/`;
+        const getLocalePath = (locale: string) => `/prod/${locale}/`;
 
         // Act
         const newContent = localizeString({
@@ -159,7 +159,7 @@ describe("localizeString", () => {
             "3e083afe95f447481cad": "593cff1d8e1e383f2471",
             "29d70669460257a74073": "9e0f5e0d0704dd61ee2f",
         };
-        const getLocalePath = (locale) => `/prod/${locale}/`;
+        const getLocalePath = (locale: string) => `/prod/${locale}/`;
 
         // Act
         const newContent = localizeString({
@@ -184,7 +184,7 @@ describe("localizeString", () => {
             "3e083afe95f447481cad": "593cff1d8e1e383f2471",
             "29d70669460257a74073": "9e0f5e0d0704dd61ee2f",
         };
-        const getLocalePath = (locale) => `/prod/${locale}/`;
+        const getLocalePath = (locale: string) => `/prod/${locale}/`;
 
         // Act
         const newContent = localizeString({
@@ -209,7 +209,7 @@ describe("localizeString", () => {
             "3e083afe95f447481cad": "593cff1d8e1e383f2471",
             "29d70669460257a74073": "9e0f5e0d0704dd61ee2f",
         };
-        const getLocalePath = (locale) => `/prod/${locale}/`;
+        const getLocalePath = (locale: string) => `/prod/${locale}/`;
 
         // Act
         const newContent = localizeString({
@@ -229,7 +229,7 @@ describe("localizeString", () => {
         // Arrange
         const locale = "es";
         const content = "/genwebpack/prod/en/foo.3e083afe95f447481cad.js";
-        const getLocalePath = (locale) => `/prod/${locale}/`;
+        const getLocalePath = (locale: string) => `/prod/${locale}/`;
 
         // Act
         const newContent = localizeString({locale, content, getLocalePath});
@@ -248,7 +248,7 @@ describe("localizeFile", () => {
         const oldHash = "3e083afe95f447481cad";
         const locale = "es";
         const hashMap = {"2e083afe95f447481cad": "4e083afe95f447481cad"};
-        const getLocalePath = (locale) => `/prod/${locale}/`;
+        const getLocalePath = (locale: string) => `/prod/${locale}/`;
 
         // Act
         const results = localizeFile({
@@ -275,7 +275,7 @@ describe("localizeFile", () => {
         const oldHash = "3e083afe95f447481cad";
         const locale = "es";
         const hashMap = {"2e083afe95f447481cad": "4e083afe95f447481cad"};
-        const getLocalePath = (locale) => `/prod/${locale}/`;
+        const getLocalePath = (locale: string) => `/prod/${locale}/`;
 
         // Act
         const result = localizeFile({
@@ -305,7 +305,7 @@ describe("localizeFile", () => {
             "2e083afe95f447481cad": "4e083afe95f447481cad",
             c0ffee: "3e083afe95f447481cad",
         };
-        const getLocalePath = (locale) => `/prod/${locale}/`;
+        const getLocalePath = (locale: string) => `/prod/${locale}/`;
 
         // Act
         const result = localizeFile({
@@ -333,7 +333,7 @@ describe("localizeFile", () => {
         const oldHash = null;
         const locale = "es";
         const hashMap = {"2e083afe95f447481cad": "4e083afe95f447481cad"};
-        const getLocalePath = (locale) => `/prod/${locale}/`;
+        const getLocalePath = (locale: string) => `/prod/${locale}/`;
 
         // Act
         const result = localizeFile({

diff --git a/packages/wonder-stuff-i18n/src/utils/extract-i18n.js b/packages/wonder-stuff-i18n/src/utils/extract-i18n.js
@@ -320,7 +320,7 @@ const stripJSComments = (jsCode: string, keep: ?string = null) =>
  * This helps to clean up the strings somewhat, avoids escaping things that
  * don't need to be escaped.
  */
-const convertEscapes = (textString) =>
+const convertEscapes = (textString: string) =>
     textString
         .replace(/\\n/g, "\n")
         .replace(/\\"/g, '"')

diff --git a/packages/wonder-stuff-testing/src/jest/isolate-modules.js b/packages/wonder-stuff-testing/src/jest/isolate-modules.js
@@ -13,10 +13,10 @@ import {assertJest} from "./internal/assert-jest.js";
  * discourage dynamic `import` use, which doesn't play well with standard
  * jest yet.
  */
-export const isolateModules = <T>(action: () => ?T): ?T => {
+export const isolateModules = <T>(action: () => T): T => {
     assertJest();
 
-    let result: ?T = undefined;
+    let result: T;
     jest.isolateModules(() => {
         result = action();
     });