fix(ssr): replace __vite_ssr_identity__ with (0, ...) and inject ; between statements

[hi-ogawa]

Description

• related SSR transformed named imported functions don't work with debuggers "step-into" #18325

I tried a simple heuristics of injecting ; between statements to ensure (0, ...) won't break syntax. I would appreciate if anyone can find edge cases with this approach.

I tested Ari's reproduction and "step-into" now jumps back once, which is the original behavior Ari reported.

[pkg-pr-new]

Open in Stackblitz

npm i https://pkg.pr.new/vite@18748

commit: 54a6bc1

[hi-ogawa]

There are a few TypeError: {(intermediate value)} is not a function and this is due to a bad transform.

• https://github.com/vitest-dev/vitest/blob/021944cd588958f3fa22d632e2bfd3efa0f1310d/test/core/test/strict.test.js#L18-L25
• https://astexplorer.net/#/gist/f638202900991057882bf1d1c85bb2e9/1ad2ace1a3c0de3b3792d4ea60dfad255c549910

// this becomes CallExpression { get x() {... } }(0, ...) 
const obj2 = {
  get x() {
    return 17
  },
}
(0,__vite_ssr_import_0__.expect)(() => {
  obj2.x = 5
}).toThrowError(TypeError)

It looks like } heuristic needs to be a bit smarter.

[hi-ogawa]

I think ecosystem ci looks good. Vitest failure is because typecheck mode relies on static analysis of ssr transform and I need to update it once more like I did for __vite_ssr_identity__ in vitest-dev/vitest#6867.

[vite-ecosystem-ci]

📝 Ran ecosystem CI on b965337: Open

suite	result	latest scheduled
astro	❌ failure	❌ failure
histoire	❌ failure	❌ failure
redwoodjs	❌ failure	❌ failure
remix	❌ failure	❌ failure
sveltekit	❌ failure	❌ failure
vike	❌ failure	❌ failure
vitest	❌ failure	✅ success
waku	❌ failure	❌ failure

✅ analogjs, ladle, laravel, marko, nuxt, previewjs, quasar, qwik, rakkas, storybook, unocss, vite-environment-examples, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-swc, vite-plugin-svelte, vite-plugin-vue, vite-setup-catalogue, vitepress, vuepress

[sapphi-red]

I didn't find any cases that gets broken. Does this affect performance?

[hi-ogawa]

Does this affect performance?

Do you mean perf of transform or runtime? I didn't check anything yet though. For transform, I don't think it would affect significantly since it's reusing the same walk pass and new logic only add extra loops for node.body array (+ adding more magic string chunks for injecting ;)

[sapphi-red]

I mean the performance of the transform. Yeah, the magic-string chunks was what I was a bit worried about.

[hi-ogawa]

I added a simple bench and for this one, there's no difference other than some noise. Each run changes around 0.99 - 1.01.

pnpm bench --compare bench.json

 ✓ packages/vite/src/node/ssr/__tests__/ssrTransform.bench.ts (1) 606ms
     name          hz     min     max    mean     p75     p99    p995    p999     rme  samples
   · basic  13,045.63  0.0521  0.4151  0.0767  0.0771  0.2320  0.2664  0.3544  ±1.20%     6523  [0.99x] ⇓
     basic  13,221.92  0.0581  1.7304  0.0756  0.0730  0.2316  0.2761  0.3743  ±1.32%     6611  (baseline)

I'll check with bigger one next.

[hi-ogawa]

Testing it again, it looks like this makes it slower consistently (baseline is main).

✓ packages/vite/src/node/ssr/__tests__/ssrTransform.bench.ts (2) 10781ms
     name          hz      min     max    mean     p75     p99    p995    p999     rme  samples
   · basic  14,863.13   0.0542  1.3083  0.0673  0.0636  0.1931  0.2162  0.2794  ±0.26%    74316  [0.97x] ⇓
     basic  15,399.97   0.0517  1.7042  0.0649  0.0618  0.1872  0.2078  0.2628  ±0.27%    77000  (baseline)
   · large     8.9687   100.12  129.34  111.50  116.44  129.34  129.34  129.34  ±1.95%       45  [0.91x] ⇓
     large     9.8136  92.7499  118.97  101.90  109.35  118.97  118.97  118.97  ±1.94%       50  (baseline)

But I'm not sure if it's significant for overall transform pipeline.

[patak-dev]

I think the perf regression is ok, it should work first and be fast second.

[sapphi-red]

The perf looks fine to me. Thanks for checking! I pushed a commit that fixes one case I found.

[hi-ogawa]

Oh, nice catch of switch case 👍 I cleaned up bench since it was referencing a file on my local.