fix: bug fixes, build improvements, and Sentry removal#59
Merged
Conversation
- Fix critical `in` operator bug in refreshProxy() — `proxyType in ["system", "direct"]` always returned false; replaced with `["system", "direct"].includes(proxyType)` - Fix service worker crash: wrap `new URL(details.url)` in try/catch in background.ts - Fix document.body null access in preference.ts using optional chaining - Replace deepClone() JSON.parse/JSON.stringify with structuredClone() for correctness - Fix window.open() called with import.meta.url as target name; use '_blank' instead - Replace .map() with .forEach() for side-effect-only iterations (profile.ts, auth.ts) - Remove leftover debug console.log calls from ThemeSwitcher, AutoSwitchInput, background, PopupPage - Fix i18n typo: "Advance Config" → "Advanced Config" Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
structuredClone() throws a DataCloneError on JavaScript Proxy objects, including Vue reactive() and ref() wrappers. The JSON.parse/JSON.stringify approach correctly serializes through the Proxy traps, producing a plain object safe for chrome.storage. Extend the comment to document why structuredClone() is intentionally avoided here, to prevent this mistake in future. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Provides build/test commands, architecture overview (adapter layer, PAC-via-AST proxy engine, profile system), and critical gotchas like the deepClone JSON round-trip requirement. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Work around Rolldown panic on nested member expressions of JSON imports by using createRequire for manifest.json. Fix newProxyString using the unnormalized cfg.scheme instead of the defaulted scheme variable. Clean up dead code (commented-out mocks, redundant comments, unused getter, console.debug). Add tests for deepClone, LRUCache, shExpMatch, isPlainHostName, newProxyString, and strengthen findProfile assertions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove @sentry/vue and @sentry/vite-plugin dependencies, Sentry.init() from main.ts, the vite plugin config, env variables, CI workflow refs, and the .env.sentry-build-plugin file. Reduces main.js bundle by ~32%. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes Sentry integration, updates core dependencies (including Vite 8 and TypeScript 6), and significantly expands the test suite. Key improvements include safer URL handling in the background script, refactored proxy profile logic, and a restructured Vite configuration to resolve build-time issues. Feedback was provided regarding the shExpMatch implementation, which currently fails to escape several regex special characters, potentially leading to incorrect matching or errors when processing certain URL patterns.
Comment on lines
+7
to
+11
| const re = new RegExp( | ||
| "^" + | ||
| pattern.replace(/\./g, "\\.").replace(/\*/g, ".*").replace(/\?/g, ".") + | ||
| "$", | ||
| ); |
There was a problem hiding this comment.
The shExpMatch implementation only escapes dots and converts wildcards, but it fails to escape other regular expression special characters such as +, (, ), [, ], {, }, ^, $, and |. This can lead to incorrect matching behavior when these characters appear in URLs (e.g., + in query parameters) or cause the RegExp constructor to throw an error for invalid patterns like *+.
const re = new RegExp(
"^" +
pattern
.replace(/[.+^${}()|[\\\\]|]/g, "\\\\$&")
.replace(/\\*/g, ".*")
.replace(/\\?/g, ".") +
"$",
);`npm ci` on CI (Ubuntu) fails because `@emnapi/core@1.9.2` and `@emnapi/runtime@1.9.2` (transitive deps of the optional `@rolldown/binding-wasm32-wasi` package) were not resolved in the lockfile. This happened because macOS skips the wasm32-wasi optional package entirely during resolution. Regenerating the lockfile from scratch ensures all transitive dependencies are present. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- actions/checkout v4 → v6 - actions/setup-node v4 → v6 - actions/upload-artifact v4 → v7 - actions/download-artifact v4 → v8 - softprops/action-gh-release v2 → v3 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
inoperator on array →.includes(), URL parsing crash guard, safe DOM access,.map()→.forEach()for side effects, popupwindow.opentarget fix, typo in i18nscriptHelper,pacSimulator,statsservice, andutils; expandprofile2configtestsdeepCloneback to JSON round-trip (structuredClone throws on Vue Proxy objects)Commits
655b11dfix the object clone issue, close Error occurred: Error: DataCloneError: Proxy object could not be cloned. #496a95d24fix: resolve multiple bugs and code quality issues2571e1drevert: deepClone back to JSON round-trip from structuredClone()ee9ab6ddocs: add CLAUDE.md with codebase guidance for Claude Code3f5c587fix an basic authentication issue47c4d46fix: Vite 8 build panic, scriptHelper scheme bug, and add testsdf89d37remove: completely remove Sentry integrationTest plan
npm run coverage)npm run build:firefox) produces valid extension🤖 Generated with Claude Code