mirror of
https://github.com/Kong/insomnia.git
synced 2026-11-03 19:45:33 -05:00
refactor: break main-process circular imports (git-service/plugin-window/plugin discovery)
## Background The main process carried three circular import groups that `scripts/circular-references/known-violations.json` had to record: - `main/git-service.ts` and `sync/git/repo-file-watcher.ts` imported each other because the `WorkspaceFileIssue` type lived in `git-service`, while the watcher that produces those issues imported the type back. - `main/plugin-window.ts` reached `main/window-utils.ts` for `getMainWindow`, both directly and through `main/prompt-bridge.ts`. - `main/templating-worker-database.ts` and `plugins/index.ts` imported each other because `plugins/index` lazily imported the sandbox-backed user-plugin export discovery from the main-only templating worker module. ## Changes - Move `WorkspaceFileIssue` to `sync/git/repo-file-watcher.ts`, which owns the file issues, and update the renderer type imports. - Add `main/window-registry.ts` to own `browserWindows` and `getMainWindow`, so `plugin-window`, `prompt-bridge` and `templating-db-auth` read the main window without importing `window-utils`. `window-utils` still writes the registry and drives the plugin window directly. - Add `common/templating/user-plugin-export-discovery.ts` as the registration slot for the sandbox-backed discovery. `main/templating-worker-database` registers its implementation at module load, and `plugins/index` calls the registered function in the main process while keeping the protocol path in renderers. Calling it without registration throws an explicit error. - Remove the five resolved cycles from `scripts/circular-references/known-violations.json`. - Register the discovery stub through the new hook in the plugin-load-order QuickJS test, and assert the main-process registration contract in the templating-worker-database test. ## Testing - `npm run check-cycle-references`: insomnia 13 -> 8 cycles, 0 new. - `npm run type-check -w insomnia` and eslint on the changed files: clean. - `npx vitest run` for the plugins suite, the templating-worker-database suites, plugin-window IPC authorization, and the window security/guard tests: 134 passed, 1 skipped. - `npx vitest run` for the repo-file-watcher and import suites: 75 passed. - `npm run build:electron-entrypoints`: succeeds. ## References Follows #10496 in the circular-reference cleanup chain.
This commit is contained in:
16 files changed
+108
-52
No files matched your search
@@ -0,0 +1,27 @@
|
||||
import type { PluginExportManifest } from '~/templating/sandbox/marshal';
|
||||
|
||||
// Shared registry for the sandbox-backed user-plugin export discovery. `plugins/index` runs in both
|
||||
// the main process and the renderer (plugin window) and must not statically import
|
||||
// `main/templating-worker-database` (the main-side implementation) — that module in turn loads
|
||||
// `plugins/index` for getPlugins/getTemplateTags, which would be a circular dependency. Keeping the
|
||||
// registration slot in this neutral module lets both sides depend on it without importing each other.
|
||||
type UserPluginExportDiscovery = (body: {
|
||||
directory: string;
|
||||
name: string;
|
||||
permissions?: { modules?: string[]; capabilities?: string[] };
|
||||
}) => Promise<PluginExportManifest>;
|
||||
|
||||
let discoverUserPluginExportsInMain: UserPluginExportDiscovery | null = null;
|
||||
|
||||
export function registerUserPluginExportDiscovery(discover: UserPluginExportDiscovery): void {
|
||||
discoverUserPluginExportsInMain = discover;
|
||||
}
|
||||
|
||||
export function getRegisteredUserPluginExportDiscovery(): UserPluginExportDiscovery {
|
||||
if (!discoverUserPluginExportsInMain) {
|
||||
throw new Error(
|
||||
'User plugin export discovery is not registered; sandboxed plugin discovery requires the Electron main process (main/templating-worker-database)',
|
||||
);
|
||||
}
|
||||
return discoverUserPluginExportsInMain;
|
||||
}
|
||||
@@ -40,7 +40,7 @@ vi.mock('electron', () => ({
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('../window-utils', () => ({
|
||||
vi.mock('../window-registry', () => ({
|
||||
getMainWindow: vi.fn(() => fakeMainWindow),
|
||||
}));
|
||||
|
||||
|
||||
@@ -46,10 +46,12 @@ vi.mock('../prompt-bridge', () => ({ requestPromptFromRenderer: vi.fn() }));
|
||||
vi.mock('../secure-read-file', () => ({ secureReadFile: vi.fn() }));
|
||||
|
||||
import { parsePluginPermissions } from '~/common/plugins/permissions';
|
||||
import { getRegisteredUserPluginExportDiscovery } from '~/common/templating/user-plugin-export-discovery';
|
||||
|
||||
import { requestPromptFromRenderer } from '../prompt-bridge';
|
||||
import {
|
||||
_testOnlyResetMigrationWarnings,
|
||||
discoverUserPluginExportsForLoader,
|
||||
getPluginEntrySource,
|
||||
maybeWarnMissingManifest,
|
||||
readPluginModuleMap,
|
||||
@@ -280,3 +282,9 @@ describe('resolveDbByKey — app.prompt', () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('user plugin export discovery registration', () => {
|
||||
it('registers the sandbox-backed discovery with the plugin loader', () => {
|
||||
expect(getRegisteredUserPluginExportDiscovery()).toBe(discoverUserPluginExportsForLoader);
|
||||
});
|
||||
});
|
||||
@@ -41,7 +41,7 @@ import {
|
||||
type ProviderEmail,
|
||||
type ProviderRepository,
|
||||
} from '~/sync/git/providers';
|
||||
import type { FileIssue, FileIssueKind } from '~/sync/git/repo-file-watcher';
|
||||
import type { FileIssue, WorkspaceFileIssue } from '~/sync/git/repo-file-watcher';
|
||||
|
||||
import { getAppVersion, INSOMNIA_GITLAB_API_URL } from '../common/constants';
|
||||
import { database } from '../common/database';
|
||||
@@ -184,14 +184,6 @@ export function vcsEventProperties(type: 'git', action: VCSAction, error?: strin
|
||||
return { type, action, error };
|
||||
}
|
||||
|
||||
export interface WorkspaceFileIssue {
|
||||
workspaceId: string;
|
||||
gitRepositoryId: string;
|
||||
relPath: string;
|
||||
kind: FileIssueKind;
|
||||
message: string;
|
||||
}
|
||||
|
||||
interface GetProjectGitFileIssuesOptions {
|
||||
projectId: string;
|
||||
workspaceId?: string;
|
||||
|
||||
@@ -2,7 +2,7 @@ import type { IpcMainInvokeEvent } from 'electron';
|
||||
|
||||
import { getPluginWindow } from '../plugin-window';
|
||||
import { getOrCreateTemplatingDbAuthToken } from '../templating-worker-database-auth';
|
||||
import { getMainWindow } from '../window-utils';
|
||||
import { getMainWindow } from '../window-registry';
|
||||
import { ipcMainHandle } from './electron';
|
||||
|
||||
// Only hands the templating db auth token to the main app window or the hidden plugin window.
|
||||
|
||||
@@ -4,7 +4,7 @@ import path from 'node:path';
|
||||
import { app, BrowserWindow, ipcMain, type IpcMainEvent, type IpcMainInvokeEvent } from 'electron';
|
||||
|
||||
import { requestPromptFromRenderer } from './prompt-bridge';
|
||||
import { getMainWindow } from './window-utils';
|
||||
import { getMainWindow } from './window-registry';
|
||||
|
||||
let pluginWindow: BrowserWindow | null = null;
|
||||
let windowReady = false;
|
||||
|
||||
@@ -4,7 +4,7 @@ import { ipcMain } from 'electron';
|
||||
|
||||
import type { AppPromptOptions } from '~/common/templating/types';
|
||||
|
||||
import { getMainWindow } from './window-utils';
|
||||
import { getMainWindow } from './window-registry';
|
||||
|
||||
export interface PromptRequestOptions extends AppPromptOptions {
|
||||
title: string;
|
||||
|
||||
@@ -20,6 +20,7 @@ import type {
|
||||
PluginTemplateTagContext,
|
||||
PluginToMainAPIPaths,
|
||||
} from '~/common/templating/types';
|
||||
import { registerUserPluginExportDiscovery } from '~/common/templating/user-plugin-export-discovery';
|
||||
import { getPluginCommonContext, getPlugins, getTemplateTags } from '~/plugins';
|
||||
import {
|
||||
HOOK_REQUEST_FIELDS,
|
||||
@@ -482,6 +483,12 @@ export const discoverUserPluginExportsForLoader = async (body: {
|
||||
});
|
||||
};
|
||||
|
||||
// Register the sandbox-backed user-plugin discovery with the plugin loader (plugins/index.ts) via
|
||||
// the shared discovery registry, which exists because plugins/index.ts can't import this
|
||||
// main-only module without creating a circular dependency. Loading this module in main always
|
||||
// precedes plugin discovery, since getPlugins/getTemplateTags live here.
|
||||
registerUserPluginExportDiscovery(discoverUserPluginExportsForLoader);
|
||||
|
||||
const pickHookRequestFields = (req: Record<string, any>): Record<string, any> => {
|
||||
const out: Record<string, any> = {};
|
||||
for (const key of HOOK_REQUEST_FIELDS) {
|
||||
|
||||
@@ -0,0 +1,11 @@
|
||||
import { type BrowserWindow } from 'electron';
|
||||
|
||||
// Holds the application's live BrowserWindow handles so modules that only need to read the main
|
||||
// window (`plugin-window`, `prompt-bridge`, `templating-db-auth`) don't have to import
|
||||
// `window-utils` — which, combined with `window-utils` driving the plugin window from this module,
|
||||
// would otherwise form a circular import.
|
||||
export const browserWindows = new Map<'Insomnia' | 'HiddenBrowserWindow', BrowserWindow>();
|
||||
|
||||
export function getMainWindow(): BrowserWindow | null {
|
||||
return browserWindows.get('Insomnia') ?? null;
|
||||
}
|
||||
@@ -30,16 +30,13 @@ import { ipcMainOn } from './ipc/electron';
|
||||
import { getLogDirectory } from './log';
|
||||
import { createPluginWindow, destroyPluginWindow, getPluginWindow } from './plugin-window';
|
||||
import { isTrustedAppOrigin } from './trusted-origin';
|
||||
import { browserWindows } from './window-registry';
|
||||
import { MAIN_WINDOW_SECURITY } from './window-security';
|
||||
|
||||
const DEFAULT_WIDTH = 1280;
|
||||
const DEFAULT_HEIGHT = 720;
|
||||
const MINIMUM_WIDTH = 500;
|
||||
const MINIMUM_HEIGHT = 400;
|
||||
const browserWindows = new Map<'Insomnia' | 'HiddenBrowserWindow', ElectronBrowserWindow>();
|
||||
export function getMainWindow(): ElectronBrowserWindow | null {
|
||||
return browserWindows.get('Insomnia') ?? null;
|
||||
}
|
||||
let hiddenWindowIsBusy = false;
|
||||
|
||||
// Every open window has its own copy of the menu, so keep them all in sync with the Debug Mode setting.
|
||||
|
||||
+29
-22
@@ -22,32 +22,39 @@ import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
|
||||
import { services } from 'insomnia-data';
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
import { afterEach, describe, expect, it } from 'vitest';
|
||||
|
||||
vi.mock('~/main/templating-worker-database', () => ({
|
||||
discoverUserPluginExportsForLoader: async () => {
|
||||
// Stands in for the real function's eventual `await import('../templating/sandbox/
|
||||
// plugin-tag-sandbox')` → `getQuickJSModule()` call chain, without needing to also mock
|
||||
// `buildSandboxBridge`/`readPluginModuleMap`/`electron.app.getVersion()` for no added
|
||||
// evidentiary value — the property under test is what `getQuickJSModule()` itself resolves to,
|
||||
// not the rest of real discovery's plumbing.
|
||||
const { getQuickJSModule } = await import('~/templating/sandbox/quickjs-runtime');
|
||||
(globalThis as any).__sandboxedPluginResolvedQuickJSModule = await getQuickJSModule();
|
||||
return {
|
||||
templateTags: [],
|
||||
requestHooks: 0,
|
||||
responseHooks: 0,
|
||||
requestActions: [],
|
||||
requestGroupActions: [],
|
||||
workspaceActions: [],
|
||||
documentActions: [],
|
||||
themes: [],
|
||||
};
|
||||
},
|
||||
}));
|
||||
// plugins/index.ts no longer dynamically imports `~/main/templating-worker-database`; the main
|
||||
// process now hands it the sandbox-backed discovery via the shared `registerUserPluginExportDiscovery`
|
||||
// registry. Mock that module would never register anything, so register a stub discovery through the
|
||||
// real registration hook instead — standing in for the real function's eventual
|
||||
// `await import('../templating/sandbox/plugin-tag-sandbox')` → `getQuickJSModule()` call chain,
|
||||
// without needing to also mock `buildSandboxBridge`/`readPluginModuleMap`/`electron.app.getVersion()`
|
||||
// for no added evidentiary value — the property under test is what `getQuickJSModule()` itself
|
||||
// resolves to, not the rest of real discovery's plumbing.
|
||||
import { registerUserPluginExportDiscovery } from '~/common/templating/user-plugin-export-discovery';
|
||||
|
||||
import { _testOnlySetPlugins, getPlugins } from '../index';
|
||||
|
||||
registerUserPluginExportDiscovery(async () => {
|
||||
// Deliberately dynamic: this test's premise is that the process's first-ever
|
||||
// `quickjs-emscripten` resolution happens *during* the sandboxed sibling's discovery call, so a
|
||||
// static import would defeat what's being proven. The assignment below avoids a cast; the test
|
||||
// assertions read the flag back off `globalThis`.
|
||||
const { getQuickJSModule } = await import('~/templating/sandbox/quickjs-runtime');
|
||||
Object.assign(globalThis, { __sandboxedPluginResolvedQuickJSModule: await getQuickJSModule() });
|
||||
return {
|
||||
templateTags: [],
|
||||
requestHooks: 0,
|
||||
responseHooks: 0,
|
||||
requestActions: [],
|
||||
requestGroupActions: [],
|
||||
workspaceActions: [],
|
||||
documentActions: [],
|
||||
themes: [],
|
||||
};
|
||||
});
|
||||
|
||||
const originalProcessType = process.type;
|
||||
|
||||
const writePluginFolder = (baseDir: string, folderName: string, pluginName: string, indexJs: string) => {
|
||||
|
||||
@@ -21,6 +21,7 @@ import type {
|
||||
} from '~/common/plugins/types';
|
||||
import { fetchFromTemplateWorkerDatabase } from '~/common/templating/liquid-extension-worker';
|
||||
import type { PluginTemplateTag, RenderPurpose } from '~/common/templating/types';
|
||||
import { getRegisteredUserPluginExportDiscovery } from '~/common/templating/user-plugin-export-discovery';
|
||||
import type { ActionDescriptor, PluginExportManifest } from '~/templating/sandbox/marshal';
|
||||
|
||||
import { getAppBundlePlugins, isDevelopment } from '../common/constants';
|
||||
@@ -63,7 +64,9 @@ export async function init() {
|
||||
// main process (via getTemplateTags in templating-worker-database) and the plugin window, so the
|
||||
// discovery has to reach the sandbox from either. In main we call the sandbox directly — the
|
||||
// `insomnia-templating-worker-database://` protocol is a renderer<->main channel and main's own
|
||||
// `fetch` can't resolve it. In a renderer (the plugin window) we go over that protocol.
|
||||
// `fetch` can't resolve it. In a renderer (the plugin window) we go over that protocol. In main the
|
||||
// implementation is provided by `main/templating-worker-database`, registered through the shared
|
||||
// `user-plugin-export-discovery` module — this file can't import that main module without a cycle.
|
||||
async function discoverUserPluginExports(
|
||||
directory: string,
|
||||
name: string,
|
||||
@@ -73,8 +76,7 @@ async function discoverUserPluginExports(
|
||||
if (__IS_RENDERER__) {
|
||||
return (await fetchFromTemplateWorkerDatabase('plugin.discoverUserPluginExports', body)) as PluginExportManifest;
|
||||
}
|
||||
const { discoverUserPluginExportsForLoader } = await import('~/main/templating-worker-database');
|
||||
return discoverUserPluginExportsForLoader(body);
|
||||
return getRegisteredUserPluginExportDiscovery()(body);
|
||||
}
|
||||
|
||||
function buildUserPluginModuleFromManifest(pluginName: string, manifest: PluginExportManifest): Plugin['module'] {
|
||||
|
||||
@@ -44,8 +44,6 @@ import path from 'node:path';
|
||||
import type { BaseModel, Workspace, WorkspaceMeta } from 'insomnia-data';
|
||||
import { models, services } from 'insomnia-data';
|
||||
|
||||
import type { WorkspaceFileIssue } from '~/main/git-service';
|
||||
|
||||
import { database as db } from '../../common/database';
|
||||
import { InsomniaFileTypeValues } from '../../common/import-v5-parser';
|
||||
import { getInsomniaV5DataExport, tryImportV5Data } from '../../common/insomnia-v5';
|
||||
@@ -70,6 +68,19 @@ export interface FileIssue {
|
||||
message: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* A file issue scoped to its workspace and git repository, as reported over IPC to the
|
||||
* renderer. Lives here (not in `main/git-service`) to avoid a circular dependency:
|
||||
* git-service loads this module for the watcher registry.
|
||||
*/
|
||||
export interface WorkspaceFileIssue {
|
||||
workspaceId: string;
|
||||
gitRepositoryId: string;
|
||||
relPath: string;
|
||||
kind: FileIssueKind;
|
||||
message: string;
|
||||
}
|
||||
|
||||
export interface FileProblemsChangedPayload {
|
||||
repoId: string;
|
||||
problems: FileIssue[];
|
||||
|
||||
@@ -5,7 +5,7 @@ import React, { useCallback } from 'react';
|
||||
import { Button, GridListItem } from 'react-aria-components';
|
||||
|
||||
import { scopeToBgColorMap, scopeToIconMap, scopeToTextColorMap } from '~/common/get-workspace-label';
|
||||
import type { WorkspaceFileIssue } from '~/main/git-service';
|
||||
import type { WorkspaceFileIssue } from '~/sync/git/repo-file-watcher';
|
||||
import { scrollElementIntoView } from '~/ui/utils';
|
||||
|
||||
import { type BaseTab, type TabType, useInsomniaTabContext } from '../../context/app/insomnia-tab-context';
|
||||
|
||||
@@ -10,8 +10,7 @@ import React, {
|
||||
} from 'react';
|
||||
|
||||
import { invariant } from '~/common/utils/invariant';
|
||||
import type { WorkspaceFileIssue } from '~/main/git-service';
|
||||
import type { FileProblemsChangedPayload } from '~/sync/git/repo-file-watcher';
|
||||
import type { FileProblemsChangedPayload, WorkspaceFileIssue } from '~/sync/git/repo-file-watcher';
|
||||
|
||||
const mapIssuesByWorkspaceId = (issues: WorkspaceFileIssue[]) => {
|
||||
return Object.fromEntries(issues.map(issue => [issue.workspaceId, issue])) as Record<string, WorkspaceFileIssue>;
|
||||
|
||||
@@ -2,11 +2,6 @@
|
||||
"insomnia": [
|
||||
"packages/insomnia/src/common/import.ts -> packages/insomnia/src/common/insomnia-v5.ts",
|
||||
"packages/insomnia/src/common/render.ts -> packages/insomnia/src/network/network.ts",
|
||||
"packages/insomnia/src/main/git-service.ts -> packages/insomnia/src/sync/git/repo-file-watcher.ts",
|
||||
"packages/insomnia/src/main/plugin-window.ts -> packages/insomnia/src/main/prompt-bridge.ts -> packages/insomnia/src/main/window-utils.ts",
|
||||
"packages/insomnia/src/main/plugin-window.ts -> packages/insomnia/src/main/window-utils.ts",
|
||||
"packages/insomnia/src/main/templating-worker-database.ts -> packages/insomnia/src/plugins/index.ts",
|
||||
"packages/insomnia/src/main/templating-worker-database.ts -> packages/insomnia/src/templating/index.ts -> packages/insomnia/src/plugins/index.ts",
|
||||
"packages/insomnia/src/sync/git/providers/github.ts -> packages/insomnia/src/sync/git/utils.ts -> packages/insomnia/src/sync/git/providers/index.ts",
|
||||
"packages/insomnia/src/sync/git/providers/gitlab.ts -> packages/insomnia/src/sync/git/utils.ts -> packages/insomnia/src/sync/git/providers/index.ts",
|
||||
"packages/insomnia/src/ui/components/modals/export-requests-modal.tsx -> packages/insomnia/src/ui/components/settings/import-export.tsx",
|
||||
|
||||
Reference in new issue
Block a user