diff --git a/.changeset/cool-oranges-fail.md b/.changeset/cool-oranges-fail.md new file mode 100644 index 000000000000..7751b6119189 --- /dev/null +++ b/.changeset/cool-oranges-fail.md @@ -0,0 +1,7 @@ +--- +"@cloudflare/vite-plugin": patch +--- + +Append Cloudflare defaults to existing `.assetsignore` files during build output + +When a project includes a `PUBLIC_DIR/.assetsignore`, the plugin now preserves those rules and appends the required `wrangler.json` and `.dev.vars` entries instead of replacing the file content. diff --git a/.changeset/fix-angular-localhost-ssr.md b/.changeset/fix-angular-localhost-ssr.md new file mode 100644 index 000000000000..d46a2d4deb39 --- /dev/null +++ b/.changeset/fix-angular-localhost-ssr.md @@ -0,0 +1,10 @@ +--- +"create-cloudflare": patch +"wrangler": patch +--- + +Fix Angular scaffolding to allow localhost SSR in development mode + +Recent versions of Angular's `AngularAppEngine` block serving SSR on `localhost` by default. This caused `wrangler dev` / `wrangler pages dev` to fail with `URL with hostname "localhost" is not allowed.` + +The fix passes `allowedHosts: ["localhost"]` to the `AngularAppEngine` constructor in `server.ts`, which is safe to do even in production since Cloudflare will already restrict which host is allowed. diff --git a/.changeset/keen-fish-melt.md b/.changeset/keen-fish-melt.md new file mode 100644 index 000000000000..55c6527a9e4c --- /dev/null +++ b/.changeset/keen-fish-melt.md @@ -0,0 +1,10 @@ +--- +"@cloudflare/workers-utils": patch +--- + +Add `removeDir` and `removeDirSync` helpers with automatic retry logic for Windows EBUSY errors + +These new helpers wrap `fs.rm`/`fs.rmSync` with `maxRetries: 5` and `retryDelay: 100` to handle cases where file handles aren't immediately released (common on Windows with workerd). +The async helper also has a `fireAndForget` option to silently swallow errors and not await removal. + +This improves reliability of cleanup operations across the codebase. diff --git a/.changeset/light-clocks-enter.md b/.changeset/light-clocks-enter.md new file mode 100644 index 000000000000..9ebf092ddca6 --- /dev/null +++ b/.changeset/light-clocks-enter.md @@ -0,0 +1,9 @@ +--- +"@cloudflare/local-explorer-ui": minor +--- + +Adds the tab definition for the table explorer. + +This serves as another stepping stone for adding the complete data studio to the local explorer. + +This is a WIP experimental feature. diff --git a/fixtures/additional-modules/package.json b/fixtures/additional-modules/package.json index baebaac68d74..559c828a5881 100644 --- a/fixtures/additional-modules/package.json +++ b/fixtures/additional-modules/package.json @@ -14,6 +14,7 @@ "@cloudflare/eslint-config-shared": "workspace:*", "@cloudflare/workers-tsconfig": "workspace:*", "@cloudflare/workers-types": "catalog:default", + "@fixture/shared": "workspace:*", "typescript": "catalog:default", "undici": "catalog:default", "vitest": "catalog:default", diff --git a/fixtures/additional-modules/test/index.test.ts b/fixtures/additional-modules/test/index.test.ts index 635d9a98b1d4..eb3c969b0914 100644 --- a/fixtures/additional-modules/test/index.test.ts +++ b/fixtures/additional-modules/test/index.test.ts @@ -3,6 +3,7 @@ import { existsSync } from "node:fs"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; +import { removeDir } from "@fixture/shared/src/fs-helpers"; import { afterAll, assert, beforeAll, describe, test, vi } from "vitest"; import { unstable_startWorker } from "wrangler"; import { wranglerEntryPath } from "../../shared/src/run-wrangler-long-lived"; @@ -41,21 +42,7 @@ describe("find_additional_modules dev", () => { }); afterAll(async () => { await worker.dispose(); - try { - await fs.rm(tmpDir, { recursive: true, force: true }); - } catch (e) { - // It seems that Windows doesn't let us delete this, with errors like: - // - // Error: EBUSY: resource busy or locked, rmdir 'C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ' - // ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ - // Serialized Error: { - // "code": "EBUSY", - // "errno": -4082, - // "path": "C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ", - // "syscall": "rmdir", - // } - console.error(e); - } + removeDir(tmpDir, { fireAndForget: true }); }); test("supports bundled modules", async ({ expect }) => { @@ -141,9 +128,7 @@ describe("find_additional_modules deploy", () => { beforeAll(async () => { tmpDir = await getTmpDir(); }); - afterAll(async () => { - await fs.rm(tmpDir, { recursive: true, force: true }); - }); + afterAll(async () => await removeDir(tmpDir, { fireAndForget: true })); test("doesn't bundle additional modules", async ({ expect }) => { const outDir = path.join(tmpDir, "out"); diff --git a/fixtures/interactive-dev-tests/tests/index.test.ts b/fixtures/interactive-dev-tests/tests/index.test.ts index 3e55066b5d25..9cbaf0885909 100644 --- a/fixtures/interactive-dev-tests/tests/index.test.ts +++ b/fixtures/interactive-dev-tests/tests/index.test.ts @@ -6,6 +6,7 @@ import rl from "node:readline"; import stream from "node:stream"; import { setTimeout } from "node:timers/promises"; import { stripVTControlCharacters } from "node:util"; +import { removeDir } from "@fixture/shared/src/fs-helpers"; import { fetch } from "undici"; /* eslint-disable workers-sdk/no-vitest-import-expect -- complex test with .each patterns */ import { @@ -380,22 +381,8 @@ if (process.platform === "win32") { }); } }); - afterAll(async () => { - try { - fs.rmSync(tmpDir, { recursive: true, force: true }); - } catch (e) { - // It seems that Windows doesn't let us delete this, with errors like: - // - // Error: EBUSY: resource busy or locked, rmdir 'C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ' - // ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ - // Serialized Error: { - // "code": "EBUSY", - // "errno": -4082, - // "path": "C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ", - // "syscall": "rmdir", - // } - console.error(e); - } + afterAll(() => { + removeDir(tmpDir, { fireAndForget: true }); }); it("should print rebuild containers hotkey", async () => { @@ -560,22 +547,8 @@ if (process.platform === "win32") { } }); - afterAll(async () => { - try { - fs.rmSync(tmpDir, { recursive: true, force: true }); - } catch (e) { - // It seems that Windows doesn't let us delete this, with errors like: - // - // Error: EBUSY: resource busy or locked, rmdir 'C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ' - // ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ - // Serialized Error: { - // "code": "EBUSY", - // "errno": -4082, - // "path": "C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ", - // "syscall": "rmdir", - // } - console.error(e); - } + afterAll(() => { + removeDir(tmpDir, { fireAndForget: true }); }); it("should allow quitting while the image is building", async () => { @@ -675,22 +648,8 @@ if (process.platform === "win32") { }); } }); - afterAll(async () => { - try { - fs.rmSync(tmpDir, { recursive: true, force: true }); - } catch (e) { - // It seems that Windows doesn't let us delete this, with errors like: - // - // Error: EBUSY: resource busy or locked, rmdir 'C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ' - // ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ - // Serialized Error: { - // "code": "EBUSY", - // "errno": -4082, - // "path": "C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ", - // "syscall": "rmdir", - // } - console.error(e); - } + afterAll(() => { + removeDir(tmpDir, { fireAndForget: true }); }); it("should print build logs for all the containers", async () => { @@ -837,22 +796,8 @@ if (process.platform === "win32") { }); } }); - afterAll(async () => { - try { - fs.rmSync(tmpDir, { recursive: true, force: true }); - } catch (e) { - // It seems that Windows doesn't let us delete this, with errors like: - // - // Error: EBUSY: resource busy or locked, rmdir 'C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ' - // ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ - // Serialized Error: { - // "code": "EBUSY", - // "errno": -4082, - // "path": "C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ", - // "syscall": "rmdir", - // } - console.error(e); - } + afterAll(() => { + removeDir(tmpDir, { fireAndForget: true }); }); it("should be able to interact with both workers, rebuild the containers with the hotkey and all containers should be cleaned up at the end", async () => { diff --git a/fixtures/shared/package.json b/fixtures/shared/package.json index 11a882dcc27a..84519ee2f7ac 100644 --- a/fixtures/shared/package.json +++ b/fixtures/shared/package.json @@ -3,6 +3,7 @@ "private": true, "description": "Shared fixtures for testing", "devDependencies": { + "@cloudflare/workers-utils": "workspace:*", "wrangler": "workspace:*" }, "volta": { diff --git a/fixtures/shared/src/fs-helpers.ts b/fixtures/shared/src/fs-helpers.ts new file mode 100644 index 000000000000..e23ce7121e44 --- /dev/null +++ b/fixtures/shared/src/fs-helpers.ts @@ -0,0 +1,3 @@ +// Re-exported so that fixtures can import from `@fixture/shared/src/fs-helpers` +// without each fixture needing a direct dependency on `@cloudflare/workers-utils`. +export { removeDir, removeDirSync } from "@cloudflare/workers-utils"; diff --git a/fixtures/wildcard-modules/package.json b/fixtures/wildcard-modules/package.json index 8e3552d37b8b..83595f13a988 100644 --- a/fixtures/wildcard-modules/package.json +++ b/fixtures/wildcard-modules/package.json @@ -14,6 +14,7 @@ "@cloudflare/eslint-config-shared": "workspace:*", "@cloudflare/workers-tsconfig": "workspace:*", "@cloudflare/workers-types": "catalog:default", + "@fixture/shared": "workspace:*", "undici": "catalog:default", "wrangler": "workspace:*" }, diff --git a/fixtures/wildcard-modules/test/index.test.ts b/fixtures/wildcard-modules/test/index.test.ts index 98c21d2bfc87..a5e2d12f2bfd 100644 --- a/fixtures/wildcard-modules/test/index.test.ts +++ b/fixtures/wildcard-modules/test/index.test.ts @@ -4,6 +4,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { setTimeout } from "node:timers/promises"; +import { removeDir } from "@fixture/shared/src/fs-helpers"; import { fetch } from "undici"; import { afterAll, assert, beforeAll, describe, test } from "vitest"; import { @@ -55,21 +56,7 @@ describe("wildcard imports: dev", () => { }); afterAll(async () => { await worker.stop(); - try { - await fs.rm(tmpDir, { recursive: true, force: true }); - } catch (e) { - // It seems that Windows doesn't let us delete this, with errors like: - // - // Error: EBUSY: resource busy or locked, rmdir 'C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ' - // ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ - // Serialized Error: { - // "code": "EBUSY", - // "errno": -4082, - // "path": "C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ", - // "syscall": "rmdir", - // } - console.error(e); - } + removeDir(tmpDir, { fireAndForget: true }); }); test("supports bundled modules", async ({ expect }) => { @@ -152,8 +139,8 @@ describe("wildcard imports: deploy", () => { beforeAll(async () => { tmpDir = await getTmpDir(); }); - afterAll(async () => { - await fs.rm(tmpDir, { recursive: true, force: true }); + afterAll(() => { + removeDir(tmpDir, { fireAndForget: true }); }); test("bundles wildcard modules", async ({ expect }) => { diff --git a/packages/create-cloudflare/e2e/helpers/index.ts b/packages/create-cloudflare/e2e/helpers/index.ts index e3111040d90a..bbd61260387f 100644 --- a/packages/create-cloudflare/e2e/helpers/index.ts +++ b/packages/create-cloudflare/e2e/helpers/index.ts @@ -48,10 +48,11 @@ const testProjectDir = (suite: string, test: string) => { realpathSync(mkdtempSync(nodePath.join(tmpdir(), `c3-tests-${suite}`))); const filepath = getPath(); + // eslint-disable-next-line workers-sdk/no-direct-recursive-rm -- see log-stream.ts for rationale rmSync(filepath, { recursive: true, force: true, - maxRetries: 10, + maxRetries: 5, retryDelay: 100, }); } catch (e) { diff --git a/packages/create-cloudflare/e2e/helpers/log-stream.ts b/packages/create-cloudflare/e2e/helpers/log-stream.ts index a3bcc35b9b7d..ca3b77a578f2 100644 --- a/packages/create-cloudflare/e2e/helpers/log-stream.ts +++ b/packages/create-cloudflare/e2e/helpers/log-stream.ts @@ -20,9 +20,19 @@ export function createTestLogStream(task: RunnerTask) { export function recreateLogFolder(suite: RunnerTestSuite) { // Clean the old folder if exists (useful for dev) + // + // Note: this is intentionally inlined rather than importing `removeDirSync` + // from `@cloudflare/workers-utils`. That package's barrel export pulls in CJS + // dependencies (e.g. `xdg-app-paths`) that break when Vite bundles the vitest + // config (which imports this file) into ESM — the shimmed `require()` calls + // throw "Dynamic require of 'path' is not supported". + // Keep aligned with `removeDirSync()` in `packages/workers-utils/src/fs-helpers.ts`. + // eslint-disable-next-line workers-sdk/no-direct-recursive-rm rmSync(getLogPath(suite), { recursive: true, force: true, + maxRetries: 5, + retryDelay: 100, }); mkdirSync(getLogPath(suite), { recursive: true }); diff --git a/packages/create-cloudflare/e2e/tests/cli/cli.test.ts b/packages/create-cloudflare/e2e/tests/cli/cli.test.ts index 1d4b83cbfdf8..d57c1079bd72 100644 --- a/packages/create-cloudflare/e2e/tests/cli/cli.test.ts +++ b/packages/create-cloudflare/e2e/tests/cli/cli.test.ts @@ -1,5 +1,5 @@ import { execSync } from "node:child_process"; -import fs, { readFileSync } from "node:fs"; +import fs from "node:fs"; import { basename, join, resolve } from "node:path"; import { detectPackageManager } from "helpers/packageManagers"; // eslint-disable-next-line workers-sdk/no-vitest-import-expect -- e2e test with complex patterns @@ -188,10 +188,11 @@ describe("Create Cloudflare CLI", () => { expect(output).toContain(`lang TypeScript`); expect(output).toContain(`no deploy`); } finally { + // eslint-disable-next-line workers-sdk/no-direct-recursive-rm -- see e2e/helpers/log-stream.ts for rationale fs.rmSync(existingFilePath, { recursive: true, force: true, - maxRetries: 10, + maxRetries: 5, retryDelay: 100, }); } @@ -257,7 +258,7 @@ describe("Create Cloudflare CLI", () => { // underlying issue appears to be with the packages pinned in // the template - not whether or not the settings.json file is // created - expect(readFileSync(`${project.path}/.vscode/settings.json`, "utf8")) + expect(fs.readFileSync(`${project.path}/.vscode/settings.json`, "utf8")) .toMatchInlineSnapshot(` "{ "files.associations": { diff --git a/packages/create-cloudflare/e2e/tests/frameworks/test-config.ts b/packages/create-cloudflare/e2e/tests/frameworks/test-config.ts index 7dd3c7d980ea..62c91442133a 100644 --- a/packages/create-cloudflare/e2e/tests/frameworks/test-config.ts +++ b/packages/create-cloudflare/e2e/tests/frameworks/test-config.ts @@ -167,9 +167,6 @@ function getFrameworkTestConfig(pm: string): NamedFrameworkTestConfig[] { }, nodeCompat: false, flags: ["--style", "sass"], - // TODO: currently the Angular tests are failing with `URL with hostname "localhost" is not allowed.` - // we need to investigate this so that we can un-quarantine the Angular tests - quarantine: true, }, { name: "angular:workers", @@ -189,9 +186,6 @@ function getFrameworkTestConfig(pm: string): NamedFrameworkTestConfig[] { }, nodeCompat: false, flags: ["--style", "sass"], - // TODO: currently the Angular tests are failing with `URL with hostname "localhost" is not allowed.` - // we need to investigate this so that we can un-quarantine the Angular tests - quarantine: true, }, { name: "gatsby:pages", @@ -790,9 +784,6 @@ function getExperimentalFrameworkTestConfig( nodeCompat: false, flags: ["--style", "sass"], verifyTypes: false, - // TODO: currently the Angular tests are failing with `URL with hostname "localhost" is not allowed.` - // we need to investigate this so that we can un-quarantine the Angular tests - quarantine: true, }, { name: "nuxt:workers", diff --git a/packages/create-cloudflare/src/helpers/packageManagers.ts b/packages/create-cloudflare/src/helpers/packageManagers.ts index 49b1041af3ae..802626cec0d4 100644 --- a/packages/create-cloudflare/src/helpers/packageManagers.ts +++ b/packages/create-cloudflare/src/helpers/packageManagers.ts @@ -105,7 +105,20 @@ export const rectifyPmMismatch = async (ctx: C3Context) => { const nodeModulesPath = nodePath.join(ctx.project.path, "node_modules"); if (existsSync(nodeModulesPath)) { - rmSync(nodeModulesPath, { recursive: true }); + // Note: this is intentionally inlined rather than importing `removeDirSync` + // from `@cloudflare/workers-utils`. That package's barrel export pulls in CJS + // dependencies (e.g. `xdg-app-paths`) that break when Vite bundles code into + // ESM — the shimmed `require()` calls throw "Dynamic require of 'path' is not + // supported". While the production build (esbuild → CJS) would handle this + // fine, the e2e tests import this file through Vitest which uses Vite's bundler. + // Keep aligned with `removeDirSync()` in `packages/workers-utils/src/fs-helpers.ts`. + // eslint-disable-next-line workers-sdk/no-direct-recursive-rm + rmSync(nodeModulesPath, { + recursive: true, + force: true, + maxRetries: 5, + retryDelay: 100, + }); } const lockfilePath = nodePath.join(ctx.project.path, "package-lock.json"); diff --git a/packages/create-cloudflare/templates/angular/pages/templates/src/server.ts b/packages/create-cloudflare/templates/angular/pages/templates/src/server.ts index f75db0a749af..a13fcf7814ad 100644 --- a/packages/create-cloudflare/templates/angular/pages/templates/src/server.ts +++ b/packages/create-cloudflare/templates/angular/pages/templates/src/server.ts @@ -1,6 +1,10 @@ import { AngularAppEngine, createRequestHandler } from '@angular/ssr'; -const angularApp = new AngularAppEngine(); +const angularApp = new AngularAppEngine({ + // It is safe to set allow `localhost`, so that SSR can run in local development, + // as, in production, Cloudflare will ensure that `localhost` is not the host. + allowedHosts: ['localhost'], +}); /** * This is a request handler used by the Angular CLI (dev-server and during build). diff --git a/packages/create-cloudflare/templates/angular/workers/templates/src/server.ts b/packages/create-cloudflare/templates/angular/workers/templates/src/server.ts index f75db0a749af..a13fcf7814ad 100644 --- a/packages/create-cloudflare/templates/angular/workers/templates/src/server.ts +++ b/packages/create-cloudflare/templates/angular/workers/templates/src/server.ts @@ -1,6 +1,10 @@ import { AngularAppEngine, createRequestHandler } from '@angular/ssr'; -const angularApp = new AngularAppEngine(); +const angularApp = new AngularAppEngine({ + // It is safe to set allow `localhost`, so that SSR can run in local development, + // as, in production, Cloudflare will ensure that `localhost` is not the host. + allowedHosts: ['localhost'], +}); /** * This is a request handler used by the Angular CLI (dev-server and during build). diff --git a/packages/eslint-config-shared/index.js b/packages/eslint-config-shared/index.js index 61756b4b3e66..19ac24be0c33 100644 --- a/packages/eslint-config-shared/index.js +++ b/packages/eslint-config-shared/index.js @@ -6,6 +6,7 @@ import turbo from "eslint-plugin-turbo"; import unusedImports from "eslint-plugin-unused-imports"; import { defineConfig, globalIgnores } from "eslint/config"; import tseslint from "typescript-eslint"; +import noDirectRecursiveRm from "./rules/no-direct-recursive-rm.mjs"; import noUnsafeCommandExecution from "./rules/no-unsafe-command-execution.mjs"; import noVitestImportExpect from "./rules/no-vitest-import-expect.mjs"; @@ -34,6 +35,7 @@ export default defineConfig( "no-only-tests": noOnlyTests, "workers-sdk": { rules: { + "no-direct-recursive-rm": noDirectRecursiveRm, "no-unsafe-command-execution": noUnsafeCommandExecution, "no-vitest-import-expect": noVitestImportExpect, }, @@ -87,6 +89,7 @@ export default defineConfig( argsIgnorePattern: "^_", }, ], + "workers-sdk/no-direct-recursive-rm": "error", "workers-sdk/no-unsafe-command-execution": "error", // Enforce default import for ci-info so that test mocks work correctly. diff --git a/packages/eslint-config-shared/package.json b/packages/eslint-config-shared/package.json index 60e76f0a1d35..bfe27ace18af 100644 --- a/packages/eslint-config-shared/package.json +++ b/packages/eslint-config-shared/package.json @@ -11,8 +11,8 @@ }, "scripts": { "deploy": "echo 'no deploy'", - "test": "node rules/__tests__/no-unsafe-command-execution.test.mjs", - "test:ci": "node rules/__tests__/no-unsafe-command-execution.test.mjs" + "test": "node rules/__tests__/no-unsafe-command-execution.test.mjs && node rules/__tests__/no-direct-recursive-rm.test.mjs", + "test:ci": "node rules/__tests__/no-unsafe-command-execution.test.mjs && node rules/__tests__/no-direct-recursive-rm.test.mjs" }, "devDependencies": { "@cloudflare/workers-tsconfig": "workspace:*", diff --git a/packages/eslint-config-shared/rules/__tests__/no-direct-recursive-rm.test.mjs b/packages/eslint-config-shared/rules/__tests__/no-direct-recursive-rm.test.mjs new file mode 100644 index 000000000000..120bfd27a941 --- /dev/null +++ b/packages/eslint-config-shared/rules/__tests__/no-direct-recursive-rm.test.mjs @@ -0,0 +1,129 @@ +import { RuleTester } from "eslint"; +import rule from "../no-direct-recursive-rm.mjs"; + +const ruleTester = new RuleTester({ + languageOptions: { ecmaVersion: 2022, sourceType: "module" }, +}); + +ruleTester.run("no-direct-recursive-rm", rule, { + valid: [ + // Single file removal (no recursive option) — should be allowed + { + code: 'import fs from "node:fs"; fs.rmSync("file.txt");', + }, + { + code: 'import fs from "node:fs"; fs.rmSync("file.txt", { force: true });', + }, + { + code: 'import fs from "node:fs/promises"; await fs.rm("file.txt");', + }, + { + code: 'import fs from "node:fs/promises"; await fs.rm("file.txt", { force: true });', + }, + // Named import without recursive + { + code: 'import { rmSync } from "node:fs"; rmSync("file.txt");', + }, + { + code: 'import { rm } from "node:fs/promises"; await rm("file.txt", { force: true });', + }, + // Named import rm from "node:fs" (callback-based) without recursive + { + code: 'import { rm } from "node:fs"; rm("file.txt", (err) => {});', + }, + // Using the correct helpers + { + code: 'import { removeDir } from "@cloudflare/workers-utils"; await removeDir("dir");', + }, + { + code: 'import { removeDirSync } from "@cloudflare/workers-utils"; removeDirSync("dir");', + }, + // recursive: false is fine + { + code: 'import fs from "node:fs"; fs.rmSync("dir", { recursive: false });', + }, + // Unrelated function called rm + { + code: 'function rm(path, opts) {} rm("dir", { recursive: true });', + }, + // CommonJS single file removal + { + code: 'const fs = require("node:fs"); fs.rmSync("file.txt");', + }, + { + code: 'const fs = require("node:fs/promises"); fs.rm("file.txt", { force: true });', + }, + ], + + invalid: [ + // Default import from "node:fs" — fs.rmSync with recursive + { + code: 'import fs from "node:fs"; fs.rmSync("dir", { recursive: true, force: true });', + errors: [{ messageId: "noDirectRecursiveRm" }], + }, + // Namespace import from "node:fs" — fs.rmSync with recursive + { + code: 'import * as fs from "node:fs"; fs.rmSync("dir", { recursive: true });', + errors: [{ messageId: "noDirectRecursiveRm" }], + }, + // Named import from "node:fs" — rmSync with recursive + { + code: 'import { rmSync } from "node:fs"; rmSync("dir", { recursive: true, force: true });', + errors: [{ messageId: "noDirectRecursiveRm" }], + }, + // Default import from "node:fs/promises" — fs.rm with recursive + { + code: 'import fs from "node:fs/promises"; await fs.rm("dir", { recursive: true, force: true });', + errors: [{ messageId: "noDirectRecursiveRm" }], + }, + // Named import from "node:fs/promises" — rm with recursive + { + code: 'import { rm } from "node:fs/promises"; await rm("dir", { recursive: true, maxRetries: 10 });', + errors: [{ messageId: "noDirectRecursiveRm" }], + }, + // fs.promises.rm pattern + { + code: 'import fs from "node:fs"; fs.promises.rm("dir", { recursive: true, force: true });', + errors: [{ messageId: "noDirectRecursiveRm" }], + }, + // Without "node:" prefix + { + code: 'import fs from "fs"; fs.rmSync("dir", { recursive: true, force: true });', + errors: [{ messageId: "noDirectRecursiveRm" }], + }, + { + code: 'import fs from "fs/promises"; await fs.rm("dir", { force: true, recursive: true });', + errors: [{ messageId: "noDirectRecursiveRm" }], + }, + // CommonJS require — default + { + code: 'const fs = require("node:fs"); fs.rmSync("dir", { recursive: true, force: true });', + errors: [{ messageId: "noDirectRecursiveRm" }], + }, + // CommonJS require — promises + { + code: 'const fs = require("node:fs/promises"); fs.rm("dir", { recursive: true });', + errors: [{ messageId: "noDirectRecursiveRm" }], + }, + // CommonJS require — fs.promises.rm + { + code: 'const fs = require("node:fs"); fs.promises.rm("dir", { recursive: true, force: true });', + errors: [{ messageId: "noDirectRecursiveRm" }], + }, + // Callback-based fs.rm from "node:fs" with recursive in second arg + { + code: 'import fs from "node:fs"; fs.rm("dir", { recursive: true, force: true }, (err) => {});', + errors: [{ messageId: "noDirectRecursiveRm" }], + }, + // Named import rm from "node:fs" (callback-based) with recursive + { + code: 'import { rm } from "node:fs"; rm("dir", { recursive: true, force: true }, (err) => {});', + errors: [{ messageId: "noDirectRecursiveRm" }], + }, + // Named import rm from "node:fs" with recursive (no callback) + { + code: 'import { rm } from "node:fs"; rm("dir", { recursive: true, force: true });', + errors: [{ messageId: "noDirectRecursiveRm" }], + }, + ], +}); diff --git a/packages/eslint-config-shared/rules/__tests__/no-unsafe-command-execution.test.mjs b/packages/eslint-config-shared/rules/__tests__/no-unsafe-command-execution.test.mjs index 358bf3f79b0a..b18bee00ce1c 100644 --- a/packages/eslint-config-shared/rules/__tests__/no-unsafe-command-execution.test.mjs +++ b/packages/eslint-config-shared/rules/__tests__/no-unsafe-command-execution.test.mjs @@ -190,5 +190,3 @@ if (!commitMessage) { }, ], }); - -console.log("✅ All tests passed!"); diff --git a/packages/eslint-config-shared/rules/no-direct-recursive-rm.mjs b/packages/eslint-config-shared/rules/no-direct-recursive-rm.mjs new file mode 100644 index 000000000000..63bc2f54def3 --- /dev/null +++ b/packages/eslint-config-shared/rules/no-direct-recursive-rm.mjs @@ -0,0 +1,235 @@ +/** + * Module sources for Node.js filesystem APIs + */ +const FS_MODULES = ["fs", "node:fs"]; +const FS_PROMISES_MODULES = ["fs/promises", "node:fs/promises"]; + +/** + * Check if an options argument contains `recursive: true` + */ +function hasRecursiveOption(node) { + if (!node || node.type !== "ObjectExpression") { + return false; + } + + return node.properties.some( + (prop) => + prop.type === "Property" && + prop.key.type === "Identifier" && + prop.key.name === "recursive" && + prop.value.type === "Literal" && + prop.value.value === true + ); +} + +/** + * Resolve the import source for a given identifier by walking the scope chain. + * Returns the module specifier string (e.g. "node:fs") or null if not from an fs module. + */ +function resolveImportSource(name, scope) { + let currentScope = scope; + while (currentScope) { + const variable = currentScope.set.get(name); + if (variable && variable.defs.length > 0) { + const def = variable.defs[0]; + + // ES module import: import fs from "node:fs" / import { rmSync } from "node:fs" + if ( + def.type === "ImportBinding" && + def.parent?.type === "ImportDeclaration" + ) { + return def.parent.source?.value ?? null; + } + + // CommonJS require: const fs = require("node:fs") + if ( + def.type === "Variable" && + def.node?.init?.type === "CallExpression" && + def.node.init.callee.name === "require" && + def.node.init.arguments[0]?.type === "Literal" + ) { + return def.node.init.arguments[0].value; + } + } + currentScope = currentScope.upper; + } + return null; +} + +/** + * Get the scope for a node using the available API. + */ +function getScope(node, context) { + const sourceCode = context.sourceCode || context.getSourceCode(); + return sourceCode.getScope ? sourceCode.getScope(node) : context.getScope(); +} + +/** + * Check if a CallExpression is a recursive rm/rmSync call from an fs module. + * + * Supported patterns: + * - fs.rm(path, { recursive: true }) (fs from "node:fs/promises") + * - fs.rmSync(path, { recursive: true }) (fs from "node:fs") + * - fs.promises.rm(path, { recursive: true }) (fs from "node:fs") + * - rm(path, { recursive: true }) (named import from "node:fs/promises") + * - rmSync(path, { recursive: true }) (named import from "node:fs") + */ +function checkCall(node, context) { + const scope = getScope(node, context); + const callee = node.callee; + + // Pattern 1: Direct named import call — rm(...) or rmSync(...) + if (callee.type === "Identifier") { + const name = callee.name; + if (name !== "rm" && name !== "rmSync") { + return null; + } + + const source = resolveImportSource(name, scope); + if (!source) { + return null; + } + + // rmSync must come from "node:fs" / "fs" + if (name === "rmSync" && FS_MODULES.includes(source)) { + const options = node.arguments[1]; + if (hasRecursiveOption(options)) { + return name; + } + } + + // rm can come from "node:fs/promises" / "fs/promises" + if (name === "rm" && FS_PROMISES_MODULES.includes(source)) { + const options = node.arguments[1]; + if (hasRecursiveOption(options)) { + return name; + } + } + + // rm can also come from "node:fs" / "fs" (callback-based) + if (name === "rm" && FS_MODULES.includes(source)) { + const options = node.arguments[1]; + if (hasRecursiveOption(options)) { + return name; + } + // callback-based fs.rm: options may be in arg[2] if arg[1] is the callback + const options2 = node.arguments[2]; + if (hasRecursiveOption(options2)) { + return name; + } + } + + return null; + } + + // Pattern 2: Member expression — obj.rm(...) or obj.rmSync(...) or obj.promises.rm(...) + if (callee.type === "MemberExpression") { + const prop = callee.property; + if (prop.type !== "Identifier") { + return null; + } + + // Sub-pattern 2a: fs.rmSync(...) or fs.rm(...) + if ( + (prop.name === "rm" || prop.name === "rmSync") && + callee.object.type === "Identifier" + ) { + const source = resolveImportSource(callee.object.name, scope); + if (!source) { + return null; + } + + // fs.rmSync from "node:fs" + if (prop.name === "rmSync" && FS_MODULES.includes(source)) { + const options = node.arguments[1]; + if (hasRecursiveOption(options)) { + return prop.name; + } + } + + // fs.rm from "node:fs/promises" + if (prop.name === "rm" && FS_PROMISES_MODULES.includes(source)) { + const options = node.arguments[1]; + if (hasRecursiveOption(options)) { + return prop.name; + } + } + + // fs.rm from "node:fs" — this is fs.rm (callback-based) + if (prop.name === "rm" && FS_MODULES.includes(source)) { + const options = node.arguments[1]; + // callback-based fs.rm: the options may be arg[1] or arg[2] if there's a callback + if (hasRecursiveOption(options)) { + return prop.name; + } + const options2 = node.arguments[2]; + if (hasRecursiveOption(options2)) { + return prop.name; + } + } + + return null; + } + + // Sub-pattern 2b: fs.promises.rm(...) + if ( + prop.name === "rm" && + callee.object.type === "MemberExpression" && + callee.object.property.type === "Identifier" && + callee.object.property.name === "promises" && + callee.object.object.type === "Identifier" + ) { + const source = resolveImportSource(callee.object.object.name, scope); + if (source && FS_MODULES.includes(source)) { + const options = node.arguments[1]; + if (hasRecursiveOption(options)) { + return "promises.rm"; + } + } + return null; + } + } + + return null; +} + +export default { + meta: { + type: "suggestion", + docs: { + description: + "Disallow direct fs.rm/fs.rmSync with recursive option; use removeDir/removeDirSync from @cloudflare/workers-utils instead", + category: "Best Practices", + recommended: true, + }, + messages: { + noDirectRecursiveRm: + 'Use {{ replacement }} from "@cloudflare/workers-utils" instead of {{ method }} with { recursive: true }. ' + + "The helper provides sensible defaults for retries on Windows.", + }, + schema: [], // no options + }, + + create(context) { + return { + CallExpression(node) { + const method = checkCall(node, context); + if (!method) { + return; + } + + const isSync = method === "rmSync"; + const replacement = isSync ? "removeDirSync" : "removeDir"; + + context.report({ + node, + messageId: "noDirectRecursiveRm", + data: { + method, + replacement, + }, + }); + }, + }; + }, +}; diff --git a/packages/local-explorer-ui/package.json b/packages/local-explorer-ui/package.json index d221181e880d..f4060444d596 100644 --- a/packages/local-explorer-ui/package.json +++ b/packages/local-explorer-ui/package.json @@ -20,6 +20,12 @@ "dependencies": { "@base-ui/react": "^1.1.0", "@cloudflare/kumo": "^1.5.0", + "@cloudflare/workers-editor-shared": "^0.1.1", + "@codemirror/autocomplete": "^6.20.0", + "@codemirror/lang-sql": "^6.10.0", + "@codemirror/language": "^6.12.1", + "@codemirror/state": "^6.5.4", + "@codemirror/view": "^6.39.14", "@phosphor-icons/react": "^2.1.10", "@tailwindcss/vite": "^4.0.15", "@tanstack/react-router": "^1.158.0", diff --git a/packages/local-explorer-ui/src/__tests__/drivers/common.test.ts b/packages/local-explorer-ui/src/__tests__/drivers/common.test.ts index 7d577d0db578..0fc5a14f06dd 100644 --- a/packages/local-explorer-ui/src/__tests__/drivers/common.test.ts +++ b/packages/local-explorer-ui/src/__tests__/drivers/common.test.ts @@ -15,6 +15,7 @@ const EMPTY_RESULT = { rowsAffected: 0, rowsRead: null, rowsWritten: null, + rowCount: 0, }, } satisfies StudioResultSet; diff --git a/packages/local-explorer-ui/src/__tests__/drivers/sqlite/driver.test.ts b/packages/local-explorer-ui/src/__tests__/drivers/sqlite/driver.test.ts index 4d923b8e6d6a..0a630fb3976d 100644 --- a/packages/local-explorer-ui/src/__tests__/drivers/sqlite/driver.test.ts +++ b/packages/local-explorer-ui/src/__tests__/drivers/sqlite/driver.test.ts @@ -10,6 +10,7 @@ const EMPTY_RESULT = { rowsAffected: 0, rowsRead: null, rowsWritten: null, + rowCount: 0, }, } satisfies StudioResultSet; diff --git a/packages/local-explorer-ui/src/__tests__/utils/studio.test.ts b/packages/local-explorer-ui/src/__tests__/utils/studio.test.ts index ab700c175f8b..f6fc0d21f2e5 100644 --- a/packages/local-explorer-ui/src/__tests__/utils/studio.test.ts +++ b/packages/local-explorer-ui/src/__tests__/utils/studio.test.ts @@ -1,9 +1,9 @@ import { describe, expect, test } from "vitest"; import { escapeSqlValue, - tokenizeSQL, transformStudioArrayBasedResult, } from "../../utils/studio"; +import { tokenizeSQL } from "../../utils/studio/sql"; describe("escapeSqlValue", () => { test("undefined returns `DEFAULT`", () => { diff --git a/packages/local-explorer-ui/src/components/Sidebar.tsx b/packages/local-explorer-ui/src/components/Sidebar.tsx index 3141d0f1dd0c..6b5a73214156 100644 --- a/packages/local-explorer-ui/src/components/Sidebar.tsx +++ b/packages/local-explorer-ui/src/components/Sidebar.tsx @@ -35,25 +35,26 @@ function SidebarItemGroup({ title, }: SidebarItemGroupProps): JSX.Element { return ( - - - - + + + + {title} -