fix(ssr): replace `__vite_ssr_identity__` with `(0, ...)` and inject `;` between statements by hi-ogawa · Pull Request #18748 · vitejs/vite

hi-ogawa · 2024-11-25T03:42:34Z

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 · 2024-11-25T03:44:49Z

Open in Stackblitz

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

commit: 54a6bc1

hi-ogawa · 2024-11-25T04:36:24Z

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

// 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 · 2024-11-25T05:23:57Z

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 · 2024-11-25T05:33:38Z

📝 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 · 2024-11-25T08:45:34Z

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

hi-ogawa · 2024-11-25T09:04:43Z

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 · 2024-11-25T09:08:25Z

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

hi-ogawa · 2024-11-25T09:19:13Z

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 · 2024-11-25T10:25:01Z

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-cat · 2024-11-25T13:19:56Z

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

sapphi-red · 2024-11-26T04:53:53Z

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

hi-ogawa · 2024-11-26T05:35:29Z

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

| datasource | package | from | to | | ---------- | ------- | ------ | ----- | | npm | vite | 5.4.11 | 6.0.1 | ## [v6.0.1](https://github.com/vitejs/vite/blob/HEAD/packages/vite/CHANGELOG.md#small601-2024-11-27-small) - fix: default empty server `proxy` prevents starting http2 server ([#18788](vitejs/vite#18788)) ([bbaf514](vitejs/vite@bbaf514)), closes [#18788](vitejs/vite#18788) - fix(manifest): do not override existing js manifest entry ([#18776](vitejs/vite#18776)) ([3b0837e](vitejs/vite@3b0837e)), closes [#18776](vitejs/vite#18776) - fix(server): close \_ssrCompatModuleRunner on server close ([#18784](vitejs/vite#18784)) ([9b4c410](vitejs/vite@9b4c410)), closes [#18784](vitejs/vite#18784) - fix(server): skip hot channel client normalization for wsServer ([#18782](vitejs/vite#18782)) ([cc7670a](vitejs/vite@cc7670a)), closes [#18782](vitejs/vite#18782) - fix(worker): fix `applyToEnvironment` hooks on worker build ([#18793](vitejs/vite#18793)) ([0c6cdb0](vitejs/vite@0c6cdb0)), closes [#18793](vitejs/vite#18793) - chore: flat v6 config file ([#18777](vitejs/vite#18777)) ([c7b3308](vitejs/vite@c7b3308)), closes [#18777](vitejs/vite#18777) - chore: split changelog ([#18787](vitejs/vite#18787)) ([8542632](vitejs/vite@8542632)), closes [#18787](vitejs/vite#18787) - chore: update changelog for v6 ([#18773](vitejs/vite#18773)) ([b254fac](vitejs/vite@b254fac)), closes [#18773](vitejs/vite#18773) - revert: update moduleResolution value casing ([#18409](vitejs/vite#18409)) ([#18774](vitejs/vite#18774)) ([b0fc6e3](vitejs/vite@b0fc6e3)), closes [#18409](vitejs/vite#18409) [#18774](vitejs/vite#18774) ## [v6.0.0](https://github.com/vitejs/vite/blob/HEAD/packages/vite/CHANGELOG.md#600-2024-11-26) - fix: `createRunnableDevEnvironment` returns `RunnableDevEnvironment`, not `DevEnvironment` ([#18673](vitejs/vite#18673)) ([74221c3](vitejs/vite@74221c3)), closes [#18673](vitejs/vite#18673) - fix: `getModulesByFile` should return a `serverModule` ([#18715](vitejs/vite#18715)) ([b80d5ec](vitejs/vite@b80d5ec)), closes [#18715](vitejs/vite#18715) - fix: catch error in full reload handler ([#18713](vitejs/vite#18713)) ([a10e741](vitejs/vite@a10e741)), closes [#18713](vitejs/vite#18713) - fix: display pre-transform error details ([#18764](vitejs/vite#18764)) ([554f45f](vitejs/vite@554f45f)), closes [#18764](vitejs/vite#18764) - fix: exit code on `SIGTERM` ([#18741](vitejs/vite#18741)) ([cc55e36](vitejs/vite@cc55e36)), closes [#18741](vitejs/vite#18741) - fix: expose missing `InterceptorOptions` type ([#18766](vitejs/vite#18766)) ([6252c60](vitejs/vite@6252c60)), closes [#18766](vitejs/vite#18766) - fix: log error when send in module runner failed ([#18753](vitejs/vite#18753)) ([ba821bb](vitejs/vite@ba821bb)), closes [#18753](vitejs/vite#18753) - fix(client): overlay not appearing when multiple vite clients were loaded ([#18647](vitejs/vite#18647)) ([27d70b5](vitejs/vite@27d70b5)), closes [#18647](vitejs/vite#18647) - fix(deps): update all non-major dependencies ([#18691](vitejs/vite#18691)) ([f005461](vitejs/vite@f005461)), closes [#18691](vitejs/vite#18691) - fix(html): fix inline proxy modules invalidation ([#18696](vitejs/vite#18696)) ([8ab04b7](vitejs/vite@8ab04b7)), closes [#18696](vitejs/vite#18696) - fix(module-runner): make evaluator optional ([#18672](vitejs/vite#18672)) ([fd1283f](vitejs/vite@fd1283f)), closes [#18672](vitejs/vite#18672) - fix(optimizer): detect npm / yarn / pnpm dependency changes correctly ([#17336](vitejs/vite#17336)) ([#18560](vitejs/vite#18560)) ([818cf3e](vitejs/vite@818cf3e)), closes [#17336](vitejs/vite#17336) [#18560](vitejs/vite#18560) - fix(optimizer): trigger onCrawlEnd after manual included deps are registered ([#18733](vitejs/vite#18733)) ([dc60410](vitejs/vite@dc60410)), closes [#18733](vitejs/vite#18733) - fix(optimizer): workaround firefox's false warning for no sources source map ([#18665](vitejs/vite#18665)) ([473424e](vitejs/vite@473424e)), closes [#18665](vitejs/vite#18665) - fix(ssr): replace `__vite_ssr_identity__` with `(0, ...)` and inject `;` between statements ([#18748](vitejs/vite#18748)) ([94546be](vitejs/vite@94546be)), closes [#18748](vitejs/vite#18748) - chore: add 5.4.x changelogs ([#18768](vitejs/vite#18768)) ([26b58c8](vitejs/vite@26b58c8)), closes [#18768](vitejs/vite#18768) - chore: add some comments about mimes ([#18705](vitejs/vite#18705)) ([f07e9b9](vitejs/vite@f07e9b9)), closes [#18705](vitejs/vite#18705) - chore(deps): update all non-major dependencies ([#18746](vitejs/vite#18746)) ([0ad16e9](vitejs/vite@0ad16e9)), closes [#18746](vitejs/vite#18746) - perf: reduce bundle size for `Object.keys(import.meta.glob(...))` / \`Object.values(import.meta.glob( ([ed99a2c](vitejs/vite@ed99a2c)), closes [#18666](vitejs/vite#18666) - perf(worker): inline worker without base64 ([#18752](vitejs/vite#18752)) ([90c66c9](vitejs/vite@90c66c9)), closes [#18752](vitejs/vite#18752) - feat: add support for .cur type ([#18680](vitejs/vite#18680)) ([5ec9eed](vitejs/vite@5ec9eed)), closes [#18680](vitejs/vite#18680) - feat: enable HMR by default on ModuleRunner side ([#18749](vitejs/vite#18749)) ([4d2abc7](vitejs/vite@4d2abc7)), closes [#18749](vitejs/vite#18749) - feat: support `module-sync` condition when loading config if enabled ([#18650](vitejs/vite#18650)) ([cf5028d](vitejs/vite@cf5028d)), closes [#18650](vitejs/vite#18650) - feat!: drop node 21 support in version ranges ([#18729](vitejs/vite#18729)) ([a384d8f](vitejs/vite@a384d8f)), closes [#18729](vitejs/vite#18729) - fix(deps)!: update dependency dotenv-expand to v12 ([#18697](vitejs/vite#18697)) ([0c658de](vitejs/vite@0c658de)), closes [#18697](vitejs/vite#18697) - docs: rename `HotUpdateContext` to `HotUpdateOptions` ([#18718](vitejs/vite#18718)) ([824c347](vitejs/vite@824c347)), closes [#18718](vitejs/vite#18718) - test: simplify `playground/json/__tests__/ssr` ([#18701](vitejs/vite#18701)) ([f731ca2](vitejs/vite@f731ca2)), closes [#18701](vitejs/vite#18701) - refactor: first character judgment replacement regexp ([#18658](vitejs/vite#18658)) ([58f1df3](vitejs/vite@58f1df3)), closes [#18658](vitejs/vite#18658)

TheAlexLichter · 2024-12-29T15:40:18Z

Seems this breaks using @headlessui/vue's transition component together with ssr/vite-node. (Issue in Nuxt: nuxt/ui#2971)

Apparently, the minified

x(() => {
                if (y.value === T.Hidden && c) {
                    if (v.value && i.value !== "visible") {
                        i.value = "visible";
                        return
                    }
                    O(i.value, {
                        ["hidden"]: () => H(c),
                        ["visible"]: () => D(c)
                    })
                }
            });

is transformed to

(0, __vite_ssr_import_0__.watchEffect)(() => {
                    if (y.value === __vite_ssr_import_6__.RenderStrategy.Hidden && c) {
                        if (v.value && i.value !== "visible") {
                            i.value = "visible";
                            return
                        }(0, ; __vite_ssr_import_5__.match)(i.value, {
                            ["hidden"]: () => H(c),
                            ["visible"]: () => D(c)
                        })
                    }
                });

where (0, ; __vite_ssr_import_5__.match) is a syntax error.

I'll try to come up with a useful repro/test (tracking in #19096)!

…`;` between statements (vitejs#18748) Co-authored-by: sapphi-red <49056869+sapphi-red@users.noreply.github.com>

hi-ogawa added 4 commits November 25, 2024 12:19

wip: ensure ";" between statements

5fd29ac

wip: use (0, ...)

b17e3a1

test: test transform

3e8bf82

chore: cleanup

7d611d3

hi-ogawa added the trigger: preview label Nov 25, 2024

This comment was marked as outdated.

Sign in to view

refactor: walk single pass

ef59c9e

hi-ogawa added 2 commits November 25, 2024 13:55

fix: tweak "}" skip heuristics

3589011

fix: check StaticBlock

b965337

hi-ogawa marked this pull request as ready for review November 25, 2024 05:24

chore: add bench

83b303f

hi-ogawa added 2 commits November 25, 2024 18:39

Merge branch 'main' into fix-ssr-transform-call-wrapper

f0a5ff5

wip

eafe491

fix: add ; in switch

31a436b

sapphi-red added the feat: ssr label Nov 26, 2024

sapphi-red previously approved these changes Nov 26, 2024

View reviewed changes

chore: remove bench

54a6bc1

hi-ogawa dismissed sapphi-red’s stale review via 54a6bc1 November 26, 2024 05:33

sapphi-red approved these changes Nov 26, 2024

View reviewed changes

patak-cat merged commit 94546be into vitejs:main Nov 26, 2024

hi-ogawa deleted the fix-ssr-transform-call-wrapper branch November 26, 2024 08:52

hi-ogawa mentioned this pull request Nov 27, 2024

fix(typecheck): fix typecheck collect on Vite 6 vitest-dev/vitest#6972

Merged

6 tasks

hi-ogawa mentioned this pull request Dec 7, 2024

[Bug]: semantic of function's this changes when calling function from other chunks from in cjs output rolldown/rolldown#3073

Closed

TheAlexLichter mentioned this pull request Dec 29, 2024

ssrTransform can cause syntax error #19096

Closed

7 tasks

moonlitusun pushed a commit to moonlitusun/vite that referenced this pull request May 25, 2025

fix(ssr): replace __vite_ssr_identity__ with (0, ...) and inject …

9f17467

…`;` between statements (vitejs#18748) Co-authored-by: sapphi-red <49056869+sapphi-red@users.noreply.github.com>

Uh oh!

Conversation

hi-ogawa commented Nov 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

pkg-pr-new bot commented Nov 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

hi-ogawa commented Nov 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hi-ogawa commented Nov 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vite-ecosystem-ci bot commented Nov 25, 2024

Uh oh!

sapphi-red commented Nov 25, 2024

Uh oh!

hi-ogawa commented Nov 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sapphi-red commented Nov 25, 2024

Uh oh!

hi-ogawa commented Nov 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hi-ogawa commented Nov 25, 2024

Uh oh!

patak-cat commented Nov 25, 2024

Uh oh!

sapphi-red commented Nov 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hi-ogawa commented Nov 26, 2024

Uh oh!

TheAlexLichter commented Dec 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hi-ogawa commented Nov 25, 2024 •

edited

Loading

pkg-pr-new bot commented Nov 25, 2024 •

edited

Loading

hi-ogawa commented Nov 25, 2024 •

edited

Loading

hi-ogawa commented Nov 25, 2024 •

edited

Loading

hi-ogawa commented Nov 25, 2024 •

edited

Loading

hi-ogawa commented Nov 25, 2024 •

edited

Loading

sapphi-red commented Nov 26, 2024 •

edited

Loading

TheAlexLichter commented Dec 29, 2024 •

edited

Loading