feat: Added support for Gecko (firefox) by CWZMorro · Pull Request #7 · james-yap/openbanker

CWZMorro · 2025-11-29T09:43:27Z

No description provided.

james-yap

✅ Code review completed. Changes look solid — clean architecture for multi-browser support.

james-yap

Found several issues that need addressing before merge. See inline comments.

james-yap · 2026-05-10T17:20:42Z

Code Review — feat: Added support for Gecko (Firefox)

Verdict: COMMENT (no hard blockers, but a few things worth addressing before merge)

✅ What's Great

Clean manifest split — separating base/chrome/firefox manifests is the right approach. Easy to maintain and extend.
webextension-polyfill adoption — replacing raw chrome.* calls with browser.* via the polyfill is the idiomatic cross-browser approach.
Callback → Promise migration in useChromeStorage.ts — the old chrome.storage callback style is replaced with clean .then()/.catch() chains. More readable and consistent with modern async patterns.
Explicit Transaction[] cast in App.tsx — res[0].result is unknown in the polyfill types, so the explicit cast is correct and necessary.
strict_min_version: 109.0 — good lower bound; browser.scripting is available from Firefox 101 and the Manifest V2 background scripts field is supported, so this is safe.

⚠️ Issues Found

1. declare const browser: any is a type-safety bypass (utils.ts)

The getChromeContext() runtime check is valid, but declare const browser: any opts out of the polyfill's proper types. Since the rest of the codebase already imports browser from "webextension-polyfill", a simple typeof browser !== 'undefined' guard is all that's needed — no ambient declaration required. See inline comment.

2. No dev:firefox script

The dev script is hardcoded to TARGET=chrome. There's no way to spin up a Firefox dev build during development. Minor, but worth adding a dev:firefox counterpart.

3. Trailing space in manifest.firefox.json

Line 8 has a trailing space after the extension ID string. Cosmetic but worth a quick fix.

4. Shallow merge in mergeManifests (vite.config.ts)

{ ...base, ...specific } does a shallow merge. Fine for now since keys don't overlap at a nested level, but if a future manifest adds a nested key that exists in both base and specific, specific will silently clobber the entire nested object rather than merging it. A comment warning future contributors about this would help.

💡 Suggestions

webextension-polyfill in both frontend/package.json and chrome/package.json — Is the frontend package actually using the polyfill directly, or is it only needed in the extension context? If it's only called from within the extension wrapper, you might be able to keep it only in chrome/package.json. Not a blocker — just worth auditing.

Reviewed by Hermes Agent

james-yap · 2026-05-10T17:40:32Z

@@ -1,17 +1,19 @@
 import { clsx, type ClassValue } from "clsx"
 import { twMerge } from "tailwind-merge"
+// [FIX] Tell TypeScript that 'browser' is a valid global variable


⚠️ declare const browser: any bypasses the polyfill's type definitions. Since browser is already imported from webextension-polyfill in other files, this ambient declaration is not needed. The typeof browser !== 'undefined' runtime guard below works fine without it — just remove this line. If you need the types here, import them from webextension-polyfill directly.

james-yap · 2026-05-10T17:40:33Z

+  },
+  "browser_specific_settings": {
+    "gecko": {
+      "id": "openbanker@voidranjer", 


💡 Trailing space after the extension ID string — minor nit, worth cleaning up.

- Remove ambient declare const browser in utils.ts; use (globalThis as any).browser?.runtime?.id instead - Add dev:firefox script to package.json - Remove trailing space in manifest.firefox.json - Add shallow-merge warning comment in vite.config.ts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

CWZMorro · 2026-05-13T21:40:17Z

Thanks for the detailed review! Addressed all the points:

declare const browser: any — removed the ambient declaration and its comment. Rewrote the browser check using (globalThis as any).browser?.runtime?.id so it stays type-safe without bypassing the polyfill's types.
dev:firefox script — added alongside the existing dev script.
Trailing space in manifest.firefox.json — cleaned up.
Shallow merge comment — replaced // Merge them with a warning about the clobber behavior so future contributors are aware of the limitation.

Copilot

Pull request overview

This PR aims to add Firefox (Gecko) support for the browser extension by moving the frontend code to webextension-polyfill APIs and introducing per-browser manifest/build outputs in the extension package.

Changes:

Switched frontend extension interactions (storage/tabs/scripting) to webextension-polyfill and updated extension-context detection.
Added per-target build outputs (dist/chrome, dist/firefox) and build scripts to generate a merged manifest.json per target.
Introduced manifest.base.json plus browser-specific manifest overrides for Chrome and Firefox.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
packages/frontend/src/lib/utils.ts	Update extension-context detection to include Firefox (`browser.*`).
packages/frontend/src/hooks/useChromeStorage.ts	Replace callback-style `chrome.storage` usage with promise-based `webextension-polyfill` storage.
packages/frontend/src/App.tsx	Use `webextension-polyfill` for `tabs.query` and `scripting.executeScript`; add Transaction casting.
packages/frontend/package.json	Add `webextension-polyfill` + types.
packages/chrome/vite.config.ts	Add manifest merging + per-target output directories + manifest generation plugin.
packages/chrome/scripts/copy-frontend.js	Copy frontend build into per-target extension dist directory.
packages/chrome/package.json	Add `TARGET`-based dev/build scripts; add polyfill deps.
packages/chrome/manifest.base.json	Split shared manifest configuration into a base manifest.
packages/chrome/manifest.chrome.json	Chrome MV3 background service worker override.
packages/chrome/manifest.firefox.json	Firefox-specific manifest override (background + Gecko settings).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

CWZMorro · 2026-05-13T22:23:59Z

@@ -1,42 +1,62 @@
 import { defineConfig } from "vite";
 import { resolve } from "path";
+import fs from "fs";


Fixed — changed to import * as fs from "node:fs" in vite.config.ts.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

feat: Added support for Gecko (firefox)

186f5aa

james-yap approved these changes May 10, 2026

View reviewed changes

james-yap requested changes May 10, 2026

View reviewed changes

james-yap reviewed May 10, 2026

View reviewed changes

Comment thread packages/chrome/vite.config.ts

james-yap reviewed May 10, 2026

View reviewed changes

Comment thread packages/chrome/package.json

Copilot AI review requested due to automatic review settings May 13, 2026 21:40

Copilot started reviewing on behalf of CWZMorro May 13, 2026 21:40 View session

Copilot AI reviewed May 13, 2026

View reviewed changes

CWZMorro and others added 2 commits May 13, 2026 16:12

Potential fix for pull request finding

77427ef

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

fix(review): address pr feedback for using namespace import

0426ee1

Navigation Menu

feat: Added support for Gecko (firefox) - #7

feat: Added support for Gecko (firefox)#7
CWZMorro wants to merge 4 commits into
james-yap:mainfrom
CWZMorro:firefox

CWZMorro commented Nov 29, 2025

Uh oh!

james-yap left a comment

Uh oh!

james-yap left a comment

Uh oh!

james-yap commented May 10, 2026 •

edited

Loading

Uh oh!

james-yap May 10, 2026

Uh oh!

james-yap May 10, 2026

Uh oh!

Uh oh!

Uh oh!

CWZMorro commented May 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

CWZMorro May 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

CWZMorro commented Nov 29, 2025

Uh oh!

james-yap left a comment

Choose a reason for hiding this comment

Uh oh!

james-yap left a comment

Choose a reason for hiding this comment

Uh oh!

james-yap commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review — feat: Added support for Gecko (Firefox)

✅ What's Great

⚠️ Issues Found

💡 Suggestions

Uh oh!

james-yap May 10, 2026

Choose a reason for hiding this comment

Uh oh!

james-yap May 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

CWZMorro commented May 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

CWZMorro May 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

james-yap commented May 10, 2026 •

edited

Loading