Merge branch 'TurboWarp:master' into master

Author: Frogpants

.github/dependabot.yml

@@ -3,9 +3,9 @@ updates:
   - package-ecosystem: "npm"
     directory: "/"
     schedule:
-      interval: "weekly"
-    ignore:
-      # Prettier updates that result in style changes result in cascading format validation
-      # errors until all pull requests get rebased. It's not worth the effort to follow
-      # every update. We'll do this manually when there's some change we want.
-      - dependency-name: prettier
+      interval: "daily"
+    assignees:
+      - "GarboMuffin"
+    allow:
+      # Only try to automatically bump first-party dependencies.
+      - dependency-name: "@turbowarp/*"

.github/workflows/validate.yml

@@ -35,7 +35,7 @@ jobs:
           cache: npm
       - name: Install dependencies
         run: npm ci
-      - name: Validate
+      - name: Lint
         run: npm run lint
 
   format:
@@ -52,5 +52,33 @@ jobs:
           cache: npm
       - name: Install dependencies
         run: npm ci
-      - name: Validate
-        run: npm run check-format
+      - name: Check format
+        shell: bash
+        run: |
+          # Rewrite the prettier output format into the GitHub Actions annotation format
+          # This is a hack, but this is easier than reimplementing the prettier CLI's ignore
+          # rules ourselves.
+          NO_COLOR=1 npm run check-format 2> >(tee >( \
+            grep -v 'Run Prettier with --write to fix' | \
+            sed -e 's/^\[warn] \(.*\)$/::warning file=\1,line=1::File was not formatted with prettier. Usually, commenting !format will fix this./' \
+          ))
+
+  type-warnings:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+        with:
+          persist-credentials: false
+      - name: Install Node.js
+        uses: actions/setup-node@v4
+        with:
+          node-version: 22
+          cache: npm
+      - name: Install dependencies
+        run: npm ci
+      - name: Generate type warnings
+        run: node development/ci-generate-type-warnings.js
+        env:
+          PR_NUMBER: "${{ github.event.number }}"
+          GH_REPO: "${{ github.repository }}"

development/builder.js

@@ -960,12 +960,13 @@ class Builder {
   async validate() {
     const errors = [];
     const build = await this.build();
-    for (const [fileName, file] of Object.entries(build.files)) {
+    for (const [outputFileName, file] of Object.entries(build.files)) {
       try {
         await file.validate();
       } catch (e) {
         errors.push({
-          fileName,
+          outputFileName,
+          inputFilePath: file.sourcePath,
           error: e,
         });
       }

development/ci-generate-type-warnings.js

@@ -0,0 +1,88 @@
+import pathUtil from "node:path";
+import ts from "typescript";
+import { createAnnotation, getChangedFiles, isCI } from "./ci-interop.js";
+
+/**
+ * @fileoverview Generates CI annotations for type warnings in files modified by the
+ * pull request. Standard TypeScript CLI will include all files and its output is
+ * detected as errors, so not usable for us.
+ */
+
+const check = async () => {
+  const rootDir = pathUtil.join(import.meta.dirname, "..");
+
+  const changedFiles = Array.from(await getChangedFiles()).sort();
+  const changedFilesAbsolute = changedFiles.map((f) =>
+    pathUtil.join(rootDir, f)
+  );
+
+  console.log(`${changedFiles.size} changed files:`);
+  console.log(Array.from(changedFiles).sort().join("\n"));
+  console.log("");
+
+  const tsconfigPath = pathUtil.join(rootDir, "tsconfig.json");
+  const commandLine = ts.getParsedCommandLineOfConfigFile(
+    tsconfigPath,
+    {},
+    ts.sys
+  );
+
+  const program = ts.createProgram({
+    rootNames: commandLine.fileNames,
+    options: commandLine.options,
+  });
+  const emitted = program.emit();
+
+  const diagnostics = [
+    ...ts.getPreEmitDiagnostics(program),
+    ...emitted.diagnostics,
+  ];
+
+  let numWarnings = 0;
+
+  for (const diagnostic of diagnostics) {
+    if (!changedFilesAbsolute.includes(diagnostic.file.fileName)) {
+      continue;
+    }
+
+    const startPosition = ts.getLineAndCharacterOfPosition(
+      diagnostic.file,
+      diagnostic.start
+    );
+    const endPosition = ts.getLineAndCharacterOfPosition(
+      diagnostic.file,
+      diagnostic.start
+    );
+    // Won't be the most readable, but it's probably fine.
+    const flattened = ts.flattenDiagnosticMessageText(
+      diagnostic.messageText,
+      " ",
+      0
+    );
+
+    numWarnings++;
+    createAnnotation({
+      type: "warning",
+      file: diagnostic.file.fileName,
+      title: "Type warning - may indicate a bug - ignore if no bug",
+      onlyIfChanged: true,
+      message: flattened,
+      line: startPosition.line + 1,
+      col: startPosition.character + 1,
+      endLine: endPosition.line + 1,
+      endCol: endPosition.character + 1,
+    });
+  }
+
+  console.log(`Warnings in changed files: ${numWarnings}`);
+  console.log(`Warnings in all files: ${diagnostics.length}`);
+};
+
+if (isCI()) {
+  check();
+} else {
+  console.error(
+    "This script is only intended to be used in CI. For development, use normal TypeScript CLI instead."
+  );
+  process.exit(1);
+}

development/ci-interop.js

@@ -0,0 +1,95 @@
+import pathUtil from "node:path";
+
+/**
+ * @fileoverview Various utilities related to integrating with CI.
+ */
+
+// https://github.com/actions/toolkit/blob/main/docs/problem-matchers.md#limitations
+export const MAX_ANNOTATIONS_PER_TYPE_PER_STEP = 10;
+export const MAX_ANNOTATIONS_PER_JOB = 50;
+
+/**
+ * @returns {boolean} true if running in CI (GitHub Actions, etc.)
+ */
+export const isCI = () => !!process.env.CI;
+
+/**
+ * Note: PR may have changed between when this CI job was queued and when this job runs.
+ * So, don't use this for security-critical things where accuracy is a must.
+ * @returns {Promise<Set<string>>} List of paths (relative to root without leading / or ./) changed by this PR.
+ */
+export const getChangedFiles = async () => {
+  if (!isCI()) {
+    return [];
+  }
+
+  const prNumber = +process.env.PR_NUMBER;
+  if (!prNumber) {
+    throw new Error("Missing PR_NUMBER");
+  }
+
+  const repo = process.env.GH_REPO;
+  if (typeof repo !== "string" || !repo.includes("/")) {
+    throw new Error("Missing GH_REPO");
+  }
+
+  const diffResponse = await fetch(
+    `https://patch-diff.githubusercontent.com/raw/${repo}/pull/${prNumber}.diff`
+  );
+  const diffText = await diffResponse.text();
+  const fileMatches = [
+    ...diffText.matchAll(/^(?:---|\+\+\+) [ab]\/(.+)$/gm),
+  ].map((match) => match[1]);
+
+  return new Set(fileMatches);
+};
+
+/**
+ * @typedef Annotation
+ * @property {'notice'|'warning'|'error'} type
+ * @property {string} file Absolute path to file or relative from repository root
+ * @property {string} title
+ * @property {string} message
+ * @property {number} [line] 1-indexed
+ * @property {number} [col] 1-indexed
+ * @property {number} [endLine] 1-indexed
+ * @property {number} [endCol] 1-indexed
+ */
+
+/**
+ * @param {Annotation} annotation
+ */
+export const createAnnotation = (annotation) => {
+  const rootDir = pathUtil.join(import.meta.dirname, "..");
+  const relativeFileName = pathUtil.relative(rootDir, annotation.file);
+
+  // https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-commands
+
+  let output = "";
+  output += `::${annotation.type} `;
+  output += `file=${relativeFileName}`;
+
+  // Documentation says line number is not required, but in practice that gets interpreted as
+  // line 0 and ends up not showing up. So, we'll default to line 1.
+  if (typeof annotation.line === "number") {
+    output += `,line=${annotation.line}`;
+  } else {
+    output += ",line=1";
+  }
+
+  // These are all actually optional.
+  if (typeof annotation.col === "number") {
+    output += `,col=${annotation.col}`;
+  }
+  if (typeof annotation.endLine === "number") {
+    output += `,endLine=${annotation.endLine}`;
+  }
+  if (typeof annotation.endCol === "number") {
+    output += `,endCol=${annotation.endCol}`;
+  }
+
+  output += `,title=${annotation.title}::`;
+  output += annotation.message;
+
+  console.log(output);
+};

development/validate.js

@@ -1,9 +1,13 @@
+import pathUtil from "node:path";
 import Builder from "./builder.js";
+import { createAnnotation, isCI } from "./ci-interop.js";
 import * as Colors from "./colors.js";
 
 const builder = new Builder("production");
 const errors = await builder.validate();
 
+const rootDir = pathUtil.join(import.meta.dirname, "..");
+
 if (errors.length === 0) {
   console.log(
     `${Colors.GREEN}${Colors.BOLD}Validation checks passed.${Colors.RESET}`
@@ -15,10 +19,25 @@ if (errors.length === 0) {
       errors.length === 1 ? "file" : "files"
     } failed validation.${Colors.RESET}`
   );
+
   console.error("");
-  for (const { fileName, error } of errors) {
-    console.error(`${Colors.BOLD}${fileName}${Colors.RESET}: ${error}`);
+
+  for (const { outputFileName, inputFilePath, error } of errors) {
+    const displayName = inputFilePath
+      ? pathUtil.relative(rootDir, inputFilePath)
+      : outputFileName;
+    console.error(`${Colors.BOLD}${displayName}${Colors.RESET}: ${error}`);
+
+    if (isCI() && inputFilePath) {
+      createAnnotation({
+        type: "error",
+        file: inputFilePath,
+        title: "Validation error",
+        message: error,
+      });
+    }
   }
-  console.error(``);
+
+  console.error("");
   process.exit(1);
 }

extensions/Lily/Skins.js

@@ -428,7 +428,7 @@
       const skinId = createdSkins[skinName];
       if (!skins[skinId]) {
         // If the skin doesnt exist in the renderer we should probably just delete it on our end...
-        this.safeDispose(skinId, skinName);
+        this._disposeSafe(skinId, skinName);
         return 0;
       }