perf: reduce size of injected __vite__mapDeps code

[panstromek]

Description

This reduces size of injected __vite__mapDeps code by roughly a factor of 2 (not counting the dependency list) using few minification tricks.

The main ideas are:

• replace function with arrow function, which is shorter (Especially when braces can be ommited)
• extract dependency list to a separate variable which
  • allows both variables to be grouped
  • avoid referencing the long names multiple times
• remove redundant whitespace and parens
• shorten parameter names

Note that it also removes the lazy initialization of the dependency list array. I honestly don't see the benefit of doing it, but if it's desirable to keep it, there's another version of the function that keeps the behaviour unchanged (except for renaming the internal function object property, which is also optional, though):

const __vite__mapDeps=(i,f=__vite__mapDeps,d=(f.f||(f.f=[/*dependency list*/])))=>i.map(i=>d[i])

This version uses optional parameters to:

• assign function name to a variable to avoid referencing the long name multiple times
• lazy intialize the internal file list property and assign the result to local variable
• optional parameters are used to avoid adding parenthesis to the function and adding const|let keyword

[edit]
I found a comment on original PR that explains why the lazy initialization is there. https://github.com/vitejs/vite/pull/14550/files#r1350449734

I might have to re-add it, but let's see if there's a demand for merging this in the first place. I tested this locally on my project and patched it in GitHub UI, so I haven't seen the impact on tests, yet.

Additional context

This optimization was first added here: #14550

cc @gajus who contributed the original implemetation.

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines, especially the Pull Request Guidelines.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Update the corresponding documentation if needed.
• Ideally, include relevant tests that fail without this PR but pass with it.

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[panstromek]

Lint failed but I couldn't find any linter output, so I don't see what is the problem

[bluwy]

Thanks! This looks great to me. Something I'd also like to followup later is to remove the mapDeps altogether if no preloading is needed for the file. Currently it's added unconditionally. And the changes could get a bit big.

Also related: #15300 (comment)

[panstromek]

Thanks! This looks great to me. Something I'd also like to followup later is to remove the mapDeps altogether if no preloading is needed for the file. Currently it's added unconditionally. And the changes could get a bit big.

Yea, I noticed that too. It can also be removed when there's only single callsite, which is another pretty common case (at least in my codebase).

(Even more generalized version could also omit it when the imported files at each callsite are distinct or distinct enough to outweigh the added code)

[bluwy]

I think this should be a simple fix in a patch and not for 5.3 🤔

[patak-dev]

In the past, there have been downstream projects doing regexes over our preload handling to inject or modify stuff. So I'm always a bit worried about touching this part. I'll trigger a ecosystem CI run. I'm ok if you want to move forward with this in a patch if it is all green.

/ecosystem-ci run

[sapphi-red]

I felt this to be in the blur between minor and patch. If the ecosystem-ci passes, I'm fine with merging it with a patch.
BTW the "/ecosystem-ci run" needs to be in the beginning of the comment (

vite/.github/workflows/ecosystem-ci-trigger.yml

Line 10 in d7c5256

if: github.repository == 'vitejs/vite' && github.event.issue.pull_request && startsWith(github.event.comment.body, '/ecosystem-ci run')

).

[sapphi-red]

/ecosystem-ci run

[vite-ecosystem-ci]

📝 Ran ecosystem CI on 0418a94: Open

suite	result	latest scheduled
nuxt	❌ failure	✅ success

✅ analogjs, astro, histoire, ladle, laravel, marko, previewjs, quasar, qwik, rakkas, remix, sveltekit, unocss, vike, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-pages, vite-plugin-react-swc, vite-plugin-svelte, vite-plugin-vue, vite-setup-catalogue, vitepress, vitest

[bluwy]

The Nuxt fail seems like a fluke since it's a dev fixture test, but I reran it again. I haven't seen downstream patching this in the past, so I figured it should be safe (but still, it's on them).

[vite-ecosystem-ci]

📝 Ran ecosystem CI on 0418a94: Open

suite	result	latest scheduled
nuxt	❌ failure	✅ success

✅ analogjs, astro, histoire, ladle, laravel, marko, previewjs, quasar, qwik, rakkas, remix, sveltekit, unocss, vike, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-pages, vite-plugin-react-swc, vite-plugin-svelte, vite-plugin-vue, vite-setup-catalogue, vitepress, vitest

[sapphi-red]

I guess it's failing because the base commit was a bit old.

[sapphi-red]

/ecosystem-ci run

[vite-ecosystem-ci]

📝 Ran ecosystem CI on 3e158e6: Open

✅ analogjs, astro, histoire, ladle, laravel, marko, nuxt, previewjs, quasar, qwik, rakkas, remix, sveltekit, unocss, vike, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-pages, vite-plugin-react-swc, vite-plugin-svelte, vite-plugin-vue, vite-setup-catalogue, vitepress, vitest

[bluwy]

Thanks for resolving it Sapphi, the fail had me defeated 😄 Since it passes on ecosystem-ci, I'll go ahead and merge this then.

[panstromek]

Thanks for carrying this through!

[skovhus]

Hi. Just found this which caused a regression in our codebase when upgrading to 5.3.x. We use the renderBuiltUrl to update the asset URL based on some configuration.

Vite config:

    experimental: {
      renderBuiltUrl(filename: string, { hostType }) {
        if (hostType === "js") {
          return { runtime: `window.__toStaticUrl(${JSON.stringify(filename)})` };
        } else {
          return { relative: true };
        }
      },
    },

window.__toStaticUrl is depending on some configuration and is loaded in a file we call preload.ts before our main file entry.ts. Snippet from our index.html file:

<script type="module" src="/src/vite/preload.ts"></script>
<script type="module" src="/src/vite/entry.ts"></script>

In prouduction vite bundles preload and entry into one chunk. This used to work fine, but the challenge with the change in this PR is that instead of lazy evaluating window.__toStaticUrl when building the mapDeps, we now expect the function to be defined beforehand. As the function is not defined we get a TypeError: window.__toStaticUrl is not a function.

What is the suggested workaround here to ensure that the window.__toStaticUrl function is defined before it's used by the mapDeps?

[AlexTugarev]

Facing the same issue as @skovhus here.
A possible workaround is to inject __toStaticUrl via transformIndexHtml plugin, but the downside is that it's getting harder to use import.meta.env.

[bluwy]

To workaround this, you probably need to use a non type="module" script tag so that it doesn't get bundled by Vite and merged together, which would cause the __vite__mapDeps code to be hoisted up and miss the window.__toStaticUrl declaration.

However, I think it's a good point and perhaps we should walk back on this slightly. @panstromek had an alternative at hand for this case

Note that it also removes the lazy initialization of the dependency list array. I honestly don't see the benefit of doing it, but if it's desirable to keep it, there's another version of the function that keeps the behaviour unchanged (except for renaming the internal function object property, which is also optional, though):

const __vite__mapDeps=(i,f=__vite__mapDeps,d=(f.f||(f.f=[/*dependency list*/])))=>i.map(i=>d[i])

I'm fine if we switch to that to fix this for now.

---

Feel free to send a PR if anyone's interested, or submit an issue for this. I'll bring this up to the other maintainers of the fix.

[skovhus]

Thanks @bluwy!

In our case I have some code that reads a configuration / env vars, so ideally I would use the same toolchain instead of having this special case were we cannot use Vite for bundling. Not sure what the performance/space impact actually is here?

I created #17575