Migrates to WXT #105
Migrates to WXT #105
Conversation
|
|
|
WalkthroughThis pull request removes several redundant configuration, manifest, and type definition files while updating build settings and code style. The changes include version bumps, minor formatting adjustments, refactoring of background and content scripts, updated CSS/script source paths, and revised URL references. A new UI state is added for copy feedback, and certain message listener files were removed. Additionally, ignore lists and changelog entries have been updated. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User/Event
participant BG as Background (main)
U->>BG: Trigger background functionality
BG->>BG: Initialize logging, sidebar listeners, & quick menu
BG-->>U: Complete setup
sequenceDiagram
participant U as User
participant CL as ChatList Component
U->>CL: Clicks copy button
CL->>CL: Set showCopiedText = true (display checkmark)
Note right of CL: Wait for 3 seconds
CL->>CL: Set showCopiedText = false (hide checkmark)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (32)
📒 Files selected for processing (95)
💤 Files with no reviewable changes (5)
🔇 Additional comments (38)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…xer, postcss, tailwindcss, and typescript
|
Help wanted? |
Oh yes ! help always welcome, for this you need this: then pnpm dev would open vivaldi with dev extension installed automatically // web-ext.config.ts
import { defineRunnerConfig } from 'wxt'
export default defineRunnerConfig({
binaries: {
chrome: '/Applications/Vivaldi.app/Contents/MacOS/Vivaldi',
},
}) |
Got it, thanks! 👏 |
Perhaps I'm doing sth wrong, trying to run the extensions, I altered |
Hmm seeing this for first time, but yeah i think first we need to do the folder structure refactoring to match wxt framework. i think it should work once we set it up |
Yep, seems it's file path issue, different error this time 😅 |
Signed-off-by: EINDEX <[email protected]>
refactor: wxt switch
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 7
🧹 Nitpick comments (14)
theme.ts (1)
90-90: Consider using a more specific type thananyin a future update.While using
: anyhelps unblock the WXT migration by bypassing type errors, it reduces type safety. Consider replacing it with a more specific interface or type definition in a follow-up PR to maintain strong typing.- maxWidth: ({ theme, breakpoints }: any) => ({ + maxWidth: ({ theme, breakpoints }: { theme: Record<string, any>; breakpoints: (screens: any) => Record<string, string> }) => ({src/entrypoints/quick-menu.content/listenContextMenu.tsx (1)
1-10: New context menu listener file overview.
The new file imports the necessary functions (findPromptandgeneratePromptInSidebar) and includes a descriptive comment outlining its purpose—to listen for messages (e.g., fromchrome.tabs.sendMessage) and generate a prompt in the sidebar.However, the file currently only includes the import statements and accompanying comments. Verify if the event listener logic (e.g., using
chrome.runtime.onMessage.addListener) is intended to be implemented here or elsewhere. If not already handled, consider adding the actual event listening mechanism to complete the functionality..gitignore (1)
27-29: Duplicate Ignore Pattern Entries Detected
The entries forweb-ext.config.ts,.wxt, and.outputappear twice in the file. To avoid redundancy and simplify maintenance, consider removing the duplicate entries (lines 27–29) if they are not required for different contexts.src/entrypoints/quick-menu.content/index.ts (2)
7-10: Content script setup looks good, with minor redundancyThe content script configuration uses correct WXT pattern with
defineContentScript. However, thematchesarray includes both specific URL patterns and<all_urls>, which is redundant since<all_urls>already covers all possible URLs.- matches: ['http://*/*', 'https://*/*','<all_urls>'], + matches: ['<all_urls>'],
11-27: Add error handling for storage operationsThe chrome storage operation lacks error handling. Consider adding error handling to gracefully manage potential storage access issues.
- chrome.storage.sync.get(['SETTINGS'], (result) => { + chrome.storage.sync.get(['SETTINGS'], (result) => { const quickMenuSettings = result.SETTINGS?.quickMenu as Settings['quickMenu'] if (quickMenuSettings) { if (quickMenuSettings.enabled) { if (quickMenuSettings.excludedSites.length === 0) initQuickMenu() else { const whitelister = new Whitelister( quickMenuSettings.excludedSites || '*', ) const isExcluded = whitelister.verify(window.location.href) if (!isExcluded) initQuickMenu() } } } else { initQuickMenu() } - }) + }).catch(error => { + console.error('Error accessing settings:', error); + // Fallback behavior - could be to not initialize or to initialize with defaults + })src/entrypoints/sidebar_bg.content/index.ts (1)
70-70: Remove redundant protocol matches.
"<all_urls>"already encompasseshttp://*/*andhttps://*/*. Leaving them in will not break anything, but removing them could make the configuration clearer.Apply this diff for clarity:
- matches: ['http://*/*', 'https://*/*','<all_urls>'], + matches: ['<all_urls>'],src/entrypoints/content/index.ts (4)
12-12: Excessively large z-index valueThe z-index value of '9000000000000000000' is unnecessarily high and may exceed JavaScript's safe integer limits. Consider using a more reasonable value like '2147483647' (max 32-bit integer) or '9999'.
- iframe.style.zIndex = '9000000000000000000' + iframe.style.zIndex = '2147483647'
47-52: Clipboard operation is missing user feedbackConsider providing visual feedback to the user when content is copied to the clipboard or when the operation fails.
if (action === 'copy-to-clipboard') { const { content } = _payload as { content: string } - navigator.clipboard.writeText(content).catch((err) => { + navigator.clipboard.writeText(content).then(() => { + iframe.contentWindow?.postMessage( + { + action: 'clipboard-copy-success', + }, + chrome.runtime.getURL(''), + ) + }).catch((err) => { console.error('Clipboard write failed', err) + iframe.contentWindow?.postMessage( + { + action: 'clipboard-copy-error', + payload: err.message, + }, + chrome.runtime.getURL(''), + ) }) }
54-65: Potential UI flicker when taking screenshotsThe current implementation hides and then shows the iframe when taking a screenshot, which could cause a visible UI flicker. Consider adding a transition effect or exploring alternative methods that don't require hiding the sidebar.
if (action === 'get-screenshot-image') { - iframe.style.width = '0px' + // Store the current width before hiding + const currentWidth = iframe.style.width + iframe.style.transition = 'none' // Disable transitions temporarily + iframe.style.width = '0px' const image = await getScreenshotImage() - iframe.style.width = '400px' + iframe.style.width = currentWidth + // Re-enable transitions after a brief delay + setTimeout(() => { + iframe.style.transition = 'width 0.3s ease' + }, 50) iframe.contentWindow?.postMessage( { action: 'get-screenshot-image', payload: image, }, - '*', + chrome.runtime.getURL(''), ) }
31-34: Improve type safety for message handlingUsing
anyfor the payload type reduces type safety. Consider creating a proper type definition for the expected message structure.+ type MessageEvent = { + action: 'get-page-content' | 'copy-to-clipboard' | 'get-screenshot-image'; + _payload?: { + content?: string; + }; + }; + window.addEventListener('message', async (event) => { - const { action, _payload } = event.data as { - action: string - _payload: any - } + const { action, _payload } = event.data as MessageEvent;.wxt/tsconfig.json (2)
23-26: Broad Inclusion Patterns – Confirm IntentThe
"include"pattern["../**/*", "./wxt.d.ts"]is quite broad and may include more files than strictly necessary. Ensure that this pattern intentionally targets only the required source files and does not inadvertently compile unrelated files, which could affect build performance.
27-27: Exclude Configuration CheckExcluding the
"../.output"directory is appropriate for preventing build artifacts from being processed. Consider reviewing if additional directories (such asnode_modulesor other build directories) should also be excluded to further optimize the compilation process.wxt.config.ts (2)
33-40: Consider using strict equality for manifest version checkThe condition uses
!=for comparison. For better TypeScript practices, consider using strict equality (!==) instead.- if (import.meta.env.MANIFEST_VERSION != 2) { + if (import.meta.env.MANIFEST_VERSION !== 2) {
45-67: Missing documentation for background and content scriptsAccording to the PR summary, background and content scripts sections were removed. Consider adding a comment explaining where these are now defined in the WXT framework structure.
export default defineConfig({ extensionApi: "webextension-polyfill", srcDir: "src", manifest: generateManifest(), + // Note: Background scripts and content scripts are now defined in their respective + // entrypoint files under the src/ directory according to WXT conventions hooks: {
🛑 Comments failed to post (7)
src/entrypoints/background/quick-menu/forwardContextMenu.ts (1)
5-5: 💡 Verification agent
❓ Verification inconclusive
Updated URL path to align with WXT convention
The URL path has been updated from a nested structure ('/src/pages/settings/index.html') to the standard WXT convention ('/options.html'). This change is appropriate for the WXT migration and maintains the same functionality.
Make sure the '/options.html' page exists and contains the proper settings UI.
🏁 Script executed:
#!/bin/bash # Verify that the options.html file exists in the project ls -la *.html | grep options.htmlLength of output: 91
I'm not seeing evidence that the options page exists at the expected location. Let's re-run a more comprehensive search to verify if there's any file named options.html in the repository:
Please run this script and share the results so we can confirm the presence and correct location of the options page.
🏁 Script executed:
#!/bin/bash # Recursively search the repository for options.html find . -type f -name "options.html"Length of output: 157
Missing options.html File Detected – Action Needed
The URL path has been updated to '/options.html' to follow the WXT migration conventions. However, our search did not find any file named "options.html" in the repository. Please confirm whether:
- The "options.html" file exists but is located elsewhere (if so, update the URL accordingly), or
- The file was inadvertently omitted and needs to be added to support the settings UI.
Ensure this is resolved so the settings page can be properly accessed in the extension.
tsconfig.json (1)
1-3:
⚠️ Potential issueCritical Configuration Issue: "extends" Property Points to a Removed File
The"extends": ".wxt/tsconfig.json"line now references a file that has been deleted in this migration. This can lead to TypeScript compilation errors. Please update the inheritance reference to point to a valid configuration file or remove it if no longer needed. For example, if the intention is to have this as the primary configuration file, consider removing the extends property:- "extends": ".wxt/tsconfig.json",📝 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.{ "compilerOptions": {src/entrypoints/quick-menu.content/index.ts (1)
28-37: 🛠️ Refactor suggestion
Add null/error checking for prompt retrieval
The code assumes
findPrompt(id)will always return an object with a validpromptproperty. This could lead to runtime errors if the prompt isn't found.chrome.runtime.onMessage.addListener(async (request) => { const { payload } = request const { selectedText, id } = payload || {} if (selectedText && id) { - const prompt = (await findPrompt(id)).prompt - if (prompt) { - generatePromptInSidebar(prompt, selectedText) - } + try { + const promptResult = await findPrompt(id); + if (promptResult && promptResult.prompt) { + generatePromptInSidebar(promptResult.prompt, selectedText) + } + } catch (error) { + console.error('Error finding prompt:', error); + } } })📝 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.}) chrome.runtime.onMessage.addListener(async (request) => { const { payload } = request const { selectedText, id } = payload || {} if (selectedText && id) { try { const promptResult = await findPrompt(id); if (promptResult && promptResult.prompt) { generatePromptInSidebar(promptResult.prompt, selectedText) } } catch (error) { console.error('Error finding prompt:', error); } } })src/entrypoints/content/index.ts (3)
1-3:
⚠️ Potential issueMissing WXT framework import for
defineContentScriptThe
defineContentScriptfunction is used on line 69 but not imported. You need to add an import statement for this function from the WXT framework.import { getScreenshotImage } from '../../lib/getScreenshotImage' import { contentScriptLog } from '../../logs' +import { defineContentScript } from 'wxt/sandbox'📝 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.import { getScreenshotImage } from '../../lib/getScreenshotImage' import { contentScriptLog } from '../../logs' import { defineContentScript } from 'wxt/sandbox'
15-15: 💡 Verification agent
🧩 Analysis chain
Path reference may need updating for WXT
The URL path '/src/pages/sidebar/index.html' appears to be using the old file structure. In WXT, the paths might work differently. Verify if this should be updated to match the new entrypoints structure.
🏁 Script executed:
#!/bin/bash # Check if the sidebar HTML file exists at the specified path if [ -f "src/pages/sidebar/index.html" ]; then echo "File exists at the current path" else echo "File NOT found at the current path" # Check if it exists in the new structure if [ -f "src/entrypoints/sidebar/index.html" ]; then echo "File exists in the entrypoints directory structure" fi fiLength of output: 279
Action Required: Update Sidebar HTML File Reference for WXT
The shell script output confirms that the old file path (
src/pages/sidebar/index.html) no longer exists, and the correct file is located atsrc/entrypoints/sidebar/index.html. Please update the URL reference insrc/entrypoints/content/index.ts(line 15) to match the new entrypoints structure.
30-45: 🛠️ Refactor suggestion
Security concern with postMessage origin
Using '*' as the target origin in postMessage allows any website to receive the messages, which is a potential security risk. Consider specifying the extension's origin explicitly.
iframe.contentWindow?.postMessage( { action: 'get-page-content', payload: pageContent, }, - '*', + chrome.runtime.getURL(''), )Also apply the same change to the other postMessage call on line 63.
📝 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.// Instance from lines 30-45 (example context) iframe.contentWindow?.postMessage( { action: 'get-page-content', payload: pageContent, }, chrome.runtime.getURL(''), ) // ... other code ... // Update for the other postMessage call on line 63 iframe.contentWindow?.postMessage( { // Presumably similar keys; adjust as needed according to actual usage. action: 'another-action', payload: anotherPayload, }, chrome.runtime.getURL(''), ).wxt/types/imports.d.ts (1)
39-51: 💡 Verification agent
🧩 Analysis chain
Consider the necessity of @ts-ignore comments.
The multiple
@ts-ignorecomments suggest potential type-checking issues with the global type exports. These might be necessary for WXT's approach to global type availability, but they bypass TypeScript's type checking.Are these
@ts-ignorecomments standard practice in WXT projects? It might be worth checking if there's a way to structure the types that doesn't require suppressing TypeScript errors.
🌐 Web query:
Does WXT require @ts-ignore for global type definitions?💡 Result:
WXT does not require
@ts-ignorefor global type definitions when configured properly. The framework provides multiple supported methods for handling type declarations:1. Declaration Files
WXT allows you to generate type declaration files through modules[[1][4]]:
// Module generating a declaration file wxt.hook('prepare:types', async (_, entries) => { entries.push({ path: 'my-module/types.d.ts', text: `declare global { /* type extensions */ }`, tsReference: true // Required for inclusion in TS project }); });This ensures global types are automatically included in the TypeScript project without manual
@ts-ignoreannotations.2. TypeScript Configuration
WXT generates a base
.wxt/tsconfig.jsonand recommends:
- Extending this config in your root
tsconfig.json[[2]]- Using
include/excludepatterns to manage global types[[5][13]]- Avoiding direct modification of TSConfig paths (use WXT's
aliasconfig instead)[[2]]3. Monorepo Example
In shared code setups, aliases and includes ensure type visibility[[5]]:
// tsconfig.json { "include": ["../../packages/ui-base/src/**/*.ts"], "paths": { "@packages/ui-base/*": ["../../packages/ui-base/src/*"] } }// wxt.config.ts alias: { "@packages/ui-base": resolve("../../packages/ui-base/src") }When You Might Need
@ts-ignore:
- Temporarily ignoring type errors in generated/manually modified code
- Working with third-party libraries lacking proper typings
- Transitional code during migrations[[10]]
For most cases, WXT's type system and module architecture eliminate the need for
@ts-ignorewhen global types are properly declared in.d.tsfiles and included via standard configuration methods.Citations:
- 1: https://wxt.dev/guide/essentials/wxt-modules
- 2: https://wxt.dev/guide/essentials/config/typescript
- 3: @ts-ignore for the block scope and imports microsoft/TypeScript#19573
- 4: https://wxt.dev/guide/essentials/config/manifest
- 5: https://weberdominik.com/blog/monorepo-wxt-nextjs/
- 6: https://eslint.org/docs/latest/use/command-line-interface
- 7: https://docs.wxwidgets.org/3.2/group__group__funcmacro__string.html
- 8: https://wxt.dev/guide/installation
- 9: https://docs.aws.amazon.com/prescriptive-guidance/latest/best-practices-cdk-typescript-iac/typescript-best-practices.html
- 10: https://chatgptwriter.ai/blog/migrate-plasmo-to-wxt
- 11: https://wxt.dev/guide/essentials/config/runtime
- 12: https://webpack.js.org/configuration/output/
- 13: Typescript global types/interfaces nuxt/nuxt#22360
- 14: https://wxt.dev/guide/essentials/config/build-mode
- 15: disable certain global types for specific files, or specify type roots for specific files. microsoft/TypeScript#37053
- 16: https://www.youtube.com/watch?v=VzJ-wUYxIow
- 17: https://www.npmjs.com/package/wxt/v/0.19.27
- 18: https://dev.to/maafaishal/ts-expect-error-over-ts-ignore-in-typescript-5140
- 19: Support global type definition files nuxt/nuxt#22220
- 20: https://www.npmjs.com/package/wxt/v/0.18.6
Action Required: Remove Unnecessary
@ts-ignoreCommentsBased on the WXT documentation, global type definitions do not require the use of
@ts-ignorewhen configured correctly. In this case, rather than bypassing TypeScript’s type checking, you can structure your global types using one of the supported approaches:
- Declaration Files: Generate and include a proper declaration file via
wxt.hook('prepare:types', …), so that the types are automatically recognized.- TypeScript Configurations: Utilize the generated
.wxt/tsconfig.json(or extend it in your roottsconfig.json) with appropriateinclude/excludepatterns, eliminating the need for manual suppression.Next Steps:
- Remove or refactor the
@ts-ignorecomments in.wxt/types/imports.d.ts(lines 39–51).- Update your type declaration setup using one of the supported methods as described in the WXT documentation, ensuring full type checking is maintained.
Closes #103
Summary by CodeRabbit
New Features
Enhancements
Chores