refactor: cleanup esm #1950

lishaduck · 2025-10-24T18:48:26Z

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 · 2025-10-24T18:48:33Z

✅ Deploy Preview for creative-fairy-df92c4 ready!

Name	Link
🔨 Latest commit	`cf3605b`
🔍 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.

aklinker1 · 2025-10-27T20:53:11Z

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

lishaduck · 2025-10-27T21:09:15Z

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?

packages/wxt/src/core/builders/vite/index.ts

packages/wxt/src/core/runners/index.ts

packages/wxt/src/core/utils/wsl.ts

packages/wxt/src/core/resolve-config.ts

packages/wxt/src/core/wxt.ts

packages/wxt/package.json

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.

packages/wxt/src/core/resolve-config.ts

packages/wxt/package.json

packages/wxt/src/core/resolve-config.ts

packages/wxt/src/core/utils/wsl.ts

lishaduck · 2025-10-28T00:14:04Z

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 · 2025-10-28T23:35:25Z

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 · 2025-10-29T05:25:51Z

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)

As far as I could tell, the original reasons are no longer reproducible.

Since wxt-dev#848, there's no need to do all this async work. At least, as far as I could tell...

These seem harmless enough I'll leave them in ig

lishaduck · 2025-10-29T05:38:25Z

packages/wxt/src/core/resolve-config.ts

+        const [resolvedInlineConfig = {}, resolvedUserConfig = {}] =
+          await Promise.all([inlineConfig.vite?.(env), userConfig.vite?.(env)]);


This could potentially be a race condition if someone is doing something stupid in their config file. Probably fine.

lishaduck · 2025-10-29T05:38:58Z

packages/wxt/src/modules.ts

+      const [vite, fromUser] = await Promise.all([
+        import('vite'),
+        userVite(env),
+      ]);


lishaduck · 2025-10-29T05:40:07Z

packages/wxt/src/core/wxt.ts

+  const [builder, pm] = await Promise.all([
+    createViteBuilder(config, hooks, () => wxt.server),
+    createWxtPackageManager(config.root),
+  ]);


We own these, should be fine, but could potentially call something user created and cause a new race condition... yeah this is ridiculous.

lishaduck · 2025-10-29T05:40:41Z

scripts/upgrade-deps.ts

    const latestVersion = dep.info['dist-tags'].latest;
    const latestVersionReleasedAt = new Date(dep.info.time[latestVersion]);

-    semver.RELEASE_TYPES;


oxclint was complaining and I figured I'd be nice to it.
Not sure why check didn't notice it tho, a bit concerning (cause it's in the root prob?).

lishaduck requested review from Timeraa and aklinker1 as code owners October 24, 2025 18:48

lishaduck force-pushed the cleanup-esm branch from ea11b14 to 8558aa7 Compare October 25, 2025 02:18

lishaduck changed the base branch from main to major October 26, 2025 01:38

lishaduck force-pushed the cleanup-esm branch 2 times, most recently from c603e7d to 8fab501 Compare October 27, 2025 22:36

aklinker1 requested changes Oct 27, 2025

View reviewed changes

lishaduck commented Oct 28, 2025

View reviewed changes

lishaduck force-pushed the cleanup-esm branch from 53af521 to 4ba7209 Compare October 29, 2025 05:18

lishaduck changed the base branch from major to main October 29, 2025 05:27

lishaduck force-pushed the cleanup-esm branch from 4ba7209 to a7b6b8c Compare October 29, 2025 05:28

lishaduck added 2 commits October 29, 2025 00:32

chore: cleanup esm

67dab8e

As far as I could tell, the original reasons are no longer reproducible.

refactor: use static imports now that wxt is esm-only

0e5263b

Since wxt-dev#848, there's no need to do all this async work. At least, as far as I could tell...

lishaduck force-pushed the cleanup-esm branch from a7b6b8c to 0e5263b Compare October 29, 2025 05:33

chore: changes i made while hunting stuff down

cf3605b

These seem harmless enough I'll leave them in ig

lishaduck requested a review from aklinker1 October 29, 2025 05:37

lishaduck commented Oct 29, 2025

View reviewed changes

		const [resolvedInlineConfig = {}, resolvedUserConfig = {}] =
		await Promise.all([inlineConfig.vite?.(env), userConfig.vite?.(env)]);

Uh oh!

Uh oh!

refactor: cleanup esm #1950

Are you sure you want to change the base?

refactor: cleanup esm #1950

Uh oh!

Conversation

lishaduck commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Manual Testing

Related Issue

Uh oh!

netlify bot commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for creative-fairy-df92c4 ready!

Uh oh!

aklinker1 commented Oct 27, 2025

Uh oh!

lishaduck commented Oct 27, 2025

Uh oh!

aklinker1 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lishaduck left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lishaduck commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aklinker1 commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lishaduck commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lishaduck Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

lishaduck Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

lishaduck Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

lishaduck Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lishaduck commented Oct 24, 2025 •

edited

Loading

netlify bot commented Oct 24, 2025 •

edited

Loading

aklinker1 left a comment •

edited

Loading

lishaduck commented Oct 28, 2025 •

edited

Loading

aklinker1 commented Oct 28, 2025 •

edited

Loading

lishaduck commented Oct 29, 2025 •

edited

Loading