From bcaaec6766928e362ef23f6b746789f33e1e6db7 Mon Sep 17 00:00:00 2001 From: James Anderson Date: Sun, 6 Oct 2024 13:35:33 +0100 Subject: [PATCH] chore: improve tsconfig strictness and ci workflows (#62) --- .github/workflows/checks.yml | 33 ++++++++++++++----- .github/workflows/playwright.yml | 31 ++++++++++++----- package.json | 4 ++- packages/cloudflare/env.d.ts | 12 +++++++ packages/cloudflare/package.json | 2 ++ .../src/api/get-cloudflare-context.ts | 2 +- .../get-chunk-installation-identifiers.ts | 2 +- .../patches/to-investigate/wrangler-deps.ts | 22 +++++++------ .../cloudflare/src/cli/templates/worker.ts | 17 +++++----- packages/cloudflare/tsconfig.json | 16 ++++++--- pnpm-lock.yaml | 18 ++++++++-- pnpm-workspace.yaml | 1 + 12 files changed, 114 insertions(+), 46 deletions(-) create mode 100644 packages/cloudflare/env.d.ts diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 46c91431..cec7f94d 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -1,19 +1,36 @@ name: Code checks + on: push: branches: [main] pull_request: branches: [main] + jobs: - test: - timeout-minutes: 60 + checks: + name: ${{ matrix.script }} runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + script: ["prettier:check", "lint:check", "ts:check", "test"] steps: - - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 + - name: Check out code + uses: actions/checkout@v4 + + - name: Setup pnpm + uses: pnpm/action-setup@v4 with: - node-version: lts/* + version: 9 + + - name: Setup Node.js environment + uses: actions/setup-node@v4 + with: + node-version: 20 + cache: "pnpm" + - name: Install dependencies - run: npm install -g pnpm && pnpm install - - name: Run code checks - run: pnpm code:checks + run: pnpm install --frozen-lockfile + + - name: ${{ matrix.script }} + run: pnpm run ${{ matrix.script }} diff --git a/.github/workflows/playwright.yml b/.github/workflows/playwright.yml index 473f7011..46e2396a 100644 --- a/.github/workflows/playwright.yml +++ b/.github/workflows/playwright.yml @@ -1,25 +1,40 @@ name: Playwright Tests + on: push: branches: [main] pull_request: branches: [main] + jobs: test: - timeout-minutes: 60 + timeout-minutes: 30 runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 + - name: Check out code + uses: actions/checkout@v4 + + - name: Setup pnpm + uses: pnpm/action-setup@v4 with: - node-version: lts/* + version: 9 + + - name: Setup Node.js environment + uses: actions/setup-node@v4 + with: + node-version: 20 + cache: "pnpm" + - name: Install dependencies - run: npm install -g pnpm && pnpm install - - name: build all workers + run: | + pnpm install --frozen-lockfile + pnpm run install-playwright + + - name: Build all workers run: pnpm -r build:worker - - name: Install Playwright browsers - run: pnpm run install-playwright + - name: Run playwright tests run: pnpm e2e + - name: Run playwright dev tests run: pnpm e2e:dev diff --git a/package.json b/package.json index 4e7b1417..c5198d90 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,9 @@ "prettier:fix": "prettier --write .", "lint:check": "pnpm -r lint:check", "lint:fix": "pnpm -r lint:fix", - "code:checks": "pnpm lint:check && pnpm lint:check", + "ts:check": "pnpm -r ts:check", + "test": "pnpm -r test", + "code:checks": "pnpm prettier:check && pnpm lint:check && pnpm ts:check", "code:fixes": "pnpm prettier:fix && pnpm lint:fix", "postinstall": "pnpm --filter cloudflare build", "install-playwright": "playwright install --with-deps", diff --git a/packages/cloudflare/env.d.ts b/packages/cloudflare/env.d.ts new file mode 100644 index 00000000..6f9b9d17 --- /dev/null +++ b/packages/cloudflare/env.d.ts @@ -0,0 +1,12 @@ +declare global { + namespace NodeJS { + interface ProcessEnv { + ASSETS: Fetcher; + __NEXT_PRIVATE_STANDALONE_CONFIG?: string; + SKIP_NEXT_APP_BUILD?: string; + [key: string]: string | Fetcher; + } + } +} + +export {}; diff --git a/packages/cloudflare/package.json b/packages/cloudflare/package.json index 5cc2245d..a21d2f98 100644 --- a/packages/cloudflare/package.json +++ b/packages/cloudflare/package.json @@ -7,6 +7,7 @@ "build:watch": "tsup --watch src", "lint:check": "eslint", "lint:fix": "eslint --fix", + "ts:check": "tsc --noEmit", "test": "vitest --run", "test:watch": "vitest" }, @@ -41,6 +42,7 @@ "devDependencies": { "@cloudflare/workers-types": "catalog:", "@eslint/js": "catalog:", + "@tsconfig/strictest": "catalog:", "@types/node": "catalog:", "esbuild": "catalog:", "eslint": "catalog:", diff --git a/packages/cloudflare/src/api/get-cloudflare-context.ts b/packages/cloudflare/src/api/get-cloudflare-context.ts index 27571dfc..50944afc 100644 --- a/packages/cloudflare/src/api/get-cloudflare-context.ts +++ b/packages/cloudflare/src/api/get-cloudflare-context.ts @@ -16,7 +16,7 @@ export type CloudflareContext< /** * the request's [cf properties](https://developers.cloudflare.com/workers/runtime-apis/request/#the-cf-property-requestinitcfproperties) */ - cf: CfProperties; + cf: CfProperties | undefined; /** * the current [execution context](https://developers.cloudflare.com/workers/runtime-apis/context) */ diff --git a/packages/cloudflare/src/cli/build/patches/investigated/update-webpack-chunks-file/get-chunk-installation-identifiers.ts b/packages/cloudflare/src/cli/build/patches/investigated/update-webpack-chunks-file/get-chunk-installation-identifiers.ts index f82655e3..d7233066 100644 --- a/packages/cloudflare/src/cli/build/patches/investigated/update-webpack-chunks-file/get-chunk-installation-identifiers.ts +++ b/packages/cloudflare/src/cli/build/patches/investigated/update-webpack-chunks-file/get-chunk-installation-identifiers.ts @@ -55,7 +55,7 @@ function getInstallChunkDeclaration(sourceFile: ts.SourceFile): ts.VariableDecla // the function we're looking for accesses its parameter three times, and it // accesses its `modules`, `ids` and `runtime` properties (in this order) - const parameterName = functionParameters[0].getText(); + const parameterName = functionParameters[0]!.getText(); const functionParameterAccessedProperties = arrowFunctionBodyBlock .getDescendantsOfKind(ts.SyntaxKind.PropertyAccessExpression) .filter( diff --git a/packages/cloudflare/src/cli/build/patches/to-investigate/wrangler-deps.ts b/packages/cloudflare/src/cli/build/patches/to-investigate/wrangler-deps.ts index 09a5a5e6..3cf82de0 100644 --- a/packages/cloudflare/src/cli/build/patches/to-investigate/wrangler-deps.ts +++ b/packages/cloudflare/src/cli/build/patches/to-investigate/wrangler-deps.ts @@ -1,4 +1,4 @@ -import fs, { writeFileSync } from "node:fs"; +import { readFileSync, statSync, writeFileSync } from "node:fs"; import { Config } from "../../../config"; import path from "node:path"; @@ -15,11 +15,12 @@ export function patchWranglerDeps(config: Config) { // "critters" = "./.next/standalone/node_modules/cf/templates/shims/empty.ts" const pagesRuntimeFile = path.join(distPath, "compiled", "next-server", "pages.runtime.prod.js"); - const patchedPagesRuntime = fs - .readFileSync(pagesRuntimeFile, "utf-8") - .replace(`e.exports=require("critters")`, `e.exports={}`); + const patchedPagesRuntime = readFileSync(pagesRuntimeFile, "utf-8").replace( + `e.exports=require("critters")`, + `e.exports={}` + ); - fs.writeFileSync(pagesRuntimeFile, patchedPagesRuntime); + writeFileSync(pagesRuntimeFile, patchedPagesRuntime); // Patch .next/standalone/node_modules/next/dist/server/lib/trace/tracer.js // @@ -33,11 +34,12 @@ export function patchWranglerDeps(config: Config) { // #"@opentelemetry/api" = "./.next/standalone/node_modules/cf/templates/shims/throw.ts" const tracerFile = path.join(distPath, "server", "lib", "trace", "tracer.js"); - const pacthedTracer = fs - .readFileSync(tracerFile, "utf-8") - .replaceAll(/\w+\s*=\s*require\([^/]*opentelemetry.*\)/g, `throw new Error("@opentelemetry/api")`); + const patchedTracer = readFileSync(tracerFile, "utf-8").replaceAll( + /\w+\s*=\s*require\([^/]*opentelemetry.*\)/g, + `throw new Error("@opentelemetry/api")` + ); - writeFileSync(tracerFile, pacthedTracer); + writeFileSync(tracerFile, patchedTracer); } /** @@ -56,7 +58,7 @@ function getDistPath(config: Config): string { for (const root of [config.paths.standaloneApp, config.paths.standaloneRoot]) { try { const distPath = path.join(root, "node_modules", "next", "dist"); - if (fs.statSync(distPath).isDirectory()) return distPath; + if (statSync(distPath).isDirectory()) return distPath; } catch { /* empty */ } diff --git a/packages/cloudflare/src/cli/templates/worker.ts b/packages/cloudflare/src/cli/templates/worker.ts index 35f51b9d..e76547fe 100644 --- a/packages/cloudflare/src/cli/templates/worker.ts +++ b/packages/cloudflare/src/cli/templates/worker.ts @@ -1,10 +1,11 @@ +import type { ExportedHandler, Fetcher } from "@cloudflare/workers-types"; import { NodeNextRequest, NodeNextResponse } from "next/dist/server/base-http/node"; import { AsyncLocalStorage } from "node:async_hooks"; -import { type CloudflareContext } from "../../api"; +import type { CloudflareContext } from "../../api"; import type { IncomingMessage } from "node:http"; import { MockedResponse } from "next/dist/server/lib/mock-request"; import type { NextConfig } from "next"; -import { type NodeRequestHandler } from "next/dist/server/next-server"; +import type { NodeRequestHandler } from "next/dist/server/next-server"; import Stream from "node:stream"; const NON_BODY_RESPONSES = new Set([101, 204, 205, 304]); @@ -30,8 +31,7 @@ const nextConfig: NextConfig = JSON.parse(process.env.__NEXT_PRIVATE_STANDALONE_ let requestHandler: NodeRequestHandler | null = null; export default { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - async fetch(request: Request & { cf: IncomingRequestCfProperties }, env: any, ctx: any) { + async fetch(request, env, ctx) { return cloudflareContextALS.run({ env, ctx, cf: request.cf }, async () => { if (requestHandler == null) { globalThis.process.env = { ...globalThis.process.env, ...env }; @@ -42,7 +42,7 @@ export default { .default as typeof import("next/dist/server/next-server").default; requestHandler = new NextNodeServer({ - conf: { ...nextConfig, env }, + conf: nextConfig, customServer: false, dev: false, dir: "", @@ -58,18 +58,17 @@ export default { if (imageUrl.startsWith("/")) { return env.ASSETS.fetch(new URL(imageUrl, request.url)); } - // @ts-ignore - return fetch(imageUrl, { cf: { cacheEverything: true } } as unknown); + return fetch(imageUrl, { cf: { cacheEverything: true } }); } const { req, res, webResponse } = getWrappedStreams(request, ctx); - ctx.waitUntil(requestHandler!(new NodeNextRequest(req), new NodeNextResponse(res))); + ctx.waitUntil(Promise.resolve(requestHandler(new NodeNextRequest(req), new NodeNextResponse(res)))); return await webResponse(); }); }, -}; +} as ExportedHandler<{ ASSETS: Fetcher }>; function getWrappedStreams(request: Request, ctx: ExecutionContext) { const url = new URL(request.url); diff --git a/packages/cloudflare/tsconfig.json b/packages/cloudflare/tsconfig.json index c2a686ab..b28f0e5f 100644 --- a/packages/cloudflare/tsconfig.json +++ b/packages/cloudflare/tsconfig.json @@ -1,12 +1,18 @@ { + "$schema": "https://json.schemastore.org/tsconfig", + "extends": "@tsconfig/strictest/tsconfig.json", "compilerOptions": { "target": "ESNext", "module": "ESNext", + "lib": ["ESNext"], "moduleResolution": "Bundler", - "esModuleInterop": true, + + "types": ["@cloudflare/workers-types"], + "forceConsistentCasingInFileNames": true, - "strict": true, - "skipLibCheck": true, - "types": ["@cloudflare/workers-types"] - } + "noImplicitReturns": false, + "exactOptionalPropertyTypes": false + }, + "include": ["**/*.ts"], + "exclude": ["dist"] } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7fbe3a8c..1ce7ac6a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -15,6 +15,9 @@ catalogs: '@playwright/test': specifier: 1.47.0 version: 1.47.0 + '@tsconfig/strictest': + specifier: ^2.0.5 + version: 2.0.5 '@types/node': specifier: ^22.2.0 version: 22.2.0 @@ -237,6 +240,9 @@ importers: '@eslint/js': specifier: 'catalog:' version: 9.11.1 + '@tsconfig/strictest': + specifier: 'catalog:' + version: 2.0.5 '@types/node': specifier: 'catalog:' version: 22.2.0 @@ -1340,6 +1346,9 @@ packages: '@ts-morph/common@0.24.0': resolution: {integrity: sha512-c1xMmNHWpNselmpIqursHeOHHBTIsJLbB+NuovbTTRCNiTLEr/U9dbJ8qy0jd/O2x5pc3seWuOUN5R2IoOTp8A==} + '@tsconfig/strictest@2.0.5': + resolution: {integrity: sha512-ec4tjL2Rr0pkZ5hww65c+EEPYwxOi4Ryv+0MtjeaSQRJyq322Q27eOQiFbuNgw2hpL4hB1/W/HBGk3VKS43osg==} + '@types/estree@1.0.5': resolution: {integrity: sha512-/kYRxGDLWzHOB7q+wtSUQlFrtcdUccpfy+X+9iMBpHK8QLLhx2wIPYuS5DYtR9Wa/YlZAbIovy7qVdB1Aq6Lyw==} @@ -2004,6 +2013,7 @@ packages: eslint@8.57.1: resolution: {integrity: sha512-ypowyDxpVSYpkXr9WPv2PAZCtNip1Mv5KTW0SCurXv/9iOpcrH9PaqUElksqEB6pChqHGDRCFTyrZlGhnLNGiA==} engines: {node: ^12.22.0 || ^14.17.0 || >=16.0.0} + deprecated: This version is no longer supported. Please see https://eslint.org/version-support for other options. hasBin: true eslint@9.11.1: @@ -4500,6 +4510,8 @@ snapshots: mkdirp: 3.0.1 path-browserify: 1.0.1 + '@tsconfig/strictest@2.0.5': {} + '@types/estree@1.0.5': {} '@types/estree@1.0.6': {} @@ -5337,7 +5349,7 @@ snapshots: debug: 4.3.6 enhanced-resolve: 5.17.1 eslint: 8.57.1 - eslint-module-utils: 2.11.0(@typescript-eslint/parser@8.7.0(eslint@8.57.1)(typescript@5.5.4))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.6.3(@typescript-eslint/parser@8.7.0(eslint@8.57.1)(typescript@5.5.4))(eslint-import-resolver-node@0.3.9)(eslint-plugin-import@2.30.0)(eslint@8.57.1))(eslint@8.57.1) + eslint-module-utils: 2.11.0(@typescript-eslint/parser@8.7.0(eslint@8.57.1)(typescript@5.5.4))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.6.3)(eslint@8.57.1) fast-glob: 3.3.2 get-tsconfig: 4.8.0 is-bun-module: 1.2.1 @@ -5350,7 +5362,7 @@ snapshots: - eslint-import-resolver-webpack - supports-color - eslint-module-utils@2.11.0(@typescript-eslint/parser@8.7.0(eslint@8.57.1)(typescript@5.5.4))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.6.3(@typescript-eslint/parser@8.7.0(eslint@8.57.1)(typescript@5.5.4))(eslint-import-resolver-node@0.3.9)(eslint-plugin-import@2.30.0)(eslint@8.57.1))(eslint@8.57.1): + eslint-module-utils@2.11.0(@typescript-eslint/parser@8.7.0(eslint@8.57.1)(typescript@5.5.4))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.6.3)(eslint@8.57.1): dependencies: debug: 3.2.7 optionalDependencies: @@ -5372,7 +5384,7 @@ snapshots: doctrine: 2.1.0 eslint: 8.57.1 eslint-import-resolver-node: 0.3.9 - eslint-module-utils: 2.11.0(@typescript-eslint/parser@8.7.0(eslint@8.57.1)(typescript@5.5.4))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.6.3(@typescript-eslint/parser@8.7.0(eslint@8.57.1)(typescript@5.5.4))(eslint-import-resolver-node@0.3.9)(eslint-plugin-import@2.30.0)(eslint@8.57.1))(eslint@8.57.1) + eslint-module-utils: 2.11.0(@typescript-eslint/parser@8.7.0(eslint@8.57.1)(typescript@5.5.4))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.6.3)(eslint@8.57.1) hasown: 2.0.2 is-core-module: 2.15.1 is-glob: 4.0.3 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index b3322f29..c6e4e8d9 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -6,6 +6,7 @@ catalog: "@cloudflare/workers-types": ^4.20240925.0 "@eslint/js": ^9.11.1 "@playwright/test": 1.47.0 + "@tsconfig/strictest": "^2.0.5" "@types/node": ^22.2.0 "@types/react": ^18 "@types/react-dom": ^18