wxt
wxt copied to clipboard
wxt-dev
refactor: cleanup esm
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.
Deploy Preview for creative-fairy-df92c4 ready!
| Name | Link |
|---|---|
| Latest commit | cf3605b89671c34ad86affdaa5dd29756a63de3d |
| Latest deploy log | https://app.netlify.com/projects/creative-fairy-df92c4/deploys/6901a765efdc900009234380 |
| 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.
![netlify[bot] avatar](https://avatars.githubusercontent.com/in/13473?v=4 "Published by a pub.dev verified publisher"){kind=small}
Ugh, this diff is massive, did you rebase it correctly?
Haven't rebased it yet
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.
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.
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)