refactor: cleanup esm

[lishaduck]

Overview

A few tiny cleanups I noticed could be made while perusing around the codebase.

Manual Testing

Bundle, check it still works.

Related Issue

This PR is stacked on #1945.

[netlify]

✅ Deploy Preview for creative-fairy-df92c4 ready!

Name	Link
🔨 Latest commit	bcec220
🔍 Latest deploy log	https://app.netlify.com/projects/creative-fairy-df92c4/deploys/6989614f5437e700089ab8ee
😎 Deploy Preview	https://deploy-preview-1950--creative-fairy-df92c4.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

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

[aklinker1]

Ugh, this diff is massive, did you rebase it correctly?

[lishaduck]

Ugh, this diff is massive, did you rebase it correctly?

Haven't rebased it yet

[aklinker1]

We can also cleanup this file: https://github.com/wxt-dev/wxt/blob/main/packages/wxt/src/core/utils/eslint.ts

Other than the changes that make Vite a required dependency, the rest looks good.

Also, should this be based on the main branch? Or did you intend this to be a breaking change as well?

[lishaduck]

Oh, apparently I didn't hit submit on my review, sorry for leaving you without guidance.

I was a bit confused, like didn't I just clarify + ask you a followup question? Makes sense.

[lishaduck]

We can also cleanup this file: main/packages/wxt/src/core/utils/eslint.ts

Not sure what you mean, I did, right? eslint is an optional peerDependency, so it still needs the dynamic import.

Other than the changes that make Vite a required dependency, the rest looks good.

Vite is already a required dependency, is still a required dependency following #1945, and I'm skeptical of the multiple builders argument (I'll follow up on that above).
If you're referencing non-dynamic imports of Vite, those also already exist. I thought it might be intentional, but I decided it was just for CJS-compat because there are a few (two?) files that just directly (non-type) import 'vite'.

I'll revert the changes, but I'm pretty lost as to the reasoning.

Also, should this be based on the main branch? Or did you intend this to be a breaking change as well?

I didn't want to target changes that might be wrong, but having messed around with the codebase more, I'm pretty confident with them.

[aklinker1]

The other imports to vite are because I got lazy... I'd prefer to keep it abstracted for now even if it's the only builder WXT supports. Maybe it's about time I clean those other imports up as well...

Undoing the builder abstraction or more tightly coupling Vite to the project is something I will stubbornly refuse to budge on right now, sorry.

[lishaduck]

The other imports to vite are because I got lazy... I'd prefer to keep it abstracted for now even if it's the only builder WXT supports. Maybe it's about time I clean those other imports up as well...

✅ It looked like more but they were just types. Could we turn on verbatimModuleSyntax now?

Undoing the builder abstraction or more tightly coupling Vite to the project is something I will stubbornly refuse to budge on right now, sorry.

No need to be sorry! I'm just curious why it's built this way, I was surprised because I thought WXT was all-in on Vite. Sorry if it comes off confrontational (and I'll try not to, but knowing me I'll probably manage to complain whenever I touch these files for all of eternity until I decide to add support for some shiny new bundler, please just ignore me)

[pkg-pr-new]

Open in StackBlitz

@wxt-dev/analytics

npm i https://pkg.pr.new/@wxt-dev/analytics@1950

@wxt-dev/auto-icons

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

@wxt-dev/browser

npm i https://pkg.pr.new/@wxt-dev/browser@1950

@wxt-dev/i18n

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

@wxt-dev/module-react

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

@wxt-dev/module-solid

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

@wxt-dev/module-svelte

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

@wxt-dev/module-vue

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

@wxt-dev/runner

npm i https://pkg.pr.new/@wxt-dev/runner@1950

@wxt-dev/storage

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

@wxt-dev/unocss

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

@wxt-dev/webextension-polyfill

npm i https://pkg.pr.new/@wxt-dev/webextension-polyfill@1950

wxt

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

commit: bcec220

[aklinker1]

Looks like there are two test failures

[PatrykKuniczak]

@aklinker1 I see there's more people, which want to make this code base, a little more readable.
I don't think more strict eslint and additional stylelint is a bad idea, but you're THE BOSS.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.99%. Comparing base (16a2aed) to head (bcec220).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1950      +/-   ##
==========================================
+ Coverage   75.86%   75.99%   +0.13%     
==========================================
  Files         113      113              
  Lines        3045     3041       -4     
  Branches      685      682       -3     
==========================================
+ Hits         2310     2311       +1     
+ Misses        650      646       -4     
+ Partials       85       84       -1     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Thanks for helping make WXT better!

[lishaduck]

Thanks for sorting that out for me Aaron!

Kept forgetting I had this sittin' around.

[aklinker1]

No worries 👍 Glad we could get it merged.