perf: avoid side effects resolving in dev and in the optimizer/scanner

[patak-dev]

Description

We already had some isBuild checks but they weren't in all the places we resolved side effects. I think we should also avoid resolving side effects during deps optimization (both in the scanner and in esbuild).

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[patak-dev]

34b9886 (#12789) implements an idea by @bluwy to avoid resolving more than needed for all plugins used from createResolver.

[patak-dev]

Perf improvements confirmed 🎉
https://github.com/fi3ework/vite-benchmark/actions/runs/4640793203

[bluwy]

You don't have to make me a co-author, but thanks 🙂 One small nit below and I think this is good!

I also noticed we can push a bit further, for container.resolveId usecases as well like:

vite/packages/vite/src/node/server/moduleGraph.ts

Line 371 in 5ac3a4d

const resolved = alreadyResolved ?? (await this.resolveId(url, !!ssr))

vite/packages/vite/src/node/server/transformRequest.ts

Line 147 in 5ac3a4d

(await pluginContainer.resolveId(url, undefined, { ssr }))?.id ??

but we can do this in another PR if you prefer.

[patak-dev]

I also noticed we can push a bit further, for container.resolveId usecases as well like:

vite/packages/vite/src/node/server/moduleGraph.ts

Line 371 in 5ac3a4d

const resolved = alreadyResolved ?? (await this.resolveId(url, !!ssr))

vite/packages/vite/src/node/server/transformRequest.ts

Line 147 in 5ac3a4d

(await pluginContainer.resolveId(url, undefined, { ssr }))?.id ??

but we can do this in another PR if you prefer.

Oh, good ones! So, should we switch from an option in the resolve plugin to a new option passed to resolveId? Or both like we do with scan? 🤔
Feel free to commit here, or merge and then we work on this idea on another PR. I can try too later today.

[patak-dev]

@bluwy I tried to use idOnly in the two cases above, but I don't know if the complexity is worth it. Let's merge this one and then we can check in a future PR. The thing is that resolving sideEffects is now already avoided in dev, so these two calls would only affect dev SSR externalize logic IIUC. And at least right now, that won't save any compute (only an extra object). Maybe this will make a difference once we refactor the should externalize logic.

[bluwy]

Sounds good to me. We could probably revisit them again in the future.