Conversation
WalkthroughIntroduces a new "blocked" UI screen for the extension with associated routing and state management, adds provider details mappings and a ProviderNameProper enum for display names, updates swap UI components to display provider logos and names, simplifies build scripts by removing prebuild dependencies, and extends the Vite configuration with a new HTML entry point. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ 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 |
|
💼 Build Files |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (4)
packages/extension/src/ui/action/icons/actions/mnemonic.vue (1)
1-17: Consider using CSS variables for theme compatibility.The SVG contains hardcoded color values (
#444,#888,#fff) which may not adapt to different themes (light/dark mode). Consider using CSS custom properties or the current CSS variable for colors used elsewhere in the extension.Example refactor:
<template> <svg width="24" height="24" viewBox="0 0 24 24" fill="none" - stroke="#444" + stroke="currentColor" stroke-width="1.6" stroke-linecap="round" stroke-linejoin="round" > <rect x="6" y="3" width="12" height="18" rx="2" ry="2" /> <path d="M9 3.5V2.8C9 2.36 9.36 2 9.8 2h4.4c.44 0 .8.36.8.8v.7" /> - <rect x="9" y="3" width="6" height="2.8" rx="0.6" ry="0.6" fill="#fff" /> - <path d="M9 9h6M9 13h4M9 17h3" stroke="#888" /> + <rect x="9" y="3" width="6" height="2.8" rx="0.6" ry="0.6" fill="var(--background-color)" /> + <path d="M9 9h6M9 13h4M9 17h3" opacity="0.6" /> </svg> </template>Then use CSS to control the color based on the theme.
packages/extension/src/ui/blocked/App.vue (1)
14-15: Consider self-hosting fonts for performance and privacy.Loading fonts from Google's CDN may have privacy implications (requests expose user IP addresses) and can affect performance if the CDN is slow or blocked in certain regions.
Consider self-hosting the Roboto font or using a system font stack as a fallback:
font-family: 'Roboto', -apple-system, BlinkMacSystemFont, 'Segoe UI', sans-serif;packages/extension/src/ui/blocked/blocked.vue (1)
30-33: Inconsistent component naming between import and usage.Line 31 imports as lowercase
arrowNext, but Vue components should use PascalCase naming convention. The template also uses kebab-case<arrow-next />which works but is inconsistent with the other imports (line 33 usesLogoBigin PascalCase).Standardize to PascalCase for consistency:
-import arrowNext from '../action/icons/common/arrow-next.vue'; -import mnemonic from '../action/icons/actions/mnemonic.vue'; +import ArrowNext from '../action/icons/common/arrow-next.vue'; +import Mnemonic from '../action/icons/actions/mnemonic.vue'; import LogoBig from '@action/icons/common/logo-big.vue';Then update template usage:
<div class="blocked-page__content__header"> - <mnemonic /> + <Mnemonic /> <h4>View My Secret Recovery Phrase</h4> - <arrow-next /> + <ArrowNext /> </div>packages/extension/src/ui/blocked/user-analytics.vue (1)
45-67: Reduce code duplication in agree and deny handlers.Both
agree()anddeny()functions share identical navigation logic (lines 50-54 and 62-66). This violates the DRY principle and makes maintenance harder.Extract the common navigation logic:
+const navigateOrClose = () => { + if (route.name === 'user-privacy') { + window.close(); + } else { + router.push({ name: 'new-wallet' }); + } +}; + const agree = async () => { const enkryptSettings = await settingsState.getEnkryptSettings(); enkryptSettings.isMetricsEnabled = true; await settingsState.setEnkryptSettings(enkryptSettings); optOutofMetrics(false); - if (route.name === 'user-privacy') { - window.close(); - } else { - router.push({ name: 'new-wallet' }); - } + navigateOrClose(); }; const deny = async () => { const enkryptSettings = await settingsState.getEnkryptSettings(); enkryptSettings.isMetricsEnabled = false; await settingsState.setEnkryptSettings(enkryptSettings); optOutofMetrics(true); - if (route.name === 'user-privacy') { - window.close(); - } else { - router.push({ name: 'new-wallet' }); - } + navigateOrClose(); };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
packages/extension/src/ui/action/assets/swap/0x-logo.pngis excluded by!**/*.pngpackages/extension/src/ui/action/assets/swap/1inch-logo.pngis excluded by!**/*.pngpackages/extension/src/ui/action/assets/swap/changelly-logo.pngis excluded by!**/*.pngpackages/extension/src/ui/action/assets/swap/jupiter-logo.pngis excluded by!**/*.pngpackages/extension/src/ui/action/assets/swap/okx-logo.pngis excluded by!**/*.pngpackages/extension/src/ui/action/assets/swap/paraswap-logo.pngis excluded by!**/*.pngpackages/extension/src/ui/action/assets/swap/rango-logo.pngis excluded by!**/*.png
📒 Files selected for processing (15)
packages/extension/blocked.html(1 hunks)packages/extension/package.json(1 hunks)packages/extension/src/libs/utils/providersDetails.ts(1 hunks)packages/extension/src/ui/action/icons/actions/mnemonic.vue(1 hunks)packages/extension/src/ui/action/views/swap/views/swap-best-offer/components/swap-best-offer-block/components/best-offer-list-item.vue(4 hunks)packages/extension/src/ui/action/views/swap/views/swap-best-offer/components/swap-best-offer-block/components/best-offer-list.vue(1 hunks)packages/extension/src/ui/action/views/swap/views/swap-best-offer/components/swap-best-offer-block/index.vue(4 hunks)packages/extension/src/ui/blocked/App.vue(1 hunks)packages/extension/src/ui/blocked/blocked.vue(1 hunks)packages/extension/src/ui/blocked/index.html(1 hunks)packages/extension/src/ui/blocked/main.ts(1 hunks)packages/extension/src/ui/blocked/routes.ts(1 hunks)packages/extension/src/ui/blocked/user-analytics.vue(1 hunks)packages/extension/vite.config.ts(1 hunks)packages/swap/src/types/index.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: buildAll
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (9)
packages/swap/src/types/index.ts (1)
135-145: LGTM! Well-structured provider display names.The new
ProviderNameProperenum provides user-facing display names that align correctly with theProviderNameenum keys. This separation of internal identifiers from display labels is a good practice.packages/extension/package.json (1)
7-7: LGTM! Build script simplification.Removing the prebuild dependencies streamlines the build process. The Firefox-specific post-build step for system info collection is appropriately isolated to that target.
Also applies to: 15-17, 21-21
packages/extension/vite.config.ts (1)
108-108: LGTM! New entry point added correctly.The
blockedentry point is properly integrated into the build configuration, consistent with the existing entry points.packages/extension/blocked.html (1)
1-21: LGTM! Standard HTML entry point.The structure is consistent with other entry HTML files in the extension, properly loading the TypeScript module and including appropriate fallback messaging.
packages/extension/src/ui/blocked/main.ts (1)
1-27: Standard Vue app bootstrap.The Vue 3 app initialization with router (hash history), Pinia store, and global filters is implemented correctly and follows the established patterns in the codebase.
packages/extension/src/ui/blocked/routes.ts (1)
1-4: LGTM! Simple and appropriate route configuration.The single-route configuration is suitable for the blocked UI module's purpose.
packages/extension/src/ui/action/views/swap/views/swap-best-offer/components/swap-best-offer-block/index.vue (1)
195-198: CSS styling for provider logo looks appropriate.The added styling for the provider logo in the heading is properly scoped and sized.
packages/extension/src/libs/utils/providersDetails.ts (1)
11-20: All ProviderName enum values are already mapped—no changes needed.The script output confirms that all 8 values from the
packages/swap/src/types/index.tsProviderName enum (oneInch,oneInchFusion,paraswap,zerox,changelly,rango,jupiter,okx) are present in the mapping at lines 12–19 ofprovidersDetails.ts. The code is complete and correct as-is.packages/extension/src/ui/action/views/swap/views/swap-best-offer/components/swap-best-offer-block/components/best-offer-list.vue (1)
4-13: <!-- [scratchpad_start]
[task verify_trades_key_uniqueness 1/1]
VERIFICATION COMPLETE - Found definitive evidence[observations]
- packages/swap/src/index.ts lines 122-136: this.providers is fixed array with unique providers
- Solana: [Jupiter, Rango, Changelly, OKX]
- EVM: [OneInch, OneInchFusion, Paraswap, Changelly, ZeroX, ...]
- getQuotes (lines 206-232) maps over this.providers (1:1)
- Each provider appears exactly once in this.providers
- getQuotes returns filtered/sorted, no deduplication needed
[analysis]
- Architecture guarantees: one provider = one quote per call to getQuotes
- trades array comes from getQuotes mapped to full swaps
- Therefore trade.provider is UNIQUE in trades array
- Using trade.provider as key is CORRECT and SAFE
- No duplicate provider keys possible
[conclusion]
Review comment is INCORRECT
- Original concern about duplicate providers is unfounded
- Provider uniqueness is guaranteed by architecture
- Code using :key="trade.provider" is appropriate and safe
[done]
Verification complete: review comment is based on incorrect assumption
</scratchpad_start]**Rewritten review comment would be marked as **
The provider uniqueness is guaranteed by architecture. The
getQuotes()method maps over the fixedthis.providersarray (one provider instance per supported swap provider), returning exactly one quote per provider. Therefore,trade.provideris guaranteed to be unique across the trades array, and using it as the:keyis correct and safe.Likely an incorrect or invalid review comment.
| export default { | ||
| [ProviderName.changelly]: { name: ProviderNameProper.changelly, logo: changellyLogo }, | ||
| [ProviderName.oneInch]: { name: ProviderNameProper.oneInch, logo: oneInch }, | ||
| [ProviderName.oneInchFusion]: { name: ProviderNameProper.oneInchFusion, logo: oneInch }, | ||
| [ProviderName.paraswap]: { name: ProviderNameProper.paraswap, logo: paraswap }, | ||
| [ProviderName.zerox]: { name: ProviderNameProper.zerox, logo: zerox }, | ||
| [ProviderName.rango]: { name: ProviderNameProper.rango, logo: rango }, | ||
| [ProviderName.jupiter]: { name: ProviderNameProper.jupiter, logo: jupiter }, | ||
| [ProviderName.okx]: { name: ProviderNameProper.okx, logo: okx } | ||
| } No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add type annotation for better type safety.
The exported object lacks a type annotation, which reduces type safety when accessing provider details elsewhere in the codebase. This makes it easier for runtime errors to occur when accessing non-existent properties.
Add a typed interface and annotation:
+interface ProviderDetails {
+ name: string;
+ logo: string;
+}
+
+const providersDetails: Record<ProviderName, ProviderDetails> = {
-export default {
[ProviderName.changelly]: { name: ProviderNameProper.changelly, logo: changellyLogo },
[ProviderName.oneInch]: { name: ProviderNameProper.oneInch, logo: oneInch },
[ProviderName.oneInchFusion]: { name: ProviderNameProper.oneInchFusion, logo: oneInch },
[ProviderName.paraswap]: { name: ProviderNameProper.paraswap, logo: paraswap },
[ProviderName.zerox]: { name: ProviderNameProper.zerox, logo: zerox },
[ProviderName.rango]: { name: ProviderNameProper.rango, logo: rango },
[ProviderName.jupiter]: { name: ProviderNameProper.jupiter, logo: jupiter },
[ProviderName.okx]: { name: ProviderNameProper.okx, logo: okx }
-}
+};
+
+export default providersDetails;This provides compile-time checking that all ProviderName enum values are present in the mapping.
🤖 Prompt for AI Agents
In packages/extension/src/libs/utils/providersDetails.ts around lines 11-20, the
exported providers object has no type annotation; add a strong type by defining
a ProviderDetail interface (e.g. { name: string; logo: string |
StaticImageImport } or the correct logo type) and then type the export as
Record<ProviderName, ProviderDetail> (or an equivalent mapped type), import the
ProviderName/ProviderNameProper types as needed, and change the default export
declaration to use that type so the compiler enforces that every ProviderName
key is present and values have the correct shape.
| <img | ||
| v-if="providersDetails[props.provider]" | ||
| :src="providersDetails[props.provider].logo" | ||
| /> |
There was a problem hiding this comment.
Image element should be accessible with alt text.
The provider logo image lacks an alt attribute, which is important for accessibility and should describe the image content.
Add an alt attribute:
<img
v-if="providersDetails[props.provider]"
:src="providersDetails[props.provider].logo"
+ :alt="`${providersDetails[props.provider].name} logo`"
/>🤖 Prompt for AI Agents
In
packages/extension/src/ui/action/views/swap/views/swap-best-offer/components/swap-best-offer-block/components/best-offer-list-item.vue
around lines 4-7, the <img> lacks an alt attribute; add a bound alt so the image
is accessible by using the provider's name when available or an empty string if
decorative (for example bind :alt="providersDetails[props.provider]?.name ||
''"), keeping the existing v-if check and src binding intact.
| <p> | ||
| {{ | ||
| props.provider | ||
| ? providersDetails[props.provider].name | ||
| : 'Unknown Provider' | ||
| }} | ||
| </p> |
There was a problem hiding this comment.
Potential runtime error when accessing provider details.
The code checks props.provider existence but doesn't verify that providersDetails[props.provider] exists before accessing .name. If a provider key exists in ProviderName but is missing from providersDetails, this will throw a runtime error.
Apply defensive coding to handle missing provider details:
<p>
{{
props.provider
- ? providersDetails[props.provider].name
+ ? providersDetails[props.provider]?.name || 'Unknown Provider'
: 'Unknown Provider'
}}
</p>Alternatively, consolidate the logic:
- <p>
- {{
- props.provider
- ? providersDetails[props.provider].name
- : 'Unknown Provider'
- }}
- </p>
+ <p>
+ {{ providersDetails[props.provider]?.name || 'Unknown Provider' }}
+ </p>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <p> | |
| {{ | |
| props.provider | |
| ? providersDetails[props.provider].name | |
| : 'Unknown Provider' | |
| }} | |
| </p> | |
| <p> | |
| {{ | |
| props.provider | |
| ? providersDetails[props.provider]?.name || 'Unknown Provider' | |
| : 'Unknown Provider' | |
| }} | |
| </p> |
| <p> | |
| {{ | |
| props.provider | |
| ? providersDetails[props.provider].name | |
| : 'Unknown Provider' | |
| }} | |
| </p> | |
| <p> | |
| {{ providersDetails[props.provider]?.name || 'Unknown Provider' }} | |
| </p> |
🤖 Prompt for AI Agents
In
packages/extension/src/ui/action/views/swap/views/swap-best-offer/components/swap-best-offer-block/components/best-offer-list-item.vue
around lines 10–16, the template accesses providersDetails[props.provider].name
without ensuring providersDetails[props.provider] exists, which can throw at
runtime; update the rendering to guard the lookup (e.g., use optional chaining
or an explicit check) and fall back to 'Unknown Provider' when
providersDetails[props.provider] is undefined or name is missing, or consolidate
into a single evaluated value like providerName =
providersDetails[props.provider]?.name ?? 'Unknown Provider' and render that.
| <h3> | ||
| Best offer including network fee powered by | ||
| <img :src="providersDetails[props.pickedTrade.provider].logo" /> | ||
| {{ providersDetails[props.pickedTrade.provider].name }} | ||
| </h3> |
There was a problem hiding this comment.
Add defensive checks for provider details access.
The code directly accesses providersDetails[props.pickedTrade.provider] without verifying that the provider exists in the mapping. This could cause runtime errors if an unexpected provider value is encountered or if the mapping is incomplete.
Add defensive checks or fallbacks:
<h3>
Best offer including network fee powered by
- <img :src="providersDetails[props.pickedTrade.provider].logo" />
- {{ providersDetails[props.pickedTrade.provider].name }}
+ <img :src="providersDetails[props.pickedTrade.provider]?.logo" />
+ {{ providersDetails[props.pickedTrade.provider]?.name || props.pickedTrade.provider }}
</h3>And similarly for lines 50-54:
<p>
Provider:
<img
height="15px"
- :src="providersDetails[props.pickedTrade.provider].logo"
+ :src="providersDetails[props.pickedTrade.provider]?.logo"
/>
- {{ providersDetails[props.pickedTrade.provider].name }}
+ {{ providersDetails[props.pickedTrade.provider]?.name || props.pickedTrade.provider }}
</p>Also applies to: 48-55
🤖 Prompt for AI Agents
In
packages/extension/src/ui/action/views/swap/views/swap-best-offer/components/swap-best-offer-block/index.vue
around lines 3-7 and 48-55, the template directly indexes
providersDetails[props.pickedTrade.provider] which can throw if the provider key
is missing; change this to resolve providerDetail safely (e.g., create a
computed getter that reads providersDetails[props.pickedTrade.provider] with a
fallback empty object or default { logo: defaultLogo, name: 'Unknown provider'
}) and use that computed value in the template (or use optional chaining and
nullish coalescing) so the img src and displayed name always have safe fallbacks
and runtime errors are prevented.
| @@ -0,0 +1,122 @@ | |||
| <template> | |||
| <div class="blocked__container"> | |||
| <logo class="blocked__logo" /> | |||
There was a problem hiding this comment.
Critical: Missing import for logo component.
The template uses <logo> at line 3, but there is no corresponding import in the script section (line 10 is empty). This will cause a runtime error when the component tries to render.
Add the logo import to the script section:
-<script setup lang="ts"></script>
+<script setup lang="ts">
+import Logo from '@action/icons/common/logo.vue';
+</script>Note: Verify the correct path for the logo component in your codebase.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <logo class="blocked__logo" /> | |
| <template> | |
| <div class="blocked__container"> | |
| <logo class="blocked__logo" /> | |
| <router-view /> | |
| </div> | |
| </template> | |
| <script setup lang="ts"> | |
| import Logo from '@action/icons/common/logo.vue'; | |
| </script> | |
| <style lang="less" scoped> | |
| /* styles remain unchanged */ | |
| </style> |
🤖 Prompt for AI Agents
In packages/extension/src/ui/blocked/App.vue around line 3 (template) and the
empty script area at line 10, the <logo> component is used but not
imported/registered; add an import for the Logo component using the correct
relative path and register it in the component's export (either via the
components object in the default export or via setup registration for
composition API) so the <logo> tag resolves at runtime.
| <div class="blocked-page__content__header"> | ||
| <mnemonic /> | ||
| <h4>View My Secret Recovery Phrase</h4> | ||
| <arrow-next /> |
There was a problem hiding this comment.
Missing click handler for recovery phrase action.
The content block displays "View My Secret Recovery Phrase" with icons suggesting it's clickable, but there's no click handler or navigation implemented. This UI element appears interactive but does nothing.
Add a click handler to make this functional:
- <div class="blocked-page__content__header">
+ <div class="blocked-page__content__header" @click="viewRecoveryPhrase" style="cursor: pointer;">
<mnemonic />
<h4>View My Secret Recovery Phrase</h4>
<arrow-next />
</div>And implement the handler in the script:
<script setup lang="ts">
import arrowNext from '../action/icons/common/arrow-next.vue';
import mnemonic from '../action/icons/actions/mnemonic.vue';
import LogoBig from '@action/icons/common/logo-big.vue';
+import { useRouter } from 'vue-router';
+
+const router = useRouter();
+
+const viewRecoveryPhrase = () => {
+ router.push({ name: 'show-recovery-phrase' });
+};
</script>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div class="blocked-page__content__header"> | |
| <mnemonic /> | |
| <h4>View My Secret Recovery Phrase</h4> | |
| <arrow-next /> | |
| <div class="blocked-page__content__header" @click="viewRecoveryPhrase" style="cursor: pointer;"> | |
| <mnemonic /> | |
| <h4>View My Secret Recovery Phrase</h4> | |
| <arrow-next /> | |
| </div> |
| </div> | ||
| <div class="blocked-page__divider-bottom"></div> | ||
| <div class="blocked-page__footer"> | ||
| <a href="#">Contact support</a> |
There was a problem hiding this comment.
Placeholder link needs a real destination.
The "Contact support" link uses href="#" which is non-functional. This should point to an actual support contact page.
Provide a real support URL:
- <a href="#">Contact support</a>
+ <a href="https://support.enkrypt.com/contact" target="_blank" rel="noopener noreferrer">Contact support</a>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <a href="#">Contact support</a> | |
| <a href="https://support.enkrypt.com/contact" target="_blank" rel="noopener noreferrer">Contact support</a> |
🤖 Prompt for AI Agents
In packages/extension/src/ui/blocked/blocked.vue at line 26, the "Contact
support" anchor uses href="#" which is non-functional; update it to point to the
real support page (for example replace with "https://support.example.com" or
your project's actual help center URL) and consider using a mailto fallback
(mailto:support@yourdomain.com) if needed so the link reliably opens the support
contact.
| > | ||
| </noscript> | ||
| <div id="app"></div> | ||
| <script type="module" src="/src/ui/onboard/main.ts"></script> |
There was a problem hiding this comment.
Critical: Incorrect script path for blocked UI entry point.
Line 18 loads /src/ui/onboard/main.ts but this HTML file is located in the blocked/ directory and should load the blocked UI bootstrap. Based on the AI summary mentioning a main.ts file at packages/extension/src/ui/blocked/main.ts, this appears to be a copy-paste error.
Apply this diff to fix the script path:
- <script type="module" src="/src/ui/onboard/main.ts"></script>
+ <script type="module" src="/src/ui/blocked/main.ts"></script>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <script type="module" src="/src/ui/onboard/main.ts"></script> | |
| <script type="module" src="/src/ui/blocked/main.ts"></script> |
🤖 Prompt for AI Agents
In packages/extension/src/ui/blocked/index.html around line 18, the script tag
incorrectly points to /src/ui/onboard/main.ts; update it to load the blocked UI
bootstrap at /src/ui/blocked/main.ts so the HTML in the blocked/ directory
imports the correct entrypoint (replace the src value with
/src/ui/blocked/main.ts).
| import routes from './routes'; | ||
| import * as filters from '@action/utils/filters'; | ||
|
|
||
| global.WeakMap = WeakMap; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the necessity of the global WeakMap assignment.
The assignment global.WeakMap = WeakMap; appears to be a polyfill or compatibility shim. Verify whether this is still necessary for the extension's target environments, as modern JavaScript environments should have native WeakMap support.
Run the following script to check if this pattern exists elsewhere in the codebase and understand its context:
🏁 Script executed:
#!/bin/bash
# Search for similar global assignments in other entry points
rg -nP 'global\.(WeakMap|Map|Set|Promise)\s*=' --type=ts -C3Length of output: 2119
🏁 Script executed:
#!/bin/bash
# Check for browserslist or similar environment specifications
fd -type f -name ".browserslistrc|browserslist|package.json" | head -20Length of output: 170
🏁 Script executed:
#!/bin/bash
# Search for WeakMap usage in the codebase to understand if polyfill is needed
rg -nP '\bWeakMap\b' --type=ts --type=js -g '!node_modules' -C2 | head -50Length of output: 1317
🏁 Script executed:
#!/bin/bash
# Check package.json for target environment info
cat package.json | grep -A5 -B5 "browser\|target\|version"Length of output: 1549
🏁 Script executed:
#!/bin/bash
# Look for comments explaining WeakMap polyfill or git history
git log -n 20 --oneline -- packages/extension/src/ui/*/main.ts 2>/dev/null | head -10Length of output: 413
🏁 Script executed:
#!/bin/bash
# Check extension manifest for browser targets
find . -name "manifest.json" -o -name "manifest.*.json" | head -5 | xargs cat 2>/dev/nullLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check if any extension-specific build config defines targets
find packages/extension -name "*.config.*" -o -name "tsconfig.json" | head -10 | xargs cat 2>/dev/null | grep -i "target\|browser"Length of output: 438
🏁 Script executed:
#!/bin/bash
# Search for any comments in the codebase explaining this pattern
rg -nP 'WeakMap.*polyfill|polyfill.*WeakMap|legacy.*WeakMap' -iLength of output: 0
🌐 Web query:
Does Vue 3 or Pinia require a global WeakMap polyfill?
💡 Result:
Short answer: No — Vue 3 and Pinia do not require you to add a global WeakMap polyfill in modern environments. They use WeakMap internally, but they expect the runtime (browser/Node) to provide the native WeakMap; if you must support very old browsers (e.g. IE11) you should include a WeakMap polyfill. [1][2]
Sources:
- Vue CLI / polyfills guidance (Vue uses environment polyfills via browserslist/@babel/preset-env; add polyfills when targeting older browsers). [1]
- WeakMap availability / browser support (widely supported in modern browsers; missing in older IE). [2]
Remove the unnecessary global.WeakMap assignment from all entry points.
The global.WeakMap = WeakMap; pattern found in packages/extension/src/ui/blocked/main.ts (line 8), packages/extension/src/ui/provider-pages/main.ts (line 9), packages/extension/src/ui/onboard/main.ts (line 8), and packages/extension/src/ui/action/main.ts (line 8) is legacy code. Vue 3 and Pinia use WeakMap internally but expect the runtime to provide native support—no explicit polyfill is required for modern browsers (Chrome, Firefox, Safari, Opera). This assignment is dead code and should be removed.
🤖 Prompt for AI Agents
In packages/extension/src/ui/blocked/main.ts around line 8, remove the
unnecessary legacy assignment "global.WeakMap = WeakMap;" — delete that single
line (and similarly remove the same assignment from the other entry points
listed) so the runtime's native WeakMap is used; after removal, ensure the file
compiles and run the app/tests to verify nothing else depends on that global
assignment.
| const agree = async () => { | ||
| const enkryptSettings = await settingsState.getEnkryptSettings(); | ||
| enkryptSettings.isMetricsEnabled = true; | ||
| await settingsState.setEnkryptSettings(enkryptSettings); | ||
| optOutofMetrics(false); | ||
| if (route.name === 'user-privacy') { | ||
| window.close(); | ||
| } else { | ||
| router.push({ name: 'new-wallet' }); | ||
| } | ||
| }; | ||
|
|
||
| const deny = async () => { | ||
| const enkryptSettings = await settingsState.getEnkryptSettings(); | ||
| enkryptSettings.isMetricsEnabled = false; | ||
| await settingsState.setEnkryptSettings(enkryptSettings); | ||
| optOutofMetrics(true); | ||
| if (route.name === 'user-privacy') { | ||
| window.close(); | ||
| } else { | ||
| router.push({ name: 'new-wallet' }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Add error handling for settings operations.
The async operations for getting and setting Enkrypt settings lack error handling. If these operations fail (e.g., storage issues, permission problems), the user will see no feedback and the consent choice may not be saved.
Add try-catch blocks to handle potential errors:
const agree = async () => {
+ try {
const enkryptSettings = await settingsState.getEnkryptSettings();
enkryptSettings.isMetricsEnabled = true;
await settingsState.setEnkryptSettings(enkryptSettings);
optOutofMetrics(false);
- if (route.name === 'user-privacy') {
- window.close();
- } else {
- router.push({ name: 'new-wallet' });
- }
+ navigateOrClose();
+ } catch (error) {
+ console.error('Failed to save analytics preference:', error);
+ // Show user feedback, e.g., toast notification
+ }
};
const deny = async () => {
+ try {
const enkryptSettings = await settingsState.getEnkryptSettings();
enkryptSettings.isMetricsEnabled = false;
await settingsState.setEnkryptSettings(enkryptSettings);
optOutofMetrics(true);
- if (route.name === 'user-privacy') {
- window.close();
- } else {
- router.push({ name: 'new-wallet' });
- }
+ navigateOrClose();
+ } catch (error) {
+ console.error('Failed to save analytics preference:', error);
+ // Show user feedback, e.g., toast notification
+ }
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/extension/src/ui/blocked/user-analytics.vue around lines 45 to 67,
the agree and deny handlers perform async get/set of enkrypt settings and call
optOutofMetrics without any error handling; wrap the await
settingsState.getEnkryptSettings() / setEnkryptSettings() and optOutofMetrics()
calls in try-catch blocks, log or report the caught error (console.error or the
app's logger), show the user a brief error message/toast if saving fails, and
only proceed to window.close() or router.push() when the settings update
succeeds (do not navigate/close on failure).
2 participants
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.