Upgrade flow to to 0.102.0

This necessitated a number of type fixes: - Upgraded the express flow-typed file to latest - Added manual flow error suppression to where the express flow-typed file is still using a deprecated utility type - Removed type polymorphism support on map.merge (see context here[1]). We weren't using the polymorphism anywhere so I figured it was simplest to just remove it. - Improve typing around jest mocks throughout the codebase. Test plan: `yarn test --full` passes. [1]: https://github.com/flow-typed/flow-typed/issues/2991
2025-01-11 13:14:28 +00:00 · 2019-07-04 18:52:09 +01:00 · 2019-07-04 18:52:09 +01:00 · eadcca8999
commit eadcca8999
parent 230756ffec
15 changed files with 103 additions and 81 deletions
--- a/config/babel.js
+++ b/config/babel.js
@ -51,11 +51,12 @@ const plugins = [
 var env = process.env.BABEL_ENV || process.env.NODE_ENV;
 var backend = process.env.SOURCECRED_BACKEND === "true";
 if (env !== "development" && env !== "test" && env !== "production") {
+  const stringified = env === undefined ? "undefined" : JSON.stringify(env);
  throw new Error(
    "Using `babel-preset-react-app` requires that you specify `NODE_ENV` or " +
      '`BABEL_ENV` environment variables. Valid values are "development", ' +
      '"test", and "production". Instead, received: ' +
-      JSON.stringify(env) +
+      stringified +
      "."
  );
 }
--- a/flow-typed/npm/express_v4.16.x.js
+++ b/flow-typed/npm/express_v4.16.x.js
@ -1,8 +1,5 @@
-// flow-typed signature: cc24a4e737d9dfb8e1381c3bd4ebaa65
-// flow-typed version: d11eab7bb5/express_v4.16.x/flow_>=v0.32.x
-
-import type { Server } from "http";
-import type { Socket } from "net";
+// flow-typed signature: b647ddbcd7635eb058534a738410dbdb
+// flow-typed version: f55cb054df/express_v4.16.x/flow_>=v0.93.x

 declare type express$RouterOptions = {
  caseSensitive?: boolean,
@ -23,7 +20,7 @@ declare class express$Request extends http$IncomingMessage mixins express$Reques
  baseUrl: string;
  body: mixed;
  cookies: { [cookie: string]: string };
-  connection: Socket;
+  connection: net$Socket;
  fresh: boolean;
  hostname: string;
  ip: string;
@ -120,12 +117,16 @@ declare class express$Response extends http$ServerResponse mixins express$Reques
 declare type express$NextFunction = (err?: ?Error | "route") => mixed;
 declare type express$Middleware =
  | ((
+      // Hack -- pending real fix here: https://github.com/flow-typed/flow-typed/pull/3337
+      // $ExpectFlowError
      req: $Subtype<express$Request>,
      res: express$Response,
      next: express$NextFunction
    ) => mixed)
  | ((
      error: Error,
+      // Hack -- pending real fix here: https://github.com/flow-typed/flow-typed/pull/3337
+      // $ExpectFlowError
      req: $Subtype<express$Request>,
      res: express$Response,
      next: express$NextFunction
@ -182,13 +183,15 @@ declare class express$Router extends express$Route {
  ): this;
  use(path: string, router: express$Router): this;
  handle(
-    req: http$IncomingMessage,
+    req: http$IncomingMessage<>,
    res: http$ServerResponse,
    next: express$NextFunction
  ): void;
  param(
    param: string,
    callback: (
+      // Hack -- pending real fix here: https://github.com/flow-typed/flow-typed/pull/3337
+      // $ExpectFlowError
      req: $Subtype<express$Request>,
      res: express$Response,
      next: express$NextFunction,
@ -196,7 +199,7 @@ declare class express$Router extends express$Route {
    ) => mixed
  ): void;
  (
-    req: http$IncomingMessage,
+    req: http$IncomingMessage<>,
    res: http$ServerResponse,
    next?: ?express$NextFunction
  ): void;
@ -219,15 +222,15 @@ declare class express$Application extends express$Router mixins events$EventEmit
    hostname?: string,
    backlog?: number,
    callback?: (err?: ?Error) => mixed
-  ): ?Server;
+  ): ?http$Server;
  listen(
    port: number,
    hostname?: string,
    callback?: (err?: ?Error) => mixed
-  ): ?Server;
-  listen(port: number, callback?: (err?: ?Error) => mixed): ?Server;
-  listen(path: string, callback?: (err?: ?Error) => mixed): ?Server;
-  listen(handle: Object, callback?: (err?: ?Error) => mixed): ?Server;
+  ): ?http$Server;
+  listen(port: number, callback?: (err?: ?Error) => mixed): ?http$Server;
+  listen(path: string, callback?: (err?: ?Error) => mixed): ?http$Server;
+  listen(handle: Object, callback?: (err?: ?Error) => mixed): ?http$Server;
  disable(name: string): void;
  disabled(name: string): boolean;
  enable(name: string): express$Application;
@ -244,13 +247,13 @@ declare class express$Application extends express$Router mixins events$EventEmit
    callback: express$RenderCallback
  ): void;
  handle(
-    req: http$IncomingMessage,
+    req: http$IncomingMessage<>,
    res: http$ServerResponse,
    next?: ?express$NextFunction
  ): void;
  // callable signature is not inherited
  (
-    req: http$IncomingMessage,
+    req: http$IncomingMessage<>,
    res: http$ServerResponse,
    next?: ?express$NextFunction
  ): void;
--- a/package.json
+++ b/package.json
@ -52,7 +52,7 @@
    "eslint-plugin-jsx-a11y": "5.1.1",
    "eslint-plugin-react": "7.4.0",
    "file-loader": "1.1.5",
-    "flow-bin": "^0.86.0",
+    "flow-bin": "^0.102.0",
    "jest": "^24.8.0",
    "jest-fetch-mock": "^1.6.5",
    "prettier": "^1.18.2",
--- a/src/cli/main.test.js
+++ b/src/cli/main.test.js
@ -5,30 +5,32 @@ import sourcecred from "./sourcecred";

 jest.mock("./sourcecred");

+const sourcecredMock: JestMockFn<any, any> = sourcecred;
+jest.spyOn(console, "log").mockImplementation(() => {});
+jest.spyOn(console, "error").mockImplementation(() => {});
+const logMock: JestMockFn<any, void> = console.log;
+const errorMock: JestMockFn<any, void> = console.error;
+
 describe("cli/main", () => {
-  beforeAll(() => {
-    jest.spyOn(console, "log").mockImplementation(() => {});
-    jest.spyOn(console, "error").mockImplementation(() => {});
-  });
  beforeEach(() => {
-    sourcecred.mockReset();
-    jest.spyOn(console, "log").mockClear();
-    jest.spyOn(console, "error").mockClear();
+    sourcecredMock.mockReset();
+    logMock.mockClear();
+    errorMock.mockClear();
  });

  it("forwards the exit code", async () => {
    process.argv = ["node", "sourcecred", "help"];
-    sourcecred.mockResolvedValueOnce(22);
+    sourcecredMock.mockResolvedValueOnce(22);
    await main();
    expect(process.exitCode).toBe(22);
  });

  it("forwards arguments", async () => {
    process.argv = ["node", "sourcecred", "help", "me"];
-    sourcecred.mockResolvedValueOnce(0);
+    sourcecredMock.mockResolvedValueOnce(0);
    await main();
-    expect(sourcecred).toHaveBeenCalledTimes(1);
-    expect(sourcecred).toHaveBeenCalledWith(["help", "me"], {
+    expect(sourcecredMock).toHaveBeenCalledTimes(1);
+    expect(sourcecredMock).toHaveBeenCalledWith(["help", "me"], {
      out: expect.any(Function),
      err: expect.any(Function),
    });
@ -39,21 +41,21 @@ describe("cli/main", () => {
    process.argv = ["node", "sourcecred", "help"];
    jest.spyOn(console, "log").mockImplementation(() => {});
    jest.spyOn(console, "error").mockImplementation(() => {});
-    sourcecred.mockImplementation(async (args, std) => {
+    sourcecredMock.mockImplementation(async (args, std) => {
      std.out("out and away");
      std.err("err, what?");
      return 0;
    });
    await main();
-    expect(console.log.mock.calls).toEqual([["out and away"]]);
-    expect(console.error.mock.calls).toEqual([["err, what?"]]);
+    expect(logMock.mock.calls).toEqual([["out and away"]]);
+    expect(errorMock.mock.calls).toEqual([["err, what?"]]);
    expect(process.exitCode).toBe(0);
  });

  it("captures an error", async () => {
    process.argv = ["node", "sourcecred", "wat"];
    jest.spyOn(console, "error").mockImplementation(() => {});
-    sourcecred.mockImplementationOnce(() => {
+    sourcecredMock.mockImplementationOnce(() => {
      throw new Error("wat");
    });
    await main();
@ -66,7 +68,7 @@ describe("cli/main", () => {
  it("captures a rejection", async () => {
    process.argv = ["node", "sourcecred", "wat"];
    jest.spyOn(console, "error").mockImplementation(() => {});
-    sourcecred.mockRejectedValueOnce("wat?");
+    sourcecredMock.mockRejectedValueOnce("wat?");
    await main();
    expect(console.log).not.toHaveBeenCalled();
    expect(console.error).toHaveBeenCalledWith('"wat?"');
--- a/src/core/graph.test.js
+++ b/src/core/graph.test.js
@ -66,7 +66,6 @@ describe("core/graph", () => {
    function graphRejectsNulls(f) {
      [null, undefined].forEach((bad) => {
        it(`${f.name} errors on ${String(bad)}`, () => {
-          // $ExpectFlowError
          expect(() => f.call(new Graph(), bad)).toThrow(String(bad));
        });
      });
@ -277,7 +276,6 @@ describe("core/graph", () => {
          function rejectsEdgeAddress(f) {
            it(`${f.name} rejects EdgeAddress`, () => {
              const e = EdgeAddress.fromParts(["foo"]);
-              // $ExpectFlowError
              expect(() => f.call(new Graph(), e)).toThrow("got EdgeAddress");
            });
          }
@ -466,7 +464,6 @@ describe("core/graph", () => {
          function rejectsNodeAddress(f) {
            it(`${f.name} rejects NodeAddress`, () => {
              const e = NodeAddress.fromParts(["foo"]);
-              // $ExpectFlowError
              expect(() => f.call(new Graph(), e)).toThrow("got NodeAddress");
            });
          }
--- a/src/explorer/adapters/explorerAdapterSet.test.js
+++ b/src/explorer/adapters/explorerAdapterSet.test.js
@ -97,15 +97,16 @@ describe("explorer/adapters/explorerAdapterSet", () => {
    });
    it("loads a dynamicExplorerAdapterSet", async () => {
      const {x, sas} = example();
-      x.loadingMock = jest.fn().mockResolvedValue();
+      const loadingMock = jest.fn().mockResolvedValue();
+      x.loadingMock = loadingMock;
      expect(x.loadingMock).toHaveBeenCalledTimes(0);
      const assets = new Assets("/my/gateway/");
      const repoId = makeRepoId("foo", "bar");
      const das = await sas.load(assets, repoId);
-      expect(x.loadingMock).toHaveBeenCalledTimes(1);
-      expect(x.loadingMock.mock.calls[0]).toHaveLength(2);
-      expect(x.loadingMock.mock.calls[0][0]).toBe(assets);
-      expect(x.loadingMock.mock.calls[0][1]).toBe(repoId);
+      expect(loadingMock).toHaveBeenCalledTimes(1);
+      expect(loadingMock.mock.calls[0]).toHaveLength(2);
+      expect(loadingMock.mock.calls[0][0]).toBe(assets);
+      expect(loadingMock.mock.calls[0][1]).toBe(repoId);
      expect(das).toEqual(expect.anything());
    });
  });
--- a/src/explorer/pagerankTable/Node.test.js
+++ b/src/explorer/pagerankTable/Node.test.js
@ -75,7 +75,7 @@ describe("explorer/pagerankTable/Node", () => {
  });

  describe("NodeRow", () => {
-    async function setup(props: $Shape<{...NodeRowProps}>) {
+    async function setup(props: $Shape<{...NodeRowProps}> | void) {
      props = props || {};
      let {sharedProps} = await example();
      if (props.sharedProps !== null) {
--- a/src/explorer/state.test.js
+++ b/src/explorer/state.test.js
@ -27,16 +27,15 @@ describe("explorer/state", () => {
    const setState = (appState) => {
      stateContainer.appState = appState;
    };
-    const loadGraphMock: (
-      assets: Assets,
-      adapters: StaticExplorerAdapterSet,
-      repoId: RepoId
-    ) => Promise<GraphWithAdapters> = jest.fn();
-    const pagerankMock: (
-      Graph,
-      EdgeEvaluator,
-      PagerankOptions
-    ) => Promise<PagerankNodeDecomposition> = jest.fn();
+    const loadGraphMock: JestMockFn<
+      [Assets, StaticExplorerAdapterSet, RepoId],
+      Promise<GraphWithAdapters>
+    > = jest.fn();
+
+    const pagerankMock: JestMockFn<
+      [Graph, EdgeEvaluator, PagerankOptions],
+      Promise<PagerankNodeDecomposition>
+    > = jest.fn();
    const stm = new StateTransitionMachine(
      getState,
      setState,
@ -127,7 +126,7 @@ describe("explorer/state", () => {
    it("transitions to READY_TO_RUN_PAGERANK on success", async () => {
      const {getState, stm, loadGraphMock} = example(readyToLoadGraph());
      const gwa = graphWithAdapters();
-      loadGraphMock.mockResolvedValue(gwa);
+      loadGraphMock.mockReturnValue(Promise.resolve(gwa));
      const succeeded = await stm.loadGraph(
        new Assets("/my/gateway/"),
        new StaticExplorerAdapterSet([])
@ -146,7 +145,7 @@ describe("explorer/state", () => {
      const error = new Error("Oh no!");
      // $ExpectFlowError
      console.error = jest.fn();
-      loadGraphMock.mockRejectedValue(error);
+      loadGraphMock.mockReturnValue(Promise.reject(error));
      const succeeded = await stm.loadGraph(
        new Assets("/my/gateway/"),
        new StaticExplorerAdapterSet([])
@ -173,7 +172,7 @@ describe("explorer/state", () => {
      for (const g of goodStates) {
        const {stm, getState, pagerankMock} = example(g);
        const pnd = pagerankNodeDecomposition();
-        pagerankMock.mockResolvedValue(pnd);
+        pagerankMock.mockReturnValue(Promise.resolve(pnd));
        await stm.runPagerank(
          defaultWeights(),
          defaultTypes(),
@ -205,7 +204,7 @@ describe("explorer/state", () => {
      const error = new Error("Oh no!");
      // $ExpectFlowError
      console.error = jest.fn();
-      pagerankMock.mockRejectedValue(error);
+      pagerankMock.mockReturnValue(Promise.reject(error));
      await stm.runPagerank(
        defaultWeights(),
        defaultTypes(),
--- a/src/util/map.js
+++ b/src/util/map.js
@ -131,9 +131,7 @@ export function mapEntries<K, V, InK, InV>(
 * are mutated. In the event that multiple maps have the same key, an
 * error will be thrown.
 */
-export function merge<K, V>(
-  maps: $ReadOnlyArray<Map<$Subtype<K>, $Subtype<V>>>
-): Map<K, V> {
+export function merge<K, V>(maps: $ReadOnlyArray<Map<K, V>>): Map<K, V> {
  const result = new Map();
  let updates = 0;
  for (const map of maps) {
--- a/src/util/map.test.js
+++ b/src/util/map.test.js
@ -293,15 +293,6 @@ describe("util/map", () => {
    it("merge works on empty list", () => {
      expect(MapUtil.merge([])).toEqual(new Map());
    });
-    it("allows upcasting the type parameters", () => {
-      const numberMap: Map<number, number> = new Map().set(1, 2);
-      const stringMap: Map<string, string> = new Map().set("one", "two");
-      type NS = number | string;
-      const _unused_polyMap: Map<NS, NS> = MapUtil.merge([
-        numberMap,
-        stringMap,
-      ]);
-    });
    it("produces expected type errors", () => {
      const numberMap: Map<number, number> = new Map().set(1, 2);
      const stringMap: Map<string, string> = new Map().set("one", "two");
--- a/src/util/null.test.js
+++ b/src/util/null.test.js
@ -73,14 +73,20 @@ describe("util/null", () => {
      expect(fn).not.toHaveBeenCalled();
    }
    it("throws the provided message on `null`", () => {
-      const fn: () => string = jest.fn().mockReturnValueOnce("uh oh");
+      const fn: JestMockFn<
+        $ReadOnlyArray<void>,
+        string
+      > = jest.fn().mockReturnValueOnce("uh oh");
      expect(() => (NullUtil.orThrow((null: ?number), fn): number)).toThrow(
        /^uh oh$/
      );
      expect(fn.mock.calls).toEqual([[]]);
    });
    it("throws a custom error on `undefined`", () => {
-      const fn: () => string = jest.fn().mockReturnValueOnce("oh dear");
+      const fn: JestMockFn<
+        $ReadOnlyArray<void>,
+        string
+      > = jest.fn().mockReturnValueOnce("oh dear");
      expect(
        () => (NullUtil.orThrow((undefined: ?number), fn): number)
      ).toThrow(/^oh dear$/);
--- a/src/webutil/createRelativeHistory.test.js
+++ b/src/webutil/createRelativeHistory.test.js
@ -413,25 +413,39 @@ describe("webutil/createRelativeHistory", () => {
        it("warns on overflow", () => {
          const {relativeHistory} = createFivePageHistory();
          relativeHistory.goBack();
-          expect(console.error).not.toHaveBeenCalled();
+          // Setup by configureEnzyme()
+          const errorMock: JestMockFn<
+            $ReadOnlyArray<void>,
+            void
+          > = (console.error: any);
+          expect(errorMock).not.toHaveBeenCalled();
          relativeHistory.go(2);
-          expect(console.error).toHaveBeenCalledTimes(1);
-          expect(console.error.mock.calls[0][0]).toMatch(
+          expect(errorMock).toHaveBeenCalledTimes(1);
+          expect(errorMock.mock.calls[0][0]).toMatch(
            /Warning:.*there is not enough history/
          );
+          // Reset console.error to a clean mock to satisfy afterEach check from
+          // configureEnzyme()
          // $ExpectFlowError
          console.error = jest.fn();
        });

        it("warns on underflow", () => {
          const {relativeHistory} = createFivePageHistory();
+          // Setup by configureEnzyme()
+          const errorMock: JestMockFn<
+            $ReadOnlyArray<void>,
+            void
+          > = (console.error: any);
          relativeHistory.go(-4);
-          expect(console.error).not.toHaveBeenCalled();
+          expect(errorMock).not.toHaveBeenCalled();
          relativeHistory.go(-2);
-          expect(console.error).toHaveBeenCalledTimes(1);
-          expect(console.error.mock.calls[0][0]).toMatch(
+          expect(errorMock).toHaveBeenCalledTimes(1);
+          expect(errorMock.mock.calls[0][0]).toMatch(
            /Warning:.*there is not enough history/
          );
+          // Reset console.error to a clean mock to satisfy afterEach check from
+          // configureEnzyme()
          // $ExpectFlowError
          console.error = jest.fn();
        });
--- a/src/webutil/memoryLocalStore.js
+++ b/src/webutil/memoryLocalStore.js
@ -20,7 +20,11 @@ export default class MemoryLocalStore implements LocalStore {
  }

  set(key: string, data: mixed): void {
-    this._data.set(key, JSON.stringify(data));
+    const stringified = JSON.stringify(data);
+    if (stringified === undefined) {
+      throw new Error("tried to serialize undefined");
+    }
+    this._data.set(key, stringified);
  }

  del(key: string): void {
--- a/src/webutil/memoryLocalStore.test.js
+++ b/src/webutil/memoryLocalStore.test.js
@ -21,6 +21,12 @@ describe("webutil/memoryLocalStore", () => {
    expect(ls.get("one")).toBe(null);
  });

+  it("throws an error on undefined", () => {
+    const ls = new MemoryLocalStore();
+    const f = () => ls.set("one", undefined);
+    expect(f).toThrowError("undefined");
+  });
+
  it("overwrites values", () => {
    const ls = new MemoryLocalStore();
    ls.set("questions", 5);
--- a/yarn.lock
+++ b/yarn.lock
@ -3871,10 +3871,10 @@ flatten@^1.0.2:
  resolved "https://registry.yarnpkg.com/flatten/-/flatten-1.0.2.tgz#dae46a9d78fbe25292258cc1e780a41d95c03782"
  integrity sha1-2uRqnXj74lKSJYzB54CkHZXAN4I=

-flow-bin@^0.86.0:
-  version "0.86.0"
-  resolved "https://registry.yarnpkg.com/flow-bin/-/flow-bin-0.86.0.tgz#153a28722b4dc13b7200c74b644dd4d9f4969a11"
-  integrity sha512-ulRvFH3ewGIYwg+qPk/OJXoe3Nhqi0RyR0wqgK0b1NzUDEC6O99zU39MBTickXvlrr6iwRO6Wm4lVGeDmnzbew==
+flow-bin@^0.102.0:
+  version "0.102.0"
+  resolved "https://registry.yarnpkg.com/flow-bin/-/flow-bin-0.102.0.tgz#3d5de44bcc26d26585e932b3201988b766f9b380"
+  integrity sha512-mYon6noeLO0Q5SbiWULLQeM1L96iuXnRtYMd47j3bEWXAwUW9EnwNWcn+cZg/jC/Dg4Wj/jnkdTDEuFtbeu1ww==

 flush-write-stream@^1.0.0:
  version "1.0.3"