From 5ebb27926152bfaf9b9f77adfc1fe8fd3901e187 Mon Sep 17 00:00:00 2001 From: Bingbing Date: Thu, 3 Sep 2026 15:35:28 +0800 Subject: [PATCH] fix: address circular reference review comments --- .dependency-cruiser.json | 6 --- .github/workflows/test.yml | 3 +- package.json | 4 +- .../src/db/adapters/insomnia-adapter.ts | 2 +- packages/insomnia-inso/src/db/models/types.ts | 4 +- .../check-cycle-references.mjs | 41 +++++++++++++------ .../dependency-cruiser.json | 4 ++ .../circular-references/known-violations.json | 4 +- 8 files changed, 41 insertions(+), 27 deletions(-) delete mode 100644 .dependency-cruiser.json rename scripts/{ => circular-references}/check-cycle-references.mjs (74%) create mode 100644 scripts/circular-references/dependency-cruiser.json rename .dependency-cruiser-known-violations.json => scripts/circular-references/known-violations.json (99%) diff --git a/.dependency-cruiser.json b/.dependency-cruiser.json deleted file mode 100644 index f26537993e..0000000000 --- a/.dependency-cruiser.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "forbidden": [ - { "name": "no-circular", "severity": "error", "from": {}, "to": { "circular": true } } - ], - "options": { "tsPreCompilationDeps": true } -} diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 64ff42545e..dd18ef8197 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -106,10 +106,11 @@ jobs: - name: Check circular references id: check-cycles continue-on-error: true + shell: bash run: npm run check-cycle-references | tee cycle-report.txt - name: Post circular references PR comment - if: github.event_name == 'pull_request' && always() + if: github.event_name == 'pull_request' && always() && steps.check-cycles.outcome != 'skipped' uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: github-token: ${{ secrets.GITHUB_TOKEN }} diff --git a/package.json b/package.json index 1e870e4927..1422e27771 100644 --- a/package.json +++ b/package.json @@ -45,8 +45,8 @@ "test:crit:package": "npm run test:package -w insomnia-smoke-test -- --project=Critical", "test:crit:dev": "npm run test:dev -w insomnia-smoke-test -- --project=Critical", "postinstall": "patch-package && npm run verify-bundle-plugins -w insomnia && npm run install-libcurl-electron", - "check-cycle-references": "node scripts/check-cycle-references.mjs", - "check-cycle-references:baseline": "node scripts/check-cycle-references.mjs --update-baseline" + "check-cycle-references": "node scripts/circular-references/check-cycle-references.mjs", + "check-cycle-references:baseline": "node scripts/circular-references/check-cycle-references.mjs --update-baseline" }, "devDependencies": { "@develohpanda/fluent-builder": "^2.1.2", diff --git a/packages/insomnia-inso/src/db/adapters/insomnia-adapter.ts b/packages/insomnia-inso/src/db/adapters/insomnia-adapter.ts index 6311e3859c..98a9d49a09 100644 --- a/packages/insomnia-inso/src/db/adapters/insomnia-adapter.ts +++ b/packages/insomnia-inso/src/db/adapters/insomnia-adapter.ts @@ -127,7 +127,7 @@ const insomniaAdapter: DbAdapter = async (filePath, filterTypes) => { const obj = parseRaw(model); // Store it, only if the key value exists - (db[obj.type] as {}[])?.push(obj); + (db[obj.type as keyof typeof db] as {}[])?.push(obj); } }); diff --git a/packages/insomnia-inso/src/db/models/types.ts b/packages/insomnia-inso/src/db/models/types.ts index 56fc7659fb..0737dea8e5 100644 --- a/packages/insomnia-inso/src/db/models/types.ts +++ b/packages/insomnia-inso/src/db/models/types.ts @@ -1,10 +1,10 @@ // Simplified and extracted from insomnia/src/models/* -import type { Database } from '../types'; +import type { AllTypes } from 'insomnia-data'; export interface BaseModel { _id: string; name: string; - type: keyof Database; + type: AllTypes; parentId: string; } diff --git a/scripts/check-cycle-references.mjs b/scripts/circular-references/check-cycle-references.mjs similarity index 74% rename from scripts/check-cycle-references.mjs rename to scripts/circular-references/check-cycle-references.mjs index 1d3a602e04..44f5cf72e4 100755 --- a/scripts/check-cycle-references.mjs +++ b/scripts/circular-references/check-cycle-references.mjs @@ -8,10 +8,10 @@ import fs from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; -const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..'); const depcruiseBin = path.join(repoRoot, 'node_modules', '.bin', 'depcruise'); -const configPath = path.join(repoRoot, '.dependency-cruiser.json'); -const baselinePath = path.join(repoRoot, '.dependency-cruiser-known-violations.json'); +const configPath = path.join(path.dirname(fileURLToPath(import.meta.url)), 'dependency-cruiser.json'); +const baselinePath = path.join(path.dirname(fileURLToPath(import.meta.url)), 'known-violations.json'); // Same scope as `npm run lint`/`type-check`/`test` (--workspaces --if-present): the packages // actually declared as npm workspaces, not every directory under packages/ (e.g. @@ -46,7 +46,15 @@ function cruisePackage({ name, dir }) { // from a prior local build) at any depth, e.g. packages/insomnia/build/. args.push('-x', 'node_modules|(^|/)(build|dist)(/|$)', '--output-type', 'json', '.'); - const stdout = execFileSync(depcruiseBin, args, { cwd: dir, encoding: 'utf8', maxBuffer: 1024 * 1024 * 100 }); + let stdout; + try { + stdout = execFileSync(depcruiseBin, args, { cwd: dir, encoding: 'utf8', maxBuffer: 1024 * 1024 * 100 }); + } catch (error) { + if (error?.status !== 1 || !error.stdout) throw error; + // dependency-cruiser exits with status 1 when forbidden violations are found. Parse its JSON + // output so the baseline comparison can decide whether those cycles are new. + stdout = error.stdout; + } const result = JSON.parse(stdout); // key: canonical signature -> Set of packages it touches @@ -104,28 +112,37 @@ function main() { if (updateBaseline) { writeBaseline(current); const total = [...current.values()].reduce((sum, set) => sum + set.size, 0); - console.log(`Baseline updated: ${baselinePath} (${total} cycle attributions across ${packageNames.length} packages)`); + console.log( + `Baseline updated: ${baselinePath} (${total} cycle attributions across ${packageNames.length} packages)`, + ); return; } const baseline = readBaseline(); let hasNew = false; + let hasDrift = false; for (const name of packageNames) { const currentKeys = current.get(name); const baselineKeys = baseline.get(name) || new Set(); const newKeys = [...currentKeys].filter(key => !baselineKeys.has(key)); - console.log(`${name}: ${currentKeys.size} cycle(s)${newKeys.length ? `, ${newKeys.length} NEW` : ''}`); + const removedKeys = [...baselineKeys].filter(key => !currentKeys.has(key)); + console.log( + `${name}: ${currentKeys.size} cycle(s)${newKeys.length ? `, ${newKeys.length} NEW` : ''}${removedKeys.length ? `, ${removedKeys.length} REMOVED` : ''}`, + ); if (newKeys.length) { hasNew = true; - for (const key of newKeys) { - console.log(` NEW: ${key}`); - } + for (const key of newKeys) console.log(` NEW: ${key}`); + } + if (removedKeys.length) { + hasDrift = true; + for (const key of removedKeys) console.log(` REMOVED FROM BASELINE: ${key}`); } } - if (hasNew) { - console.log('\nNew circular dependencies detected that are not in the baseline.'); - console.log(`If intentional/unavoidable, run "npm run check-cycle-references:baseline" and commit the updated ${path.basename(baselinePath)}.`); + if (hasNew || hasDrift) { + if (hasNew) console.log('\nNew circular dependencies detected that are not in the baseline.'); + if (hasDrift) console.log('\nBaseline contains circular dependencies no longer present in the current tree.'); + console.log(`Run "npm run check-cycle-references:baseline" and commit the updated ${path.basename(baselinePath)}.`); process.exit(1); } console.log('\nNo new circular dependencies.'); diff --git a/scripts/circular-references/dependency-cruiser.json b/scripts/circular-references/dependency-cruiser.json new file mode 100644 index 0000000000..2f1c572ade --- /dev/null +++ b/scripts/circular-references/dependency-cruiser.json @@ -0,0 +1,4 @@ +{ + "forbidden": [{ "name": "no-circular", "severity": "warn", "from": {}, "to": { "circular": true } }], + "options": { "tsPreCompilationDeps": true } +} diff --git a/.dependency-cruiser-known-violations.json b/scripts/circular-references/known-violations.json similarity index 99% rename from .dependency-cruiser-known-violations.json rename to scripts/circular-references/known-violations.json index 3c34ecdaad..6c6efee02e 100644 --- a/.dependency-cruiser-known-violations.json +++ b/scripts/circular-references/known-violations.json @@ -188,9 +188,7 @@ "insomnia-analytics": [], "insomnia-api": [], "insomnia-data": [], - "insomnia-inso": [ - "packages/insomnia-inso/src/db/models/types.ts -> packages/insomnia-inso/src/db/types.ts" - ], + "insomnia-inso": [], "insomnia-scripting-environment": [ "packages/insomnia-scripting-environment/src/objects/collection.ts -> packages/insomnia-scripting-environment/src/objects/response.ts -> packages/insomnia/src/network/network.ts -> packages/insomnia-scripting-environment/src/objects/index.ts", "packages/insomnia-scripting-environment/src/objects/collection.ts -> packages/insomnia-scripting-environment/src/objects/response.ts -> packages/insomnia/src/network/network.ts -> packages/insomnia/src/common/render.ts -> packages/insomnia/src/common/templating/mask-or-decrypt-vault-data.ts -> packages/insomnia/src/common/utils/vault.ts -> packages/insomnia/src/runtimes/index.ts -> packages/insomnia/src/runtimes/types.ts -> packages/insomnia-scripting-environment/src/objects/index.ts",