troubleshooting guide#2688
troubleshooting guide#2688leofelix077 wants to merge 2 commits intofeature/freighter-best-practices-skillfrom
Conversation
- Remove Node version mismatch section (covered by quick-start + CONTRIBUTING) - Remove Playwright browser not found (e2e README prerequisite) - Remove snapshot tests section (covered by e2e README) - Remove stale Node 22/Node 20 CI contradiction (engines requires >=22) - Fix Webpack/Tailwind section header structure after CI section removal
leofelix077
changed the title
There was a problem hiding this comment.
Pull request overview
Updates the developer troubleshooting documentation for the Freighter extension and links it from the repo’s agent/index guidance so contributors (and internal agents) have a single reference for common setup/build/test pitfalls.
Changes:
- Add/refresh a comprehensive troubleshooting guide covering setup, build, testing, CI, MV3, and React 19 topics.
- Add the troubleshooting guide as an entry point in
AGENTS.md.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| docs/docs/troubleshooting-guide.md | Adds/updates the troubleshooting guide content for common dev issues across setup/build/tests/MV3. |
| AGENTS.md | Adds a “Troubleshooting” entry pointing to the new guide. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
docs/docs/troubleshooting-guide.md
Outdated
| # N10 — Troubleshooting Guide: Freighter Extension | ||
|
|
||
| _Last updated: 2026-04-08_ | ||
|
|
||
| Common issues and solutions when developing the Freighter browser extension. |
There was a problem hiding this comment.
This doc page doesn’t include the Docusaurus frontmatter block (id/title/slug). In this repo, docs pages consistently use frontmatter, and relying on an inline “# …” title (plus the “N10 —” prefix) can lead to inconsistent routing/title handling and makes it harder to add it to sidebars later. Consider adding frontmatter (and moving the title there) for consistency with the rest of docs.
docs/docs/troubleshooting-guide.md
Outdated
|
|
||
| 1. **Reduce parallelism:** Limit Playwright workers in `playwright.config.ts`: | ||
| ```ts | ||
| workers: 1; // or 2, depending on available RAM |
There was a problem hiding this comment.
The Playwright config snippet uses workers: 1; which is not valid object-literal syntax in TS/JS config files (it should be a comma-separated property). As written, readers copying this into playwright.config.ts will get a syntax error.
| workers: 1; // or 2, depending on available RAM | |
| workers: 1, // or 2, depending on available RAM |
docs/docs/troubleshooting-guide.md
Outdated
| ### IDE and ESLint plugins not loading with Husky | ||
|
|
||
| **Symptom:** Pre-commit hooks run successfully but ESLint errors are not caught | ||
| in the editor, or the hook exits early without running ESLint. | ||
|
|
||
| **Solution:** Ensure the ESLint VSCode/IDE extension is installed and | ||
| configured: | ||
|
|
||
| 1. **VS Code:** Install the | ||
| [ESLint extension](https://marketplace.visualstudio.com/items?itemName=dbaeumer.vscode-eslint). | ||
| This project uses ESLint flat config (`eslint.config.js`) — check your | ||
| extension version is ≥3.0.10 (flat config support). | ||
| 2. **WebStorm/IntelliJ:** Enable ESLint under Languages & Frameworks > | ||
| JavaScript | ||
| > Code Quality Tools > ESLint, and set "Automatic ESLint configuration". | ||
| 3. **Verify the extension sees the config:** Run ESLint manually to confirm it | ||
| picks up the project config before relying on IDE integration: | ||
| ```bash | ||
| npx eslint extension/src/popup/App.tsx | ||
| ``` | ||
| 4. **After confirming ESLint works**, verify Husky's pre-commit script is | ||
| executable: | ||
| ```bash | ||
| ls -la .husky/pre-commit # Should show -rwxr-xr-x | ||
| chmod +x .husky/pre-commit # Fix if not executable | ||
| ``` |
There was a problem hiding this comment.
This section suggests the pre-commit hook runs (or should run) ESLint, but the repo’s .husky/pre-commit currently runs translations + pretty-quick only (no eslint/lint-staged). Please adjust the troubleshooting text so it matches the actual hook behavior (or explicitly call out that ESLint is editor/CI-only unless you wire lint-staged into the hook).
docs/docs/troubleshooting-guide.md
Outdated
| - Use `chrome.alarms` for delayed operations instead | ||
| - Store pending state in `chrome.storage` so it survives worker restarts |
There was a problem hiding this comment.
In this repo, extension code uses webextension-polyfill (browser.*) and storage wrappers (dataStorageAccess / browserLocalStorage) rather than direct chrome.storage / chrome.alarms. Using chrome.* here is likely to push contributors toward patterns the codebase avoids; consider updating these bullets to reference browser.alarms and the storage helper(s), or note the preferred abstraction.
| - Use `chrome.alarms` for delayed operations instead | |
| - Store pending state in `chrome.storage` so it survives worker restarts | |
| - Use `browser.alarms` for delayed operations instead | |
| - Store pending state with the repo's storage helpers (`dataStorageAccess` / | |
| `browserLocalStorage`) so it survives worker restarts |
docs/docs/troubleshooting-guide.md
Outdated
| ```typescript | ||
| // WRONG — listener may not be registered after worker restart | ||
| async function setup() { | ||
| const config = await loadConfig(); | ||
| chrome.runtime.onMessage.addListener(handler); | ||
| } | ||
|
|
||
| // RIGHT — listener is always registered | ||
| chrome.runtime.onMessage.addListener(handler); | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Minor consistency: the fenced code block language is typescript here, but elsewhere the doc uses ts. Not a blocker, but consider using one consistently to keep syntax highlighting predictable across the docs site.
|
|
||
| ### Playwright tests fail on stale build | ||
|
|
||
| **Symptom:** E2e tests fail with unexpected behavior, missing elements, or |
There was a problem hiding this comment.
| **Symptom:** E2e tests fail with unexpected behavior, missing elements, or | |
| **Symptom:** e2e tests fail with unexpected behavior, missing elements, or |
| 4. Tests that fail consistently across multiple runs are real failures; tests | ||
| that pass on retry are flaky | ||
|
|
||
| This avoids wasting time debugging timing-related flakes. If a test is |
There was a problem hiding this comment.
I think we should move away from this advice and move towards using Playwright's playwright-test MCP server to solve flakey tests. The MCP will often put in a more scalable fix than just increasing timeouts, which is often a band aid
| `yarn build:extension:translations` and stages locale files. These are generated | ||
| — commit them as-is. | ||
|
|
||
| ## Playwright & CI Compatibility Issues |
There was a problem hiding this comment.
I think let's group all the Playwright related stuff together
| the typed message enums in all three contexts. A mismatch causes silent message | ||
| drops. | ||
|
|
||
| ### Forgetting to test both Manifest V3 contexts |
There was a problem hiding this comment.
I'm a little confused by what this title means. What are the 2 v3 contexts that this referring to? Is this the DOM and the service worker?
|
|
||
| Background scripts run as service workers (Manifest V3), which means: | ||
|
|
||
| - No persistent state between events (use `chrome.storage`) |
There was a problem hiding this comment.
actually, one common pitfall we can document: we should always use browser.storage rather than th API chrome to maintain cross-browser compatibility
| **Solution:** If a ref callback doesn't need cleanup, ensure it returns | ||
| `undefined` (or nothing). | ||
|
|
||
| ## Known Issues & Recent Fixes |
There was a problem hiding this comment.
I think this is useful today, but we should think about how we can keep this section updated
No description provided.