Skip to content

fix: resolve multiple bugs and code quality issues#57

Open
SYM01 wants to merge 2 commits intodevelopfrom
fix/bugs-and-optimizations
Open

fix: resolve multiple bugs and code quality issues#57
SYM01 wants to merge 2 commits intodevelopfrom
fix/bugs-and-optimizations

Conversation

@SYM01
Copy link
Collaborator

@SYM01 SYM01 commented Feb 27, 2026

Summary

  • Critical logic bug: refreshProxy() used the in operator on an array (proxyType in ["system", "direct"]), which checks for index keys (0, 1), not values — always false. Preset profiles were never skipped during refresh.
  • Service worker crash: new URL(details.url) in background.ts threw on malformed URLs, silently killing the service worker. Wrapped in try/catch.
  • Null dereference: document && document.body.setAttribute(...) in preference.ts guarded document but not document.body. Replaced with optional chaining.
  • deepClone robustness: Replaced JSON.parse(JSON.stringify()) with structuredClone(), which correctly handles Date, Map, Set, and undefined values (available in Chrome 98+ / Firefox 94+).
  • window.open() bug: PopupPage.vue passed import.meta.url as the window target name instead of '_blank', causing unexpected window reuse behavior.
  • Side-effect misuse: .map() was used for side-effect-only iterations in profile.ts and auth.ts (creating wasted arrays). Replaced with .forEach().
  • Debug code removed: Leftover console.log calls removed from ThemeSwitcher.vue, AutoSwitchInput.vue, background.ts, and PopupPage.vue.
  • i18n typo: "Advance Config""Advanced Config" in messages.json.

Test plan

  • All 44 existing unit tests pass (npm test)
  • Proxy refresh correctly skips system/direct profiles after the inincludes fix
  • No service worker crashes on navigating to pages with unusual URLs
  • Theme switching works correctly without console noise
  • Opening Config page from popup opens in a new tab (not reusing existing window)
  • deepClone works correctly for all profile types

🤖 Generated with Claude Code

- 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>
@gemini-code-assist
Copy link

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request delivers a comprehensive set of bug fixes and code quality improvements across various parts of the application. It addresses critical logical errors, enhances the stability of the service worker, prevents potential runtime crashes, and refines core utility functions. The changes also include correcting user interface behaviors and cleaning up development artifacts, leading to a more robust, reliable, and maintainable codebase.

Highlights

  • Critical Logic Bug Fix: The refreshProxy() function incorrectly used the in operator on an array (proxyType in ["system", "direct"]), which checks for index keys instead of values. This bug prevented preset profiles from being correctly skipped during refresh. The fix replaces in with includes().
  • Service Worker Stability Improvement: The new URL(details.url) constructor in background.ts could cause the service worker to crash when encountering malformed URLs. This operation is now wrapped in a try/catch block to prevent crashes and ensure service worker resilience.
  • Null Dereference Prevention: In preference.ts, the code document && document.body.setAttribute(...) guarded document but not document.body, creating a potential null dereference. This has been resolved by using optional chaining (document?.body?.setAttribute(...)) for safer access.
  • Enhanced Deep Cloning Robustness: The deepClone() utility function has been upgraded from JSON.parse(JSON.stringify()) to structuredClone(). This change significantly improves cloning capabilities, allowing correct handling of Date, Map, Set, undefined, and other complex data types, as well as circular references.
  • Window Opening Behavior Correction: In PopupPage.vue, window.open() was incorrectly using import.meta.url as the window target name instead of '_blank'. This caused unexpected window reuse behavior, which has now been fixed to always open in a new tab.
  • Optimized Array Iteration: Instances where .map() was used solely for side-effect-only iterations (e.g., onProfileUpdateListeners.map(...) in profile.ts and auths.map(...) in auth.ts) have been replaced with .forEach(). This avoids the creation of unnecessary new arrays and improves code clarity.
  • Debug Code Cleanup: Several leftover console.log statements, used for debugging, have been removed from ThemeSwitcher.vue, AutoSwitchInput.vue, background.ts, and PopupPage.vue to clean up the codebase.
  • Internationalization Typo Correction: A minor typo in messages.json has been fixed, changing "Advance Config" to "Advanced Config" for better readability and correctness.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • public/_locales/en/messages.json
    • Corrected a typo in the 'config_section_advance' message from 'Advance Config' to 'Advanced Config'.
  • src/background.ts
    • Wrapped the new URL(details.url) call in a try-catch block to prevent service worker crashes on malformed URLs.
    • Removed a console.log statement related to 'onResponseStarted'.
  • src/components/configs/AutoSwitchInput.vue
    • Removed a debug console.log("test") statement from the URL validator.
  • src/components/controls/ThemeSwitcher.vue
    • Removed a console.log statement that logged the current dark mode.
  • src/pages/PopupPage.vue
    • Changed the window.open() target from import.meta.url to "_blank" to ensure new tabs are opened.
    • Removed a console.log statement from the setProxyByProfile function.
  • src/services/preference.ts
    • Updated document.body access to use optional chaining (document?.body?.setAttribute and document?.body?.removeAttribute) for safer null handling.
  • src/services/profile.ts
    • Replaced .map() with .forEach() for iterating onProfileUpdateListeners as no new array was intended.
  • src/services/proxy/auth.ts
    • Replaced .map() with .forEach() when iterating through auths as no new array was intended.
  • src/services/proxy/index.ts
    • Corrected the logic for checking preset profiles by changing current.activeProfile.proxyType in ["system", "direct"] to ["system", "direct"].includes(current.activeProfile.proxyType).
  • src/services/utils.ts
    • Replaced JSON.parse(JSON.stringify(obj)) with structuredClone(obj) in the deepClone function for more robust object cloning.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses a good number of bugs and code quality issues, from critical logic errors like the misuse of the in operator to robustness improvements and removal of debug code. The changes are well-described and the fixes are generally excellent.

I've found one critical issue with the switch to structuredClone for deepClone. While structuredClone is generally superior, it cannot handle Vue's reactive Proxy objects, which contradicts the documented purpose of the deepClone function in this codebase. This change could lead to runtime errors when saving profiles. I've left a specific comment with a suggestion to revert this particular change.

*/
export function deepClone<T>(obj: T): T {
return JSON.parse(JSON.stringify(obj));
return structuredClone(obj);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The switch to structuredClone() introduces a critical issue. The documentation for deepClone states its purpose is to 'remove all Proxy objects (e.g., from Vue reactivity)', which is necessary before saving to chrome.storage.

However, structuredClone() cannot clone Proxy objects and will throw a DataCloneError. The previous JSON.parse(JSON.stringify()) implementation correctly handled this by serializing the underlying raw object from the proxy.

While some call sites might use toRaw() before cloning, others (like in profile.ts) may not, leading to runtime errors when saving profiles. Given this, I recommend reverting to the previous implementation to ensure reactive objects are handled correctly.

Suggested change
return structuredClone(obj);
return JSON.parse(JSON.stringify(obj));

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant