Handle empty FM bridge with fallback shim#196
Conversation
- warn when FM MCP has no connected files - inject fallback bridge shim and runtime error logging - update types and tests
🦋 Changeset detectedLatest commit: 38c126e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@proofkit/better-auth
@proofkit/cli
create-proofkit
@proofkit/fmdapi
@proofkit/fmodata
@proofkit/typegen
@proofkit/webviewer
commit: |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughWhen the FM MCP responds but reports no connected files, the Vite FM bridge now emits a server-side warning, injects a fallback bridge shim into index.html, and the shim logs runtime errors if bridge methods are invoked before any file connects. Setup still fails when the MCP is unreachable/unhealthy. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Dev Server (Vite fmBridge)
participant MCP as FM MCP (connected-files endpoint)
participant Browser as Browser (index.html / runtime)
Dev->>MCP: GET /connected-files
alt MCP returns filename
MCP-->>Dev: { filename: "MyFile" }
Dev->>Browser: inject mock bridge script (head-prepend) referencing "MyFile"
else MCP returns no files
MCP-->>Dev: { files: [] } or payload with no usable filename
Dev-->>Dev: emit console.warn("no connected files")
Dev->>Browser: inject fallback shim script that stubs window.filemaker / window.FileMaker
end
Note right of Browser: At runtime, when app calls bridge methods
Browser->>Browser: call window.filemaker / FileMaker.PerformScript
alt fallback shim active (no connected file)
Browser-->>Browser: console.error("bridge used before any file connected") xN
else real bridge connected
Browser->>MCP: forward calls to connected file via bridge
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/webviewer/src/types.d.ts (1)
3-7: Consider addingFileMakerto the Window interface.The fallback shim in
buildNoConnectedFilesScriptTagalso injectswindow.FileMakerwithPerformScriptandPerformScriptWithOptionmethods. The test at line 279 accessesglobalThis.window.FileMaker?.PerformScript(...), butFileMakerisn't typed here.💡 Suggested type addition
interface Window { handleFmWVFetchCallback: (data: unknown, fetchId: string) => boolean; filemaker?: { (...args: unknown[]): unknown; performScript?: (...args: unknown[]) => unknown; performScriptWithOption?: (...args: unknown[]) => unknown; }; + FileMaker?: { + PerformScript?: (...args: unknown[]) => unknown; + PerformScriptWithOption?: (...args: unknown[]) => unknown; + }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webviewer/src/types.d.ts` around lines 3 - 7, Add a FileMaker definition to the global Window types so TypeScript knows about the fallback shim injected by buildNoConnectedFilesScriptTag: update packages/webviewer/src/types.d.ts to include a FileMaker property on the Window interface (or augment globalThis) that defines PerformScript and PerformScriptWithOption (and optional lowercase aliases if used) as callable methods; ensure the symbol names match what tests reference (window.FileMaker, PerformScript, PerformScriptWithOption) so globalThis.window.FileMaker?.PerformScript(...) is correctly typed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/webviewer/src/types.d.ts`:
- Around line 3-7: Add a FileMaker definition to the global Window types so
TypeScript knows about the fallback shim injected by
buildNoConnectedFilesScriptTag: update packages/webviewer/src/types.d.ts to
include a FileMaker property on the Window interface (or augment globalThis)
that defines PerformScript and PerformScriptWithOption (and optional lowercase
aliases if used) as callable methods; ensure the symbol names match what tests
reference (window.FileMaker, PerformScript, PerformScriptWithOption) so
globalThis.window.FileMaker?.PerformScript(...) is correctly typed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 273b0deb-39a6-460d-b49d-6b5a6bb87804
📒 Files selected for processing (4)
.changeset/fm-bridge-empty-connected-files.mdpackages/webviewer/src/fm-bridge.tspackages/webviewer/src/types.d.tspackages/webviewer/tests/vite-plugins.test.ts
- type `window.FileMaker` fallback bridge - keep webviewer typings aligned with bridge API
Summary
filemaker/FileMakershim that logs runtime errors on useTesting
packages/webviewer/tests/vite-plugins.test.tspnpm run cinot run in this branchSummary by CodeRabbit
New Features
Bug Fixes
Tests