tool-switch: create the missing counterpart file instead of silently no-opping
Tools → "Switch to PCB Editor" in a project with no .kicad_pcb (e.g. created from a lone schematic) did nothing: the nav hook found no file for the target tool and returned false. Now a session that can persist (ToolPage passes the new createFile prop) writes the templated counterpart via createProjectFileIfMissing — no download fallback, and it re-checks existence so a collaborator's file is never clobbered — then navigates to it, matching native KiCad (pcbnew opens a new board at the derived path). Sessions that can't persist (read-only viewers, scratch/local-folder) keep the logged no-op. Also latch the quit dispatcher off before every deliberate tool-switch navigation (markDeliberateNavigation): the wx port's UnloadCallback runs on BEFOREUNLOAD and closes the top frame the moment the navigation starts, so the quit hook history.back()'d over the in-flight navigation — the pagehide latch is too late (it only fires at commit time). e2e: new tests/web/tool-switch-missing-file.spec.ts reproduces the flow via a browser-local (IDB) project created from the home page; playwright-web config gains VITE_LOCAL_PROJECTS=idb and derives STANDALONE_PORT/CORS_ORIGIN from WEB_APP_URL (runs the suite past a squatted :3048); both vars declared in web/turbo.json globalEnv (turbo strict-env strips undeclared vars). Includes the previously-uncommitted tool-switch spec repairs (URL grammar + z-30 boot-overlay wait). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
f8ab901772
commit
2652d6fa81
7 changed files with 215 additions and 10 deletions
|
|
@ -6,7 +6,7 @@ import { defineConfig, devices } from '@playwright/test';
|
|||
* playwright-kicad.config.ts which drives the standalone tool harness HTMLs
|
||||
* under tests/apps/kicad.
|
||||
*
|
||||
* These tests exercise the real web open paths: navigate to /p/<project>/<tool>/<file>,
|
||||
* These tests exercise the real web open paths: navigate to /:scope/projects/:name/<file>,
|
||||
* let WasmTool boot the tool in-document (boot.ts), drive the project into MEMFS
|
||||
* and auto-open the file (open-flow.ts via Module.kicadOpenFile), and assert the
|
||||
* editor loaded wizard-free.
|
||||
|
|
@ -21,6 +21,15 @@ import { defineConfig, devices } from '@playwright/test';
|
|||
*/
|
||||
|
||||
const FRONTEND_URL = process.env.WEB_APP_URL ?? 'http://localhost:3048';
|
||||
// Keep the cold-started stack consistent with WEB_APP_URL overrides (e.g. a
|
||||
// sibling worktree squatting :3048): vite binds the URL's port and the backend
|
||||
// allows that origin, so overriding one env var relocates the whole frontend.
|
||||
const FRONTEND_PORT = new URL(FRONTEND_URL).port || '3048';
|
||||
// Backend counterpart — same override story for :3060 squatters. BACKEND_URL
|
||||
// is the var global-setup-web.ts already probes; the cold-started reference
|
||||
// backend binds its port and the editor is pointed at it.
|
||||
const BACKEND_URL = process.env.BACKEND_URL ?? 'http://localhost:3060';
|
||||
const BACKEND_PORT = new URL(BACKEND_URL).port || '3060';
|
||||
|
||||
export default defineConfig({
|
||||
globalSetup: './web/global-setup-web.ts',
|
||||
|
|
@ -73,8 +82,15 @@ export default defineConfig({
|
|||
...process.env,
|
||||
// resolved against web/backend/ (the backend's cwd)
|
||||
PROJECT_DIR: '../../tests/fixtures/demo',
|
||||
VITE_API_BASE_URL: 'http://localhost:3060',
|
||||
CORS_ORIGIN: 'http://localhost:3048',
|
||||
PORT: BACKEND_PORT,
|
||||
VITE_API_BASE_URL: BACKEND_URL,
|
||||
CORS_ORIGIN: FRONTEND_URL,
|
||||
STANDALONE_PORT: FRONTEND_PORT,
|
||||
// The missing-file tool-switch spec builds a browser-local (IDB) project
|
||||
// through the home page — same flag the dev/demo stacks set
|
||||
// (scripts/dev-gpl.mjs). Only effective on cold starts: with
|
||||
// reuseExistingServer an already-running stack must have set it itself.
|
||||
VITE_LOCAL_PROJECTS: 'idb',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
|
|
|||
79
tests/web/tool-switch-missing-file.spec.ts
Normal file
79
tests/web/tool-switch-missing-file.spec.ts
Normal file
|
|
@ -0,0 +1,79 @@
|
|||
import { test, expect, type Page } from '@playwright/test';
|
||||
import { clickMenuBarItem, clickMenuItemByText, stableShot } from '../e2e/utils/element-tracker';
|
||||
|
||||
/**
|
||||
* Tool-switch with a MISSING counterpart: eeschema Tools → "Switch to PCB
|
||||
* Editor" in a project that has no .kicad_pcb must CREATE the templated board
|
||||
* and navigate to it (native KiCad opens pcbnew on a new board at the derived
|
||||
* path) — not silently no-op (the old behavior: the WasmTool nav hook logged
|
||||
* "[nav] no project file found" and returned false).
|
||||
*
|
||||
* The backend demo fixture carries both files, so this spec builds a
|
||||
* schematic-only project the way a user does: home page Tools grid →
|
||||
* "Schematic Editor" → NewFileDialog (project "Untitled" → slug "untitled",
|
||||
* file "main.kicad_sch") → a browser-local IndexedDB project under the @local
|
||||
* scope. That flow only renders when the standalone runs with
|
||||
* VITE_LOCAL_PROJECTS=idb (playwright-web.config.ts sets it for cold starts;
|
||||
* a hand-started stack must set it too — the dialog assertion below names the
|
||||
* flag so a mis-configured stack fails self-diagnosingly). A fresh Playwright
|
||||
* context has an empty IDB, so the slug is deterministically "untitled".
|
||||
*/
|
||||
|
||||
async function waitForToolReady(page: Page, titleRe: RegExp): Promise<void> {
|
||||
await expect(page.locator('#canvas')).toBeVisible({ timeout: 120000 });
|
||||
await expect
|
||||
.poll(() => page.title(), {
|
||||
message: `editor never reached title ${titleRe}`,
|
||||
timeout: 120000,
|
||||
intervals: [1000],
|
||||
})
|
||||
.toMatch(titleRe);
|
||||
// The menu helpers drive the rendered-element registry.
|
||||
await page.waitForFunction(
|
||||
() =>
|
||||
!!(window as unknown as { wxElementRegistry?: { findAllRendered?: unknown } })
|
||||
.wxElementRegistry,
|
||||
null,
|
||||
{ timeout: 30000 }
|
||||
);
|
||||
// The boot and eager-library overlays (WasmTool, `absolute inset-0 z-30`)
|
||||
// cover the whole editor including the menubar — synthetic menu clicks land
|
||||
// on them until they clear (eeschema hydrates the full symbol set post-boot).
|
||||
await expect(page.locator('div.absolute.inset-0.z-30')).toHaveCount(0, {
|
||||
timeout: 180000,
|
||||
});
|
||||
}
|
||||
|
||||
test.describe('web app — tool switch creates the missing counterpart', () => {
|
||||
test('eeschema → Switch to PCB Editor creates and opens main.kicad_pcb', async ({ page }) => {
|
||||
test.setTimeout(420000); // two full wasm boots
|
||||
|
||||
// Home → Tools grid: with the local store on, launching a document tool
|
||||
// opens the new-file dialog (HomePage onLaunch → setNewFileTool).
|
||||
await page.goto('/');
|
||||
await page.getByRole('button', { name: 'Schematic Editor', exact: true }).click();
|
||||
await expect(
|
||||
page.locator('#newfile-projectname'),
|
||||
'new-file dialog must open — is the stack running with VITE_LOCAL_PROJECTS=idb?'
|
||||
).toBeVisible();
|
||||
// The defaults are what the URL assertions below rely on (and their
|
||||
// presence doubles as a dialog-hydrated wait).
|
||||
await expect(page.locator('#newfile-projectname')).toHaveValue('Untitled');
|
||||
await expect(page.locator('#newfile-name')).toHaveValue('main.kicad_sch');
|
||||
await page.getByRole('button', { name: 'Create & open' }).click();
|
||||
|
||||
// NewFileDialog persists to IDB, then hard-navigates to the file route.
|
||||
await page.waitForURL(/\/@local\/projects\/untitled\/main\.kicad_sch/, { timeout: 30000 });
|
||||
await waitForToolReady(page, /main — Schematic Editor/i);
|
||||
|
||||
// The switch. Before the fix nothing happens (no navigation, no dialog),
|
||||
// so the waitForURL below is where this spec fails.
|
||||
expect(await clickMenuBarItem(page, 'Tools'), 'Tools menubar item clickable').toBe(true);
|
||||
await clickMenuItemByText(page, 'Switch to PCB Editor');
|
||||
|
||||
await page.waitForURL(/\/@local\/projects\/untitled\/main\.kicad_pcb/, { timeout: 30000 });
|
||||
await waitForToolReady(page, /main — PCB Editor/i);
|
||||
|
||||
await stableShot(page, 'web-switch-missing-pcb-created.png');
|
||||
});
|
||||
});
|
||||
|
|
@ -8,8 +8,9 @@ import { clickMenuBarItem, clickMenuItemByText, stableShot } from '../e2e/utils/
|
|||
* Native KiCad spawns a process for this via ExecuteFile (common/gestfich.cpp);
|
||||
* the WASM build delegates to window.kicadWebOpenTool (WasmTool.tsx), which
|
||||
* maps the MEMFS file path to the project-relative file and calls
|
||||
* location.assign(/p/demo/<tool>/<file>). Each direction is a full page
|
||||
* navigation followed by a fresh wasm boot — hence the generous timeouts.
|
||||
* location.assign(/:scope/projects/:name/<file>) — the tool is inferred from
|
||||
* the file extension. Each direction is a full page navigation followed by a
|
||||
* fresh wasm boot — hence the generous timeouts.
|
||||
*/
|
||||
|
||||
async function waitForToolReady(page: Page, titleRe: RegExp): Promise<void> {
|
||||
|
|
@ -29,6 +30,12 @@ async function waitForToolReady(page: Page, titleRe: RegExp): Promise<void> {
|
|||
null,
|
||||
{ timeout: 30000 }
|
||||
);
|
||||
// The boot and eager-library overlays (WasmTool, `absolute inset-0 z-30`)
|
||||
// cover the whole editor including the menubar — synthetic menu clicks land
|
||||
// on them until they clear (eeschema hydrates the full symbol set post-boot).
|
||||
await expect(page.locator("div.absolute.inset-0.z-30")).toHaveCount(0, {
|
||||
timeout: 180000,
|
||||
});
|
||||
}
|
||||
|
||||
async function switchTool(
|
||||
|
|
@ -54,7 +61,7 @@ test.describe('web app — tool switching', () => {
|
|||
await switchTool(
|
||||
page,
|
||||
'Switch to PCB Editor',
|
||||
/\/p\/demo\/pcbnew\/demo\.kicad_pcb/,
|
||||
/\/default\/projects\/demo\/demo\.kicad_pcb/,
|
||||
/demo — PCB Editor/i
|
||||
);
|
||||
|
||||
|
|
@ -70,7 +77,7 @@ test.describe('web app — tool switching', () => {
|
|||
await switchTool(
|
||||
page,
|
||||
'Switch to Schematic Editor',
|
||||
/\/p\/demo\/eeschema\/demo\.kicad_sch/,
|
||||
/\/default\/projects\/demo\/demo\.kicad_sch/,
|
||||
/demo — Schematic Editor/i
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ import {
|
|||
yjsProviderConfig,
|
||||
type DocSource,
|
||||
} from "@/lib/config";
|
||||
import { defaultFileName, newFileTemplate, withExtension } from "@/lib/new-file";
|
||||
import { bootKicadTool } from "@/wasm/boot";
|
||||
import { resolveWasmBase } from "@/wasm/wasm-assets";
|
||||
import {
|
||||
|
|
@ -231,9 +232,16 @@ function installToolNavigationHook(
|
|||
slug: string;
|
||||
files: ToolFile[];
|
||||
targetPath?: string;
|
||||
/** Persist a new file into the project (see the WasmTool prop). Absent ⇒
|
||||
* this session can't create one, and a missing target stays a no-op. */
|
||||
createFile?: (relPath: string, bytes: Uint8Array) => Promise<void>;
|
||||
log: (m: string) => void;
|
||||
},
|
||||
): () => void {
|
||||
// One create at a time: a double-fired menu item must not upload twice.
|
||||
// Cleared only on failure — success navigates the page away.
|
||||
let pendingCreate: string | null = null;
|
||||
|
||||
const hook = (rawToolName: string, rawFileName: string): boolean => {
|
||||
const nextTool = normalizeToolName(rawToolName);
|
||||
|
||||
|
|
@ -248,8 +256,48 @@ function installToolNavigationHook(
|
|||
: chooseToolFile(opts.files, nextTool, requestedPath, opts.targetPath);
|
||||
|
||||
if (!FILELESS_TOOLS.has(nextTool) && !nextPath) {
|
||||
opts.log(`[nav] no project file found for ${nextTool}: ${rawFileName}`);
|
||||
return false;
|
||||
// Native KiCad's "Switch to PCB Editor" with no board opens pcbnew on a
|
||||
// NEW empty board at the derived path — mirror it by creating the
|
||||
// templated counterpart in the project (the shape NewFileDialog writes)
|
||||
// and navigating to it. Only sessions that can persist pass `createFile`
|
||||
// (ToolPage); viewers and scratch/local-folder sessions keep the quiet
|
||||
// no-op. C++ calls this hook synchronously (EM_ASM_INT) and ignores the
|
||||
// result beyond a log line, so the create+navigate runs async and we
|
||||
// answer true optimistically once it's kicked off.
|
||||
const createFile = opts.createFile;
|
||||
if (!createFile) {
|
||||
opts.log(`[nav] no project file found for ${nextTool}: ${rawFileName}`);
|
||||
return false;
|
||||
}
|
||||
if (pendingCreate) {
|
||||
opts.log(`[nav] create already pending: ${pendingCreate}`);
|
||||
return true;
|
||||
}
|
||||
const relPath =
|
||||
requestedPath ??
|
||||
(opts.targetPath
|
||||
? withExtension(nextTool, fileStem(opts.targetPath))
|
||||
: defaultFileName(nextTool));
|
||||
const url =
|
||||
projectPath(currentScope(), opts.slug, relPath) + win.location.search;
|
||||
pendingCreate = relPath;
|
||||
void (async () => {
|
||||
try {
|
||||
const bytes = new TextEncoder().encode(
|
||||
newFileTemplate(nextTool, crypto.randomUUID()),
|
||||
);
|
||||
await createFile(relPath, bytes);
|
||||
opts.log(`[nav] created missing ${nextTool} file ${relPath} -> ${url}`);
|
||||
markDeliberateNavigation();
|
||||
win.location.assign(url);
|
||||
} catch (e) {
|
||||
pendingCreate = null;
|
||||
opts.log(
|
||||
`[nav] create failed for ${relPath}: ${e instanceof Error ? e.message : String(e)}`,
|
||||
);
|
||||
}
|
||||
})();
|
||||
return true;
|
||||
}
|
||||
|
||||
// Scope/kind/name grammar: a fileless tool boots at `…/-/:tool`; a file route
|
||||
|
|
@ -261,6 +309,7 @@ function installToolNavigationHook(
|
|||
: projectPath(scope, opts.slug, nextPath)) + win.location.search;
|
||||
|
||||
opts.log(`[nav] ${rawToolName} ${rawFileName || "(no file)"} -> ${url}`);
|
||||
markDeliberateNavigation();
|
||||
win.location.assign(url);
|
||||
return true;
|
||||
};
|
||||
|
|
@ -286,6 +335,20 @@ function installToolNavigationHook(
|
|||
let activeQuitHook: (() => void) | undefined;
|
||||
let quitHandled = false;
|
||||
|
||||
/**
|
||||
* Latch the quit dispatcher off ahead of a deliberate in-app navigation (the
|
||||
* tool-switch hook's location.assign). The wx port's UnloadCallback runs on
|
||||
* BEFOREUNLOAD — i.e. the instant the navigation starts, while this document
|
||||
* keeps running until the next one commits — and closes the top frame, which
|
||||
* fires wxAppTopWindowClosed. Without the latch the quit hook then
|
||||
* history.back()s over the in-flight navigation (the pagehide latch below is
|
||||
* too late: pagehide only fires at commit time). One-shot per document, same
|
||||
* as the pagehide latch — this page is on its way out.
|
||||
*/
|
||||
function markDeliberateNavigation() {
|
||||
quitHandled = true;
|
||||
}
|
||||
|
||||
const quitDispatcher = () => {
|
||||
if (quitHandled) return;
|
||||
quitHandled = true;
|
||||
|
|
@ -686,6 +749,7 @@ export function WasmTool({
|
|||
targetPath,
|
||||
fetchBytes,
|
||||
saveBytes,
|
||||
createFile,
|
||||
docSource,
|
||||
assetBaseUrl,
|
||||
libsSource,
|
||||
|
|
@ -718,6 +782,15 @@ export function WasmTool({
|
|||
* MEMFS-only (e.g. Y.Doc-backed sessions).
|
||||
*/
|
||||
saveBytes?: SaveBytes;
|
||||
/**
|
||||
* Create a new file in the project (tool-switch auto-create: eeschema's
|
||||
* "Switch to PCB Editor" when no board exists yet). Persisted BEFORE the
|
||||
* hook navigates, so the next ToolPage load finds it. Omit for sessions
|
||||
* that can't persist a new project file (read-only viewers, scratch and
|
||||
* local-folder sessions) — a missing switch target then stays a logged
|
||||
* no-op.
|
||||
*/
|
||||
createFile?: (relPath: string, bytes: Uint8Array) => Promise<void>;
|
||||
/**
|
||||
* Where this project's DOCUMENT lives (see lib/config docSourceConfig):
|
||||
* "ydoc" materializes the target file from its collab room when the room has
|
||||
|
|
@ -953,6 +1026,7 @@ export function WasmTool({
|
|||
slug,
|
||||
files,
|
||||
targetPath,
|
||||
createFile,
|
||||
log: append,
|
||||
});
|
||||
|
||||
|
|
@ -969,7 +1043,7 @@ export function WasmTool({
|
|||
removeNavigationHook();
|
||||
removeQuitHook();
|
||||
};
|
||||
}, [slug, files, targetPath, append]);
|
||||
}, [slug, files, targetPath, createFile, append]);
|
||||
|
||||
React.useEffect(() => {
|
||||
// Guard re-entry: the WASM runtime is process-global and must boot exactly
|
||||
|
|
|
|||
|
|
@ -108,6 +108,27 @@ export async function uploadFileBytes(
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a project file that a tool switch found missing (WasmTool's nav
|
||||
* hook): write `bytes` at `relPath` unless the file already exists on the
|
||||
* source — the hook's file list is a mount-time snapshot, and a collaborator
|
||||
* may have created the file since (never clobber it with an empty template).
|
||||
* Unlike `uploadFileBytes` there is deliberately NO download fallback: a
|
||||
* read-only source rejects (uploader absent, or the composite's per-slug
|
||||
* ReadOnlyProjectError) and the caller keeps the editor where it is.
|
||||
*/
|
||||
export async function createProjectFileIfMissing(
|
||||
slug: string,
|
||||
relPath: string,
|
||||
bytes: Uint8Array,
|
||||
): Promise<void> {
|
||||
const source = projectSource();
|
||||
if (!source.uploadFileBytes) throw new ReadOnlyProjectError(slug);
|
||||
const { files } = await source.getProject(slug);
|
||||
if (files.some((file) => file.path === relPath)) return;
|
||||
await source.uploadFileBytes(slug, relPath, bytes);
|
||||
}
|
||||
|
||||
// --- collaboration drift reporting (ysync; backend-only) ---
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import { useParams, useSearchParams } from "react-router-dom";
|
||||
import { parseToolParam, toolForFile, type Tool } from "@pcbjam/shared";
|
||||
import {
|
||||
createProjectFileIfMissing,
|
||||
fetchFileBytes,
|
||||
uploadFileBytes,
|
||||
useProject,
|
||||
|
|
@ -80,6 +81,11 @@ export function ToolPage() {
|
|||
? undefined
|
||||
: (relPath, bytes) => uploadFileBytes(slug, relPath, bytes)
|
||||
}
|
||||
createFile={
|
||||
readOnly
|
||||
? undefined
|
||||
: (relPath, bytes) => createProjectFileIfMissing(slug, relPath, bytes)
|
||||
}
|
||||
docSource={docSource}
|
||||
sourceDescriptor={sourceDescriptor}
|
||||
readOnly={readOnly}
|
||||
|
|
|
|||
|
|
@ -2,10 +2,12 @@
|
|||
"$schema": "https://turbo.build/schema.json",
|
||||
"globalEnv": [
|
||||
"PORT",
|
||||
"STANDALONE_PORT",
|
||||
"PROJECT_DIR",
|
||||
"CORS_ORIGIN",
|
||||
"WASM_SRC_DIR",
|
||||
"VITE_API_BASE_URL",
|
||||
"VITE_LOCAL_PROJECTS",
|
||||
"VITE_WASM_ASSET_BASE_URL"
|
||||
],
|
||||
"tasks": {
|
||||
|
|
|
|||
Loading…
Reference in a new issue