From 4d35e6c6dee500af5dcb640cd63fd43abc76ad97 Mon Sep 17 00:00:00 2001 From: Philippe Serhal Date: Fri, 31 May 2024 14:27:13 -0400 Subject: [PATCH 1/2] chore: actually typecheck deno files, fix errors We weren't including Deno files in `tsc` typechecking, but we also weren't using any `deno` commands that typecheck. This introduces typechecking in CI and fixes resulting hidden errors. The JSON hack isn't great but really it's the whole "import a json and write new data to it" pattern that should be simplified to "just read the new data in memory", since there's no actual reason to write it to disk if you follow the code paths. It was just done that way because it was the shortest path to add the dynamic fissue annotation from the previous implementation. --- .github/workflows/deno-test.yml | 6 +++++- .github/workflows/lint.yml | 4 ++-- deno.json | 10 ---------- deno.jsonc | 29 +++++++++++++++++++++++++++++ package.json | 9 +++++++-- tests/test-config.json | 4 +++- 6 files changed, 46 insertions(+), 16 deletions(-) delete mode 100644 deno.json create mode 100644 deno.jsonc diff --git a/.github/workflows/deno-test.yml b/.github/workflows/deno-test.yml index ccd44cdf6a..cf826b7d9a 100644 --- a/.github/workflows/deno-test.yml +++ b/.github/workflows/deno-test.yml @@ -12,5 +12,9 @@ jobs: uses: denoland/setup-deno@v1 with: deno-version: vx.x.x + - name: Typecheck + run: npm run typecheck:deno + - name: Lint + run: npm run lint:deno - name: Test - run: deno test -A edge-runtime/ + run: npm run test:deno diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index d6585fc5ae..69757e1e94 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -34,9 +34,9 @@ jobs: - name: Lint # github adds inline annotation for compact or stylish format # which is different than our default for local usage - run: npm run lint -- --format=compact + run: npm run lint:node -- --format=compact - name: Types - run: npm run typecheck + run: npm run typecheck:node # we still want to check types if lint fails just to know everything # and not peel errors to uncover new ones of different type if: always() diff --git a/deno.json b/deno.json deleted file mode 100644 index 28e8ea6c3c..0000000000 --- a/deno.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "lint": { - "files": { - "include": ["edge-runtime/middleware.ts"] - } - }, - "imports": { - "@netlify/edge-functions": "https://edge.netlify.com/v1/index.ts" - } -} diff --git a/deno.jsonc b/deno.jsonc new file mode 100644 index 0000000000..fa3c41de53 --- /dev/null +++ b/deno.jsonc @@ -0,0 +1,29 @@ +{ + "exclude": [ + "edge-runtime/vendor/", + "edge-runtime/shim/" + ], + "lint": { + "include": [ + "tools/deno/", + "edge-runtime/" + ], + "exclude": [ + // TODO(serhalp) Remove this exclusion and fix the lint errors here. + "edge-runtime/lib" + ] + }, + "test": { + "include": [ + "tools/deno/", + "edge-runtime/" + ] + }, + "tasks": { + // TODO(serhalp) Add `edge-runtime/**/*.ts` and add a vendoring step in CI. + "typecheck": "deno check tools/deno/**/*.ts" + }, + "imports": { + "@netlify/edge-functions": "https://edge.netlify.com/v1/index.ts" + } +} diff --git a/package.json b/package.json index fb2b6bca9b..a64d6d9efe 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,9 @@ "pretest:integration": "npm run build && node tests/prepare.mjs", "build": "node ./tools/build.js", "build:watch": "node ./tools/build.js --watch", - "lint": "eslint --cache --format=codeframe --max-warnings=0 --ext .ts,.cts,.js src", + "lint:node": "eslint --cache --format=codeframe --max-warnings=0 --ext .ts,.cts,.js src", + "lint:deno": "deno lint", + "lint": "npm run lint:node && npm run lint:deno", "format:fix": "prettier --write .", "format:check": "prettier --check .", "test": "vitest", @@ -30,7 +32,10 @@ "test:ci:unit-and-integration": "vitest run --reporter=default --retry=3 --project=unit --project=integration", "test:ci:smoke": "vitest run --reporter=default --retry=3 --project=smoke", "test:ci:e2e": "playwright test", - "typecheck": "tsc --noEmit" + "test:deno": "deno test -A", + "typecheck:node": "tsc --noEmit", + "typecheck:deno": "deno task typecheck", + "typecheck": "npm run typecheck:node && npm run typecheck:deno" }, "repository": { "type": "git", diff --git a/tests/test-config.json b/tests/test-config.json index b7a8942293..972854e00e 100644 --- a/tests/test-config.json +++ b/tests/test-config.json @@ -124,7 +124,9 @@ { "file": "test/e2e/app-dir/app-static/app-static.test.ts", "reason": "Uses CLI output", - "tests": ["app-dir static/dynamic handling should warn for too many cache tags"] + "tests": [ + "app-dir static/dynamic handling should warn for too many cache tags" + ] }, { "file": "test/e2e/app-dir/parallel-routes-and-interception/parallel-routes-and-interception.test.ts", From cc18bddf44dc27a6c8f68c89362b576150e883ca Mon Sep 17 00:00:00 2001 From: Philippe Serhal Date: Mon, 3 Jun 2024 11:39:40 -0400 Subject: [PATCH 2/2] chore: vendor deno deps in CI before deno tests When building the package, we only care about vendoring `edge-runtime/` deps, but for Deno tests/typechecking/linting, we want to vendor the deps of our build tools written in Deno too. --- .github/workflows/deno-test.yml | 8 ++++++++ tools/build.js | 8 ++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/.github/workflows/deno-test.yml b/.github/workflows/deno-test.yml index cf826b7d9a..d111c7b79a 100644 --- a/.github/workflows/deno-test.yml +++ b/.github/workflows/deno-test.yml @@ -12,6 +12,14 @@ jobs: uses: denoland/setup-deno@v1 with: deno-version: vx.x.x + - name: Vendor Deno Dependencies + uses: actions/github-script@v7 + with: + script: | + const { vendorDeno } = await import('${{ github.workspace }}/tools/build.js') + + await vendorDeno('edge-runtime') + await vendorDeno('tools/deno') - name: Typecheck run: npm run typecheck:deno - name: Lint diff --git a/tools/build.js b/tools/build.js index 5c8a5d6274..9b23100dca 100644 --- a/tools/build.js +++ b/tools/build.js @@ -77,9 +77,9 @@ async function bundle(entryPoints, format, watch) { }) } -async function vendorDeno() { - const vendorSource = resolve('edge-runtime/vendor.ts') - const vendorDest = resolve('edge-runtime/vendor') +export async function vendorDeno(dir) { + const vendorSource = resolve(join(dir, 'vendor.ts')) + const vendorDest = resolve(join(dir, 'vendor')) try { await execaCommand('deno --version') @@ -100,7 +100,7 @@ const args = new Set(process.argv.slice(2)) const watch = args.has('--watch') || args.has('-w') await Promise.all([ - vendorDeno(), + vendorDeno('edge-runtime'), bundle(entryPointsESM, 'esm', watch), ...entryPointsCJS.map((entry) => bundle([entry], 'cjs', watch)), cp('src/build/templates', join(OUT_DIR, 'build/templates'), { recursive: true, force: true }),