diff --git a/.github/dependabot.yml b/.github/dependabot.yml index cfa5e8f12..b8ef1b1c1 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -23,6 +23,10 @@ updates: interval: weekly day: monday open-pull-requests-limit: 10 + ignore: + # patches/svelte-check@4.7.6.patch pins this version, and pnpm refuses to + # install with an unused patch. Upgrade it by hand and delete the patch. + - dependency-name: svelte-check groups: dev-dependencies: dependency-type: development diff --git a/AGENTS.md b/AGENTS.md index 6dd8d240b..39f4ad944 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -23,8 +23,9 @@ QuickAdd is an Obsidian community plugin. Source code lives in `src/`: core logi ## Build, Test, and Development Commands - `pnpm run dev`: watch-mode bundle via `esbuild.config.mjs`, regenerating `main.js` as you edit. - `pnpm run build`: run `tsc --noEmit` then produce the production bundle. -- `pnpm run build-with-lint`: type-check, run Oxlint and Svelte policy checks (`pnpm lint`), then produce the production build; use for release packaging. -- `pnpm run lint`: run Oxlint, Svelte template policies, and strict Svelte compiler/type checks. +- `pnpm run build-with-lint`: run `pnpm lint`, then produce the production build; use for release packaging. +- `pnpm run lint`: run Oxlint, Svelte template policies, a tsconfig check, and `pnpm run check` in parallel. The first failure stops the others. Never add `--no-bail`, because pnpm 10 then exits 0 when a script fails. +- `pnpm run check`: strict Svelte compiler checks plus native TypeScript checks of every `.ts` and `.svelte` file. It passes when the tsconfig matches no files, which `lint` catches. - `pnpm run test`: execute Vitest with `--passWithNoTests` for fast local verification. ## Coding Style & Naming Conventions diff --git a/package.json b/package.json index 943868c8e..00ccba0f3 100644 --- a/package.json +++ b/package.json @@ -5,10 +5,10 @@ "main": "main.js", "scripts": { "dev": "node esbuild.config.mjs", - "lint": "pnpm run lint:native && pnpm run lint:svelte && pnpm run check", + "lint": "pnpm run \"/^(lint:.+|check)$/\"", "build": "tsc -noEmit -skipLibCheck && node esbuild.config.mjs production", - "build-with-lint": "tsc -noEmit -skipLibCheck && pnpm lint && node esbuild.config.mjs production", - "check": "svelte-check --fail-on-warnings", + "build-with-lint": "pnpm lint && node esbuild.config.mjs production", + "check": "svelte-check --tsgo-experimental-api --fail-on-warnings", "packages:build": "node docs/packages/build.mjs", "obsidian:e2e": "obsidian-e2e run", "provision:e2e-vault": "obsidian-e2e provision", @@ -19,7 +19,8 @@ "test:e2e": "vitest run --config vitest.e2e.config.mts", "test:e2e:orb": ".agents/run-e2e", "lint:native": "oxlint -c .oxlintrc.syntax.json src tests && oxlint --type-aware --ignore-pattern '**/*.test.ts' src", - "lint:svelte": "node tooling/svelte-policy.mjs" + "lint:svelte": "node tooling/svelte-policy.mjs", + "lint:tsconfig": "tsc --noEmit --noCheck" }, "keywords": [], "author": "Christian B. B. Houmann", @@ -41,7 +42,7 @@ "oxlint": "1.87.0", "oxlint-tsgolint": "7.0.2004", "svelte": "^5.57.2", - "svelte-check": "^4.7.6", + "svelte-check": "4.7.6", "tslib": "^2.8.1", "typescript": "npm:@typescript/typescript6@6.0.2", "vite": "^8.3.4", @@ -70,6 +71,9 @@ "overrides": { "brace-expansion": "5.0.9", "postcss": "8.5.25" + }, + "patchedDependencies": { + "svelte-check@4.7.6": "patches/svelte-check@4.7.6.patch" } } } diff --git a/patches/svelte-check@4.7.6.patch b/patches/svelte-check@4.7.6.patch new file mode 100644 index 000000000..9f34c34c4 --- /dev/null +++ b/patches/svelte-check@4.7.6.patch @@ -0,0 +1,41 @@ +diff --git a/dist/src/index.js b/dist/src/index.js +index 8eec91714c82eaba243e87e3a7b90863c8b7710d..e875707d66fbf26e3d3fcc9834fc8683cebe2419 100644 +--- a/dist/src/index.js ++++ b/dist/src/index.js +@@ -129483,10 +129483,26 @@ function runTypeScriptDiagnostics(tsconfigPath, useTsgo, incremental, cwd) { + proc.on('error', (err) => { + reject(err); + }); +- proc.on('close', () => { ++ proc.on('close', (code, signal) => { + const output = `${stdout}\n${stderr}`; + try { +- resolve(parseDiagnostics(output, cwd)); ++ const diagnostics = parseDiagnostics(output, cwd); ++ // A non-zero exit can also mean the compiler reported diagnostics, so only ++ // treat it as an error when nothing was parsed. A signal always is: the ++ // run did not finish, so any diagnostics it printed are partial. A crash ++ // reported as an exit code after a diagnostic was printed (e.g. on Windows, ++ // where every kill is reported that way) still resolves. ++ if (signal !== null || (code !== 0 && diagnostics.length === 0)) { ++ const reason = signal !== null ++ ? `was killed by signal ${signal}` ++ : `exited with code ${code} without a parseable diagnostic`; ++ // Include what the compiler printed. Unparsed output, such as a global ++ // error or an unknown compiler option, is often the only explanation. ++ const detail = stripAnsi(output).trim(); ++ reject(new Error(`The TypeScript compiler process ${reason}.${detail ? `\n${detail.slice(0, 2000)}` : ''}`)); ++ return; ++ } ++ resolve(diagnostics); + } + catch (e) { + reject(e); +@@ -130471,5 +130487,7 @@ parseOptions(async (opts) => { + catch (_err) { + console.error(_err); + console.error('svelte-check failed'); ++ // A failed run must not look like a clean one to CI. ++ exitAfterFlush(1); + } + }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0f17fc4d6..1cec84ef4 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -8,6 +8,11 @@ overrides: brace-expansion: 5.0.9 postcss: 8.5.25 +patchedDependencies: + svelte-check@4.7.6: + hash: 3eddedcd07511c7eb7e4a26f4caeb62396acf984b2300019329f14f3d2c0cf74 + path: patches/svelte-check@4.7.6.patch + importers: .: @@ -77,8 +82,8 @@ importers: specifier: ^5.57.2 version: 5.57.2(@typescript-eslint/types@8.71.1) svelte-check: - specifier: ^4.7.6 - version: 4.7.6(@typescript/typescript6@6.0.2)(picomatch@4.0.7)(svelte@5.57.2(@typescript-eslint/types@8.71.1)) + specifier: 4.7.6 + version: 4.7.6(patch_hash=3eddedcd07511c7eb7e4a26f4caeb62396acf984b2300019329f14f3d2c0cf74)(@typescript/typescript6@6.0.2)(picomatch@4.0.7)(svelte@5.57.2(@typescript-eslint/types@8.71.1)) tslib: specifier: ^2.8.1 version: 2.8.1 @@ -3145,7 +3150,7 @@ snapshots: style-mod@4.1.4: {} - svelte-check@4.7.6(@typescript/typescript6@6.0.2)(picomatch@4.0.7)(svelte@5.57.2(@typescript-eslint/types@8.71.1)): + svelte-check@4.7.6(patch_hash=3eddedcd07511c7eb7e4a26f4caeb62396acf984b2300019329f14f3d2c0cf74)(@typescript/typescript6@6.0.2)(picomatch@4.0.7)(svelte@5.57.2(@typescript-eslint/types@8.71.1)): dependencies: '@jridgewell/trace-mapping': 0.3.31 '@sveltejs/load-config': 0.2.3 diff --git a/tests/tooling/svelteCheck.test.ts b/tests/tooling/svelteCheck.test.ts index 15be3ce5f..b0f5d050a 100644 --- a/tests/tooling/svelteCheck.test.ts +++ b/tests/tooling/svelteCheck.test.ts @@ -1,8 +1,8 @@ import { spawnSync } from "node:child_process"; -import { copyFileSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { chmodSync, copyFileSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import module from "node:module"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { basename, dirname, join } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; import { expect, it } from "vitest"; @@ -17,62 +17,126 @@ function withWorkspace(files: Record, run: (workspace: string) = try { symlinkSync(join(root, "node_modules"), join(workspace, "node_modules"), process.platform === "win32" ? "junction" : "dir"); copyFileSync(join(root, "svelte.config.mjs"), join(workspace, "svelte.config.mjs")); - for (const [name, text] of Object.entries(files)) writeFileSync(join(workspace, name), text); + for (const [name, text] of Object.entries(files)) { + mkdirSync(dirname(join(workspace, name)), { recursive: true }); + writeFileSync(join(workspace, name), text); + } run(workspace); } finally { rmSync(workspace, { recursive: true, force: true }); } } -function expectCheckFails(workspace: string, env: NodeJS.ProcessEnv = process.env): string { - const { scripts }: { scripts: { check: string } } = JSON.parse(readFileSync(join(root, "package.json"), "utf8")); - const [bin, ...args] = scripts.check.split(" "); +function scripts(): Record { + return JSON.parse(readFileSync(join(root, "package.json"), "utf8")).scripts; +} + +function runCheck(workspace: string, env: NodeJS.ProcessEnv = process.env): { status: number | null; output: string; diagnostics: string[] } { + const [bin, ...args] = scripts().check.split(" "); expect(bin).toBe("svelte-check"); const result = spawnSync(process.execPath, [join(root, "node_modules/svelte-check/bin/svelte-check"), ...args, "--output", "machine-verbose"], { cwd: workspace, encoding: "utf8", env, timeout: 30_000 }); expect(result.error).toBeUndefined(); - expect(result.status, result.stdout + result.stderr).toBe(1); - return result.stdout; + const diagnostics = result.stdout.split("\n").flatMap(line => { + const json = /^\d+ (\{.*\})$/.exec(line)?.[1]; + if (!json) return []; + const { filename, code }: { filename: string; code: string | number } = JSON.parse(json); + return [`${basename(filename)} ${code}`]; + }); + return { status: result.status, output: result.stdout + result.stderr, diagnostics }; } -it("fails strict Svelte checking on warnings, invalid handler types, and legacy reactivity", () => { +// Every Node process the check starts loads this preload. It points the native compiler's +// launcher at `packageJson` for its platform package, or makes that package unresolvable. +function nativeCompilerEnv(workspace: string, packageJson: string | null): NodeJS.ProcessEnv { + const hook = join(workspace, `native-compiler-${packageJson ? "fake" : "missing"}.mjs`); + writeFileSync(hook, [ + 'import { registerHooks } from "node:module";', + `const packageJson = ${JSON.stringify(packageJson && pathToFileURL(packageJson).href)};`, + "registerHooks({ resolve: (specifier, context, next) => {", + '\tif (!specifier.startsWith("@typescript/typescript-")) return next(specifier, context);', + "\tif (!packageJson) throw new Error(`Cannot find package ${specifier}`);", + "\treturn { url: packageJson, shortCircuit: true };", + "} });", + ].join("\n")); + const preload = `--import ${pathToFileURL(hook).href}`; + return { ...process.env, NODE_OPTIONS: [process.env.NODE_OPTIONS, preload].filter(Boolean).join(" ") }; +} + +it("fails Svelte checking on compiler warnings", () => { withWorkspace({ "tsconfig.json": tsconfig(["*.svelte"]), "accessibility.svelte": "
{}}>X
", "unused-css.svelte": "

X

", + }, workspace => { + const { status, output, diagnostics } = runCheck(workspace); + expect(status, output).toBe(1); + expect(diagnostics).toEqual(expect.arrayContaining([ + "accessibility.svelte a11y_click_events_have_key_events", + "unused-css.svelte css_unused_selector", + ])); + }); +}, 30_000); + +it("fails Svelte checking on type errors, legacy reactivity, broken components, and missing components", () => { + withWorkspace({ + "tsconfig.json": tsconfig(["*.ts", "*.svelte"]), "handler.svelte": '', "legacy.svelte": "{n}", + "unclosed.svelte": "{#if true}

X

", + "entry.ts": 'export { default as Deleted } from "./Deleted.svelte";', }, workspace => { - const output = expectCheckFails(workspace); - for (const filename of ["accessibility.svelte", "unused-css.svelte", "handler.svelte", "legacy.svelte"]) expect(output).toContain(filename); - expect(output).toContain("a11y_click_events_have_key_events"); - expect(output).toContain("css_unused_selector"); - expect(output).toContain("legacy_reactive_statement_invalid"); - expect(output).toContain("2322"); + const { status, output, diagnostics } = runCheck(workspace); + expect(status, output).toBe(1); + expect(diagnostics).toEqual(expect.arrayContaining([ + "handler.svelte 2322", + "legacy.svelte legacy_reactive_statement_invalid", + "unclosed.svelte block_unclosed", + "entry.ts 2307", + ])); }); }, 30_000); -// module.registerHooks, which the preload below uses, arrived in Node 22.15. -it.skipIf(typeof module.registerHooks !== "function")("fails Svelte checking on a type error when the native TypeScript executable cannot run", () => { +// module.registerHooks, which the preload uses, arrived in Node 22.15. +it.skipIf(typeof module.registerHooks !== "function")("fails Svelte checking when the native TypeScript compiler cannot run or crashes", () => { + const platformPackage = `@typescript/typescript-${process.platform}-${process.arch}`; withWorkspace({ "tsconfig.json": tsconfig(["*.ts"]), "entry.ts": 'export { default as Probe } from "./Probe.svelte";', - "Probe.svelte": '

{n}

', - // The native compiler's launcher resolves its platform package at startup, so this - // reproduces an install without that package in every Node process the check starts. - "no-native-typescript.mjs": [ - 'import { registerHooks } from "node:module";', - 'registerHooks({ resolve: (specifier, context, next) => { if (specifier.startsWith("@typescript/typescript-")) throw new Error(`Cannot find package ${specifier}`); return next(specifier, context); } });', - ].join("\n"), + "Probe.svelte": '

{n}

', }, workspace => { - const preload = `--import ${pathToFileURL(join(workspace, "no-native-typescript.mjs")).href}`; - const env = { ...process.env, NODE_OPTIONS: [process.env.NODE_OPTIONS, preload].filter(Boolean).join(" ") }; + const clean = runCheck(workspace); + expect(clean.status, clean.output).toBe(0); - const nativeTsc = spawnSync(process.execPath, [join(root, "node_modules/@typescript/native/bin/tsc"), "--version"], { cwd: workspace, encoding: "utf8", env, timeout: 30_000 }); - expect(nativeTsc.status).not.toBe(0); - expect(nativeTsc.stderr).toContain("Unable to resolve @typescript/typescript-"); + const missing = runCheck(workspace, nativeCompilerEnv(workspace, null)); + expect(missing.status, missing.output).toBe(1); + expect(missing.output).toContain(`Unable to resolve ${platformPackage}`); - const output = expectCheckFails(workspace, env); - expect(output).toContain('"Probe.svelte"'); - expect(output).toContain('"code":2322'); + // A shell script cannot stand in for tsc.exe. + if (process.platform === "win32") return; + const fake = join(workspace, "fake-native"); + mkdirSync(join(fake, "lib"), { recursive: true }); + writeFileSync(join(fake, "package.json"), JSON.stringify({ name: platformPackage })); + writeFileSync(join(fake, "lib/tsc"), '#!/bin/sh\n: > "$0.started"\nkill -KILL $$\n'); + chmodSync(join(fake, "lib/tsc"), 0o755); + const crashed = runCheck(workspace, nativeCompilerEnv(workspace, join(fake, "package.json"))); + expect(existsSync(join(fake, "lib/tsc.started")), crashed.output).toBe(true); + expect(crashed.status, crashed.output).toBe(1); + expect(crashed.output).not.toContain("COMPLETED"); + }); +}, 60_000); + +// svelte-check's native API path reports a clean run when the tsconfig matches no input files. +it("fails the tsconfig check in lint when the tsconfig matches no input files", () => { + withWorkspace({ + "tsconfig.json": tsconfig(["missing/*.ts"]), + "entry.ts": 'export const n: number = "wrong";', + }, workspace => { + const gate = new RegExp(/"\/(.+)\/"/.exec(scripts().lint)?.[1] ?? "$^"); + expect(["lint:tsconfig", "check"].filter(name => gate.test(name))).toEqual(["lint:tsconfig", "check"]); + const [bin, ...args] = scripts()["lint:tsconfig"].split(" "); + expect(bin).toBe("tsc"); + const result = spawnSync(process.execPath, [join(root, "node_modules/@typescript/native/bin/tsc"), ...args], { cwd: workspace, encoding: "utf8", timeout: 30_000 }); + expect(result.status, result.stdout + result.stderr).not.toBe(0); + expect(result.stdout).toContain("TS18003"); }); }, 30_000); diff --git a/tooling/README.md b/tooling/README.md index 477713121..eec4a5d50 100644 --- a/tooling/README.md +++ b/tooling/README.md @@ -1,9 +1,11 @@ # Tooling checks -`pnpm run lint` runs Oxlint, `svelte-policy.mjs`, and -`svelte-check --fail-on-warnings`. `pnpm run build-with-lint` runs the -same checks before producing the application bundle. CI also runs the policy -tests through `pnpm run test`. +`pnpm run lint` runs Oxlint, `svelte-policy.mjs`, `tsc --noEmit --noCheck`, +and `svelte-check --tsgo-experimental-api --fail-on-warnings` in parallel. +pnpm runs at most four scripts at once, so a fifth `lint:` script would wait +for a free slot. +`pnpm run build-with-lint` runs the same checks before producing the +application bundle. CI also runs the policy tests through `pnpm run test`. Oxlint checks TypeScript and extracted Svelte scripts. Its JavaScript plugin API cannot inspect Svelte template expressions, so `svelte-policy.mjs` uses @@ -13,9 +15,19 @@ matches these selectors without an ESLint runtime. Oxlint's JavaScript plugin API is alpha and does not have a semver guarantee. The dependency is pinned. The selectors execute in JavaScript, while built-in -Oxlint rules execute natively. `tsc` uses the native compiler. svelte-check -uses the TypeScript 6 API package, because svelte-check 4.7.6 exits 0 under -`--tsgo` when the native compiler cannot start. +Oxlint rules execute natively. `tsc` uses the native compiler, and so does +svelte-check through the native compiler's API. svelte-check 4.7.6 exits 0 +when that compiler cannot start, so `patches/svelte-check@4.7.6.patch` +backports the upstream fix (sveltejs/language-tools#3133). pnpm refuses to +install with a patch for a version that is no longer installed, so the +next svelte-check upgrade must delete it, which is why Dependabot skips +svelte-check. svelte-check still loads the TypeScript 6 package. + +svelte-check checks every file in the tsconfig program, but its `FILES` count +on this path covers only components and files with diagnostics. It also stays +silent when the tsconfig matches no input files, so `lint` also runs +`tsc --noEmit --noCheck`. It fails with TS18003 in that case and skips a second +type check. All current components use Svelte 5 runes. Svelte checking and production compilation require runes mode. The policy checker rejects `$:` statements @@ -88,7 +100,10 @@ valid examples, the test-file exclusion, matching diagnostics between Oxlint and the component checker, and template policy failures. Temporary fixtures are private and removed after the test. `tests/tooling/svelteCheck.test.ts` requires `pnpm run check`'s command to fail on compiler warnings, legacy -reactivity, and type errors, including when the native compiler cannot start. +reactivity, type errors, components that do not parse, imports of missing +components, and a native compiler that cannot start or crashes. It also +requires `pnpm run lint` to run `lint:tsconfig`, and that check to fail when +the tsconfig matches no input files. ```sh pnpm exec vitest run tests/tooling/sveltePolicy.test.ts tests/tooling/svelteCheck.test.ts @@ -108,7 +123,5 @@ These refinements follow the tooling migration: - Review the retired CSS checks and decide which merit separate enforcement. - Move checks to upstream Oxlint or Svelte implementations when they cover template expressions, reducing the code maintained here. -- Remove the TypeScript 6 compatibility package when svelte-check runs on the - native compiler's API and fails when that compiler cannot start. -- Measure the final verification pipeline. Earlier evaluation timings predate - the custom-rule port and the removal of ESLint. +- Remove the TypeScript 6 compatibility package when svelte-check no longer + loads the `typescript` package.