Skip to content
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

fix: resolve wxt modules from the root #1417

Merged
merged 4 commits into from
Feb 13, 2025

Conversation

aleclarson
Copy link
Contributor

Overview

I have a local clone of WXT which I make fixes on and link into my web extension project for manual testing. My project is using a "WXT module" which is not being found by the current logic (before this PR). The reason is that WXT does a basic import(…) of each string in the modules array, which doesn’t work when the wxt package is symlinked into the project.

I thought about using import-meta-resolve for a more comprehensive solution. For simplicity, I went with the same approach you took with resolveWxtModuleDir which is just using createRequire from the node:module API.

Manual Testing

Related Issue

Copy link

netlify bot commented Feb 11, 2025

Deploy Preview for creative-fairy-df92c4 ready!

Name Link
🔨 Latest commit a71a1e8
🔍 Latest deploy log https://app.netlify.com/sites/creative-fairy-df92c4/deploys/67acc933dc96460008d0c238
😎 Deploy Preview https://deploy-preview-1417--creative-fairy-df92c4.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

modulesDir: string,
modules: string[] = [],
): Promise<WxtModuleWithMetadata<any>[]> {
const nodeRequire = createRequire(path.join(rootDir, 'index.js'));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

index.js is not required to exist, but we cannot use a directory with createRequire or else the directory itself won't be searched

Copy link

pkg-pr-new bot commented Feb 12, 2025

Open in Stackblitz

@wxt-dev/auto-icons

npm i https://pkg.pr.new/@wxt-dev/auto-icons@1417

@wxt-dev/module-react

npm i https://pkg.pr.new/@wxt-dev/module-react@1417

@wxt-dev/i18n

npm i https://pkg.pr.new/@wxt-dev/i18n@1417

@wxt-dev/module-solid

npm i https://pkg.pr.new/@wxt-dev/module-solid@1417

@wxt-dev/module-svelte

npm i https://pkg.pr.new/@wxt-dev/module-svelte@1417

@wxt-dev/module-vue

npm i https://pkg.pr.new/@wxt-dev/module-vue@1417

@wxt-dev/storage

npm i https://pkg.pr.new/@wxt-dev/storage@1417

@wxt-dev/unocss

npm i https://pkg.pr.new/@wxt-dev/unocss@1417

wxt

npm i https://pkg.pr.new/wxt@1417

commit: a71a1e8

Copy link

codecov bot commented Feb 12, 2025

Codecov Report

Attention: Patch coverage is 78.57143% with 3 lines in your changes missing coverage. Please review.

Project coverage is 80.98%. Comparing base (0ff964a) to head (a71a1e8).
Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
packages/wxt/src/core/resolve-config.ts 78.57% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1417      +/-   ##
==========================================
- Coverage   81.25%   80.98%   -0.28%     
==========================================
  Files         128      128              
  Lines        6296     6284      -12     
  Branches     1070     1067       -3     
==========================================
- Hits         5116     5089      -27     
- Misses       1165     1180      +15     
  Partials       15       15              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

packages/wxt/src/core/resolve-config.ts Outdated Show resolved Hide resolved
packages/wxt/src/core/resolve-config.ts Outdated Show resolved Hide resolved
packages/wxt/src/core/resolve-config.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@aklinker1 aklinker1 left a comment

Choose a reason for hiding this comment

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

Alright, let's give it a go!

@aklinker1 aklinker1 merged commit 342057b into wxt-dev:main Feb 13, 2025
18 checks passed
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.

2 participants