-
-
Notifications
You must be signed in to change notification settings - Fork 3
Migrate from webextension-toolbox to WXT #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Migrated the project from webextension-toolbox to wxt for improved developer experience and better maintenance. Changes: - Replaced webextension-toolbox with wxt framework - Restructured project: app/ → entrypoints/ + public/ - Updated build configuration in wxt.config.ts - Converted scripts to WXT format: - background.ts: wrapped with defineBackground() - content.ts: wrapped with defineContentScript() - Inlined CSS into content.ts - Updated tsconfig.json to extend WXT's config - Updated npm scripts for wxt commands - Added .wxt/ and .output/ to .gitignore - Fixed secretlint config path in package.json Benefits: - HMR (Hot Module Replacement) for faster development - Vite-based build system (faster than webpack) - Active maintenance and modern tooling - Built-in TypeScript support with auto-generated types - Better cross-browser support (Chrome MV3, Firefox MV2, Edge MV3) All builds (Chrome, Firefox, Edge) tested successfully.
Summary of ChangesHello @yamadashy, 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 performs a comprehensive migration of the project's build system from the legacy Highlights
Using Gemini Code AssistThe 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
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 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
|
There was a problem hiding this 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 is a great initiative to modernize the project's build system by migrating from the unmaintained webextension-toolbox to WXT. The changes are well-structured and follow the migration path outlined in the description. I've identified a few areas for improvement:
- A potentially redundant background script that can be removed to simplify the code and reduce permissions.
- A race condition in the content script's
MutationObserversetup that could affect reliability. - An opportunity to make the
wxt.config.tsmore maintainable.
Overall, this is a solid migration, and addressing these points will make the extension more robust and secure.
| document.addEventListener('DOMContentLoaded', () => { | ||
| addCodeWikiButton(); | ||
|
|
||
| // Handle GitHub SPA navigation | ||
| let lastUrl = location.href; | ||
| let isProcessing = false; | ||
|
|
||
| const observer = new MutationObserver((mutations: MutationRecord[]) => { | ||
| if (isProcessing) return; | ||
| isProcessing = true; | ||
|
|
||
| const url = location.href; | ||
| if (url !== lastUrl) { | ||
| lastUrl = url; | ||
| setTimeout(() => { | ||
| addCodeWikiButton(); | ||
| isProcessing = false; | ||
| }, 500); | ||
| return; | ||
| } | ||
|
|
||
| // Monitor navigation element addition (only if button doesn't exist) | ||
| const navActions = document.querySelector<HTMLUListElement>('ul.pagehead-actions'); | ||
| const codeWikiButton = document.querySelector('.codewiki-button'); | ||
| if (navActions && !codeWikiButton) { | ||
| addCodeWikiButton(); | ||
| } | ||
|
|
||
| isProcessing = false; | ||
| }); | ||
|
|
||
| observer.observe(document.body, { | ||
| childList: true, | ||
| subtree: true, | ||
| attributes: false, | ||
| characterData: false, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The MutationObserver is currently initialized inside the DOMContentLoaded event listener. This can lead to a race condition where DOM mutations occur before the listener fires and the observer is attached, especially since the script is configured with runAt: 'document_start'. This could cause the button to not appear reliably.
To ensure all DOM changes are captured from the beginning, the MutationObserver should be initialized and started at the top level of the main function, and the DOMContentLoaded wrapper can be removed.
// Handle GitHub SPA navigation
let lastUrl = location.href;
let isProcessing = false;
const observer = new MutationObserver(() => {
if (isProcessing) return;
isProcessing = true;
const url = location.href;
if (url !== lastUrl) {
lastUrl = url;
setTimeout(() => {
addCodeWikiButton();
isProcessing = false;
}, 500);
return;
}
// Monitor navigation element addition (only if button doesn't exist)
const navActions = document.querySelector<HTMLUListElement>('ul.pagehead-actions');
const codeWikiButton = document.querySelector('.codewiki-button');
if (navActions && !codeWikiButton) {
addCodeWikiButton();
}
isProcessing = false;
});
observer.observe(document.body, {
childList: true,
subtree: true,
});| const injectContentToTab = async (tab: chrome.tabs.Tab): Promise<void> => { | ||
| // Skip if URL is undefined | ||
| if (!tab.url) { | ||
| return; | ||
| } | ||
|
|
||
| // Skip if tab is discarded | ||
| if (tab.discarded) { | ||
| return; | ||
| } | ||
|
|
||
| // Skip if tab ID is undefined | ||
| if (tab.id === undefined) { | ||
| return; | ||
| } | ||
|
|
||
| // Skip if not a GitHub URL | ||
| if (!tab.url.startsWith('https://github.com/')) { | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const manifest = chrome.runtime.getManifest(); | ||
|
|
||
| // Inject CSS | ||
| if (manifest.content_scripts?.[0]?.css) { | ||
| await chrome.scripting.insertCSS({ | ||
| target: { tabId: tab.id }, | ||
| files: manifest.content_scripts[0].css, | ||
| }); | ||
| } | ||
|
|
||
| // Inject JavaScript | ||
| if (manifest.content_scripts?.[0]?.js) { | ||
| await chrome.scripting.executeScript({ | ||
| target: { tabId: tab.id }, | ||
| files: manifest.content_scripts[0].js, | ||
| }); | ||
| } | ||
| } catch (error) { | ||
| console.error('Error injecting content script:', error); | ||
| } | ||
| }; | ||
|
|
||
| // Update extension content for tabs | ||
| chrome.tabs.query({}, async (tabs: chrome.tabs.Tab[]) => { | ||
| for (const tab of tabs) { | ||
| try { | ||
| await injectContentToTab(tab); | ||
| } catch (e) { | ||
| console.error(e); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This script for programmatically injecting the content script appears to be a holdover from the previous build system and is likely redundant. Modern browsers automatically inject manifest-defined content scripts into existing, matching tabs upon extension installation or update.
Furthermore, the current implementation is brittle as it relies on the specific structure of the auto-generated manifest (manifest.content_scripts[0]).
Recommendation:
- Remove the logic inside
defineBackground. - Remove the
"scripting"permission fromwxt.config.tsas it will no longer be needed. This reduces the extension's permission footprint, which is a security best practice.
// This logic has been removed as it is redundant.
// Browsers automatically inject content scripts on install/update.
// The "scripting" permission in `wxt.config.ts` should also be removed.| resources: [ | ||
| 'images/icon-16.png', | ||
| 'images/icon-19.png', | ||
| 'images/icon-32.png', | ||
| 'images/icon-38.png', | ||
| 'images/icon-48.png', | ||
| 'images/icon-64.png', | ||
| 'images/icon-128.png', | ||
| ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added 'tabs' permission to enable the background script to inject content scripts into existing GitHub tabs when the extension is installed or updated. This ensures the extension works on already open tabs without requiring a page reload.
Fixed permission error by filtering tabs to only GitHub URLs in chrome.tabs.query(). Previously queried all tabs, causing permission errors when trying to inject scripts into non-GitHub tabs. Changes: - Added url filter 'https://github.com/*' to chrome.tabs.query() - No additional permissions needed (tabs permission not required with url filter) - Maintains same permissions as original webextension-toolbox version
Added check for document.head existence before injecting CSS. Since runAt is 'document_start', document.head may not be available yet. Uses MutationObserver to wait for document.head if needed. Fixes: Uncaught TypeError: Cannot read properties of null (reading 'appendChild')
Summary
Migrated the project from webextension-toolbox to WXT for improved developer experience and better maintenance support.
Key Changes
Dependencies:
wxtas the new build framework@webextension-toolbox/webextension-toolbox(last updated 4 years ago)Project Structure:
Code Migration:
background.tsto WXT format usingdefineBackground()content.tsto WXT format usingdefineContentScript()Configuration:
wxt.config.tswith manifest settingstsconfig.jsonto use WXT's type definitions.wxt/and.output/to.gitignoreBenefits
🚀 Better Developer Experience:
🔧 Active Maintenance:
🌐 Better Cross-Browser Support:
Testing
All builds have been tested and verified:
New Commands
Breaking Changes
None. The extension functionality remains exactly the same. Only the build tooling has changed.
Migration Notes
After merging, developers should:
npm installto update dependenciesnpm run devinstead of old commands).output/instead ofdist/