Skip to content

Commit

Permalink
FEI-5001: Switch to TS enums, remove Flow infra (#852)
Browse files Browse the repository at this point in the history
## Summary:
Now that none of the downstream consumers of wonder-stuff are using Flow we can get rid of Flow related things from this repo.

Issue: FEI-5001

## Test plan:
- `yarn build:types`
- `yarn tsc`
- `yarn build:docs`, open docs/index.html, see that `Runtime` is now a proper `enum`

<img width="1904" alt="Screen Shot 2023-07-31 at 5 28 22 PM" src="https://github.com/Khan/wonder-stuff/assets/1044413/85bbb7a0-9bb8-4df8-9abf-c204f22dcdf6">

Author: kevinbarabash

Reviewers: jeresig

Required Reviewers:

Approved By: jeresig

Checks: ✅ Test (macos-latest, 16.x), ✅ codecov/project, ✅ Test (macos-latest, 16.x), ✅ Lint, typecheck, and coverage check (ubuntu-latest, 16.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 16.x), ⏭  gerald, ✅ CodeQL, ✅ Lint, typecheck, and coverage check (ubuntu-latest, 16.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 16.x), ✅ gerald, ✅ Analyze (javascript), ⏭  dependabot

Pull Request URL: #852
  • Loading branch information
kevinbarabash authored Jul 31, 2023
1 parent fa7daca commit 4419e81
Show file tree
Hide file tree
Showing 14 changed files with 57 additions and 160 deletions.
5 changes: 5 additions & 0 deletions .changeset/silver-experts-attend.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/wonder-stuff-server": major
---

Make Runtime a TS enum
5 changes: 5 additions & 0 deletions .changeset/young-cameras-fail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"ws-dev-build-settings": minor
---

Stop building Flow type definitions
3 changes: 0 additions & 3 deletions .github/workflows/node-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,6 @@ jobs:
- name: Build Types
run: yarn build:types

- name: Build Flow Types
run: yarn build:flowtypes

# Linting / type checking
- name: Eslint
uses: Khan/actions@full-or-limited-v0
Expand Down
1 change: 0 additions & 1 deletion build-scripts/flowgen.d.ts

This file was deleted.

70 changes: 0 additions & 70 deletions build-scripts/gen-flow-types.ts

This file was deleted.

21 changes: 0 additions & 21 deletions build-scripts/remove-test-types-from-dist.ts

This file was deleted.

39 changes: 0 additions & 39 deletions build-settings/flowgen.d.ts

This file was deleted.

4 changes: 1 addition & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
"express-async-handler": "^1.2.0",
"express-winston": "^4.2.0",
"fast-glob": "^3.3.0",
"flowgen": "git+https://[email protected]/Khan/flowgen.git#fa8050929948fb4313f738301285fb44625678c4",
"heapdump": "^0.3.15",
"jest": "^29.6.1",
"jest-extended": "^4.0.0",
Expand All @@ -82,8 +81,7 @@
"scripts": {
"build": "rollup -c build-settings/rollup.config.js",
"build:prodsizecheck": "rollup -c build-settings/rollup.config.js --configPlatforms='browser' --configFormats='esm' --configEnvironment='production'",
"build:types": "yarn tsc --build --verbose tsconfig-build.json && node -r @swc-node/register build-scripts/remove-test-types-from-dist.ts",
"build:flowtypes": "node -r @swc-node/register build-scripts/gen-flow-types.ts",
"build:types": "yarn tsc --build --verbose tsconfig-build.json",
"build:docs": "typedoc",
"watch": "rollup -c build-settings/rollup.config.js --watch",
"clean": "rm -rf packages/wonder-stuff-*/dist && rm -rf packages/wonder-stuff-*/node_modules && rm -f packages/*/*.tsbuildinfo",
Expand Down
10 changes: 10 additions & 0 deletions packages/tsconfig-shared.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
// This file is used by the tsconfig.json files in each package.
/* Visit https://aka.ms/tsconfig to read more about this file */
{
"exclude": [
"dist",
"**/*.test.*",
"**/__tests__/**",
"**/*.testdata.*",
"**/__testdata__/**",
"**/*.stories.*",
"**/__stories__/**",
"**/*.cypress.*"
],
"extends": "../tsconfig-common.json",
"compilerOptions": {
"composite": true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,9 @@ describe("#runServer", () => {
const fakeRenderEnvironment: any = {
render: jest.fn(),
};
jest.spyOn(WSServer, "getRuntimeMode").mockReturnValue("test");
jest.spyOn(WSServer, "getRuntimeMode").mockReturnValue(
WSServer.Runtime.Test,
);
jest.spyOn(WSServer, "getLogger").mockReturnValue(pretendLogger);
jest.spyOn(WSServer, "getAppEngineInfo").mockReturnValue({} as any);
const pretendApp = {
Expand Down Expand Up @@ -146,7 +148,9 @@ describe("#runServer", () => {
const fakeRenderEnvironment: any = {
render: jest.fn(),
};
jest.spyOn(WSServer, "getRuntimeMode").mockReturnValue("test");
jest.spyOn(WSServer, "getRuntimeMode").mockReturnValue(
WSServer.Runtime.Test,
);
jest.spyOn(WSServer, "getLogger").mockReturnValue(pretendLogger);
jest.spyOn(WSServer, "getAppEngineInfo").mockReturnValue({} as any);
const pretendApp = {
Expand Down Expand Up @@ -190,7 +194,9 @@ describe("#runServer", () => {
const fakeRenderEnvironment: any = {
render: jest.fn(),
};
jest.spyOn(WSServer, "getRuntimeMode").mockReturnValue("test");
jest.spyOn(WSServer, "getRuntimeMode").mockReturnValue(
WSServer.Runtime.Test,
);
jest.spyOn(WSServer, "getLogger").mockReturnValue(pretendLogger);
jest.spyOn(WSServer, "getAppEngineInfo").mockReturnValue({} as any);
const pretendApp = {
Expand Down Expand Up @@ -225,7 +231,9 @@ describe("#runServer", () => {
it("should start the gateway", async () => {
// Arrange
const fakeRenderEnvironment: any = {render: jest.fn()};
jest.spyOn(WSServer, "getRuntimeMode").mockReturnValue("test");
jest.spyOn(WSServer, "getRuntimeMode").mockReturnValue(
WSServer.Runtime.Test,
);
jest.spyOn(WSServer, "getAppEngineInfo").mockReturnValue({} as any);
const pretendApp = {
use: jest.fn().mockReturnThis(),
Expand Down Expand Up @@ -283,7 +291,9 @@ describe("#runServer", () => {
// Arrange
process.env.KA_ALLOW_HEAPDUMP = "1";
const fakeRenderEnvironment: any = {render: jest.fn()};
jest.spyOn(WSServer, "getRuntimeMode").mockReturnValue("test");
jest.spyOn(WSServer, "getRuntimeMode").mockReturnValue(
WSServer.Runtime.Test,
);
jest.spyOn(WSServer, "getAppEngineInfo").mockReturnValue({} as any);
const pretendApp = {
use: jest.fn().mockReturnThis(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ jest.mock("@khanacademy/wonder-stuff-server");
describe("#startTraceAgent", () => {
it("should start the trace agent as disabled when not in production", () => {
// Arrange
jest.spyOn(WSServer, "getRuntimeMode").mockReturnValue("development");
jest.spyOn(WSServer, "getRuntimeMode").mockReturnValue(
WSServer.Runtime.Development,
);
const startSpy = jest.spyOn(TraceAgent, "start");

// Act
Expand All @@ -20,7 +22,9 @@ describe("#startTraceAgent", () => {

it("should start the trace agent as enabled when in production", () => {
// Arrange
jest.spyOn(WSServer, "getRuntimeMode").mockReturnValue("production");
jest.spyOn(WSServer, "getRuntimeMode").mockReturnValue(
WSServer.Runtime.Production,
);
const startSpy = jest.spyOn(TraceAgent, "start");

// Act
Expand All @@ -33,7 +37,9 @@ describe("#startTraceAgent", () => {
it("should return the tracer", () => {
// Arrange
const pretendTracer = {} as any;
jest.spyOn(WSServer, "getRuntimeMode").mockReturnValue("production");
jest.spyOn(WSServer, "getRuntimeMode").mockReturnValue(
WSServer.Runtime.Production,
);
jest.spyOn(TraceAgent, "start").mockReturnValue(pretendTracer);

// Act
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ import {secret} from "@khanacademy/wonder-stuff-core";
import * as GetRuntimeMode from "../../get-runtime-mode";
import * as GetLogger from "../../get-logger";
import {requestAuthentication} from "../request-authentication";
import {Runtime} from "../../types";

describe("#requestAuthentication", () => {
describe("when not in production", () => {
beforeEach(() => {
jest.spyOn(GetRuntimeMode, "getRuntimeMode").mockReturnValue(
"development",
Runtime.Development,
);
});

Expand Down Expand Up @@ -193,7 +194,7 @@ describe("#requestAuthentication", () => {
describe("when in production", () => {
beforeEach(() => {
jest.spyOn(GetRuntimeMode, "getRuntimeMode").mockReturnValue(
"production",
Runtime.Production,
);
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import express from "express";
import type {Application} from "express";
import * as lw from "@google-cloud/logging-winston";
import type {Logger, RequestAuthentication, RuntimeValue} from "../types";
import {Runtime} from "../types";
import {Logger, RequestAuthentication, Runtime} from "../types";
import {attachAppEngineRequestID} from "./attach-app-engine-request-id";
import {commonServiceRoutes} from "./common-service-routes";
import {defaultErrorLogging} from "./default-error-logging";
Expand All @@ -13,7 +12,7 @@ import {requestAuthentication} from "./request-authentication";
export const wrapWithMiddleware = async (
app: Application,
logger: Logger,
mode: RuntimeValue,
mode: Runtime,
requestAuthOptions?: RequestAuthentication,
): Promise<Application> => {
// Setup the middleware around the app.
Expand Down
17 changes: 7 additions & 10 deletions packages/wonder-stuff-server/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,11 @@ export type Logger = WinstonLogger;
/**
* The runtime modes that a gateway can run under.
*/
// TODO(FEI-5001): Replace with TS enum
export const Runtime = {
Production: "production" as const,
Development: "development" as const,
Test: "test" as const,
} as const;

export type RuntimeValue = (typeof Runtime)[keyof typeof Runtime];
export enum Runtime {
Production = "production",
Development = "development",
Test = "test",
}

/**
* Options to configure logging.
Expand All @@ -40,7 +37,7 @@ export type LoggingOptions = {
/**
* The runtime mode.
*/
mode: RuntimeValue;
mode: Runtime;
/**
* Log only if the level of a logged entry is less than or equal to this
* level. Enables filtering out of debug message in production, for example.
Expand Down Expand Up @@ -100,7 +97,7 @@ export type ServerOptions = {
/**
* What runtime mode the server is running under.
*/
readonly mode: RuntimeValue;
readonly mode: Runtime;
/**
* Optional value in milliseconds for keepalive timeout of the server.
* For running in Google Cloud, this should be higher than the load
Expand Down

0 comments on commit 4419e81

Please sign in to comment.