fix(app): export Filesystem/Share from build:web native stubs - #12936
Conversation
build:web failed with 'Filesystem is not exported by native-stub:@capacitor/filesystem'
because src/ios-attachment-smoke.ts statically imports { Filesystem } from
@capacitor/filesystem and { Share } from @capacitor/share, but the native-module
stub plugin emitted only a default noop object for those packages.
Add explicit named Filesystem/Share exports to the stubs. Every method throws
(mobile-only) rather than silently no-opping, so a web/desktop code path can
never fabricate a successful file write or share.
Also export isSensitiveKeyName/redactLogArgs from core index.browser.ts
(superseded by develop's export * from ./security/redact — resolved at merge).
…system-stub # Conflicts: # packages/core/src/index.browser.ts
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Local validation from isolated checkout Passed: bun run --cwd packages/app test -- test/native-module-stub-plugin.test.ts
git diff --check github-develop...HEADResult: focused app test passed (1 test). I also directly exercised bun -e 'import { nativeModuleStubPlugin } from "./packages/app/vite/native-module-stub-plugin.ts"; const p = nativeModuleStubPlugin({ command: "build", mode: "production" }); const plugin = Array.isArray(p) ? p[0] : p; for (const id of ["@capacitor/filesystem", "@capacitor/share"]) { const resolved = await plugin.resolveId.call({}, id, "/tmp/entry.ts", {}); const loaded = await plugin.load.call({}, resolved); if (id.includes("filesystem") && !loaded.includes("export const Filesystem")) throw new Error("missing Filesystem export"); if (id.includes("share") && !loaded.includes("export const Share")) throw new Error("missing Share export"); }'Result: both virtual modules resolved and loaded; Could not complete locally: bun run --cwd packages/app build:webIt fails before this stub path because the linked temp install cannot resolve |
lalalune
left a comment
There was a problem hiding this comment.
Reviewed the effective diff against current develop: only packages/app/vite/native-module-stub-plugin.ts changes. The added Filesystem and Share named exports match the static imports in src/ios-attachment-smoke.ts, and using throwing mobile-only stubs is the right failure mode for accidental web/desktop calls.
Local checks: git diff --check origin/develop...pr-12936 passed; bunx @biomejs/biome check packages/app/vite/native-module-stub-plugin.ts passed. I also retried bun run --cwd packages/app build:web after building packages/shared; it passed the original Capacitor export-resolution blocker, then hit unrelated local checkout/dist dependency issues (@elizaos/core dist/browser missing warnings and a three-vrm/three.webgpu mismatch), so I’m relying on the attached clean build evidence for the full app build.
|
Added a focused regression test for this fix in Local validation after applying the PR branch:
Local blockers not caused by this PR:
I did not run |
#12985) build:web failed with 'Filesystem is not exported by native-stub:@capacitor/filesystem' because src/ios-attachment-smoke.ts statically imports { Filesystem } from @capacitor/filesystem and { Share } from @capacitor/share, but the native-module stub plugin emitted only a default noop object for those packages. Add explicit named Filesystem/Share exports to the stubs. Every method throws (mobile-only) rather than silently no-opping, so a web/desktop code path can never fabricate a successful file write or share. Also export isSensitiveKeyName/redactLogArgs from core index.browser.ts (superseded by develop's export * from ./security/redact — resolved at merge). (cherry picked from commit a06ad22)
Problem
bun run --cwd packages/app build:webfailed with:src/ios-attachment-smoke.ts(the iOS attachment save/share smoke path shipped in #11009) statically imports{ Filesystem }from@capacitor/filesystemand{ Share }from@capacitor/share.main.tsxpulls it in statically (it only runs behind anisIOSgate), so the web bundle must still resolve those named exports — but the native-module stub plugin emitted only a default noop object for both packages.Fix
packages/app/vite/native-module-stub-plugin.ts: add explicit namedFilesystem/Shareexports to the web stubs for@capacitor/filesystemand@capacitor/share.Deliberately throwing stubs (Proxy where every method throws
mobile-only) rather than silent no-ops: a silent no-op here would fabricate a successful file write or share if a web/desktop code path ever called it.Verification
bun run --cwd packages/app build:webpasses clean on this branch merged with current develop (424 assets, chunk-safety verifier OK) — previously failed at the Rollup export-resolution step.index.browser.tsredact-export hunk was already superseded by develop'sexport * from "./security/redact"fix(#12229): route Feed A2A fetch through SSRF guard + converge log redaction #12572 follow-up and resolved away at merge).Finishing + shipping uncommitted sibling-agent work from the shared worktree.
— nubs-cloud [cloud-frontdoor]