fix(ssr): hoist export to handle cyclic import better

packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts

@@ -50,59 +50,53 @@ test('namespace import', async () => {
 })
 
 test('export function declaration', async () => {
-  expect(await ssrTransformSimpleCode(`export function foo() {}`))
-    .toMatchInlineSnapshot(`
-      "function foo() {}
-      Object.defineProperty(__vite_ssr_exports__, "foo", { enumerable: true, configurable: true, get(){ return foo }});"
-    `)
+  expect(
+    await ssrTransformSimpleCode(`export function foo() {}`),
+  ).toMatchInlineSnapshot(
+    `"Object.defineProperty(__vite_ssr_exports__, "foo", { enumerable: true, configurable: true, get(){ return foo }});function foo() {}"`,
+  )
 })
 
 test('export class declaration', async () => {
-  expect(await ssrTransformSimpleCode(`export class foo {}`))
-    .toMatchInlineSnapshot(`
-      "class foo {}
-      Object.defineProperty(__vite_ssr_exports__, "foo", { enumerable: true, configurable: true, get(){ return foo }});"
-    `)
+  expect(
+    await ssrTransformSimpleCode(`export class foo {}`),
+  ).toMatchInlineSnapshot(
+    `"Object.defineProperty(__vite_ssr_exports__, "foo", { enumerable: true, configurable: true, get(){ return foo }});class foo {}"`,
+  )
 })
 
 test('export var declaration', async () => {
-  expect(await ssrTransformSimpleCode(`export const a = 1, b = 2`))
-    .toMatchInlineSnapshot(`
-      "const a = 1, b = 2
-      Object.defineProperty(__vite_ssr_exports__, "a", { enumerable: true, configurable: true, get(){ return a }});
-      Object.defineProperty(__vite_ssr_exports__, "b", { enumerable: true, configurable: true, get(){ return b }});"
-    `)
+  expect(
+    await ssrTransformSimpleCode(`export const a = 1, b = 2`),
+  ).toMatchInlineSnapshot(
+    `"Object.defineProperty(__vite_ssr_exports__, "a", { enumerable: true, configurable: true, get(){ return a }});Object.defineProperty(__vite_ssr_exports__, "b", { enumerable: true, configurable: true, get(){ return b }});const a = 1, b = 2"`,
+  )
 })
 
 test('export named', async () => {
   expect(
     await ssrTransformSimpleCode(`const a = 1, b = 2; export { a, b as c }`),
-  ).toMatchInlineSnapshot(`
-    "const a = 1, b = 2; 
-    Object.defineProperty(__vite_ssr_exports__, "a", { enumerable: true, configurable: true, get(){ return a }});
-    Object.defineProperty(__vite_ssr_exports__, "c", { enumerable: true, configurable: true, get(){ return b }});"
-  `)
+  ).toMatchInlineSnapshot(
+    `"Object.defineProperty(__vite_ssr_exports__, "a", { enumerable: true, configurable: true, get(){ return a }});Object.defineProperty(__vite_ssr_exports__, "c", { enumerable: true, configurable: true, get(){ return b }});const a = 1, b = 2; "`,
+  )
 })
 
 test('export named from', async () => {
   expect(
     await ssrTransformSimpleCode(`export { ref, computed as c } from 'vue'`),
-  ).toMatchInlineSnapshot(`
-    "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["ref","computed"]});
-    Object.defineProperty(__vite_ssr_exports__, "ref", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__.ref }});
-    Object.defineProperty(__vite_ssr_exports__, "c", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__.computed }});"
-  `)
+  ).toMatchInlineSnapshot(
+    `"Object.defineProperty(__vite_ssr_exports__, "ref", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__.ref }});Object.defineProperty(__vite_ssr_exports__, "c", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__.computed }});const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["ref","computed"]});"`,
+  )
 })
 
 test('named exports of imported binding', async () => {
   expect(
     await ssrTransformSimpleCode(
       `import {createApp} from 'vue';export {createApp}`,
     ),
-  ).toMatchInlineSnapshot(`
-    "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["createApp"]});
-    Object.defineProperty(__vite_ssr_exports__, "createApp", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__.createApp }});"
-  `)
+  ).toMatchInlineSnapshot(
+    `"Object.defineProperty(__vite_ssr_exports__, "createApp", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__.createApp }});const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["createApp"]});"`,
+  )
 })
 
 test('export * from', async () => {
@@ -119,42 +113,39 @@ test('export * from', async () => {
 })
 
 test('export * as from', async () => {
-  expect(await ssrTransformSimpleCode(`export * as foo from 'vue'`))
-    .toMatchInlineSnapshot(`
-      "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue");
-      Object.defineProperty(__vite_ssr_exports__, "foo", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__ }});"
-    `)
+  expect(
+    await ssrTransformSimpleCode(`export * as foo from 'vue'`),
+  ).toMatchInlineSnapshot(
+    `"Object.defineProperty(__vite_ssr_exports__, "foo", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__ }});const __vite_ssr_import_0__ = await __vite_ssr_import__("vue");"`,
+  )
 })
 
 test('export * as from arbitrary module namespace identifier', async () => {
   expect(
     await ssrTransformSimpleCode(`export * as "arbitrary string" from 'vue'`),
-  ).toMatchInlineSnapshot(`
-    "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue");
-    Object.defineProperty(__vite_ssr_exports__, "arbitrary string", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__ }});"
-  `)
+  ).toMatchInlineSnapshot(
+    `"Object.defineProperty(__vite_ssr_exports__, "arbitrary string", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__ }});const __vite_ssr_import_0__ = await __vite_ssr_import__("vue");"`,
+  )
 })
 
 test('export as arbitrary module namespace identifier', async () => {
   expect(
     await ssrTransformSimpleCode(
       `const something = "Something";export { something as "arbitrary string" };`,
     ),
-  ).toMatchInlineSnapshot(`
-    "const something = "Something";
-    Object.defineProperty(__vite_ssr_exports__, "arbitrary string", { enumerable: true, configurable: true, get(){ return something }});"
-  `)
+  ).toMatchInlineSnapshot(
+    `"Object.defineProperty(__vite_ssr_exports__, "arbitrary string", { enumerable: true, configurable: true, get(){ return something }});const something = "Something";"`,
+  )
 })
 
 test('export as from arbitrary module namespace identifier', async () => {
   expect(
     await ssrTransformSimpleCode(
       `export { "arbitrary string2" as "arbitrary string" } from 'vue';`,
     ),
-  ).toMatchInlineSnapshot(`
-    "const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["arbitrary string2"]});
-    Object.defineProperty(__vite_ssr_exports__, "arbitrary string", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__["arbitrary string2"] }});"
-  `)
+  ).toMatchInlineSnapshot(
+    `"Object.defineProperty(__vite_ssr_exports__, "arbitrary string", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__["arbitrary string2"] }});const __vite_ssr_import_0__ = await __vite_ssr_import__("vue", {"importedNames":["arbitrary string2"]});"`,
+  )
 })
 
 test('export default', async () => {
@@ -206,7 +197,7 @@ test('preserve line offset when rewriting imports', async () => {
     `  dirname,`,
     `  join,`,
     `} from 'node:path';`,
-    ``,
+    `export { join };`,
     `debugger;`,
     ``,
     `import fs from 'node:fs';`,
@@ -217,7 +208,7 @@ test('preserve line offset when rewriting imports', async () => {
     `  red,`,
     `  green,`,
     `} from 'kleur/colors';`,
-    ``,
+    `export { red };`,
     `debugger;`,
   ]
 
@@ -230,7 +221,7 @@ test('preserve line offset when rewriting imports', async () => {
       .map((line, i) => `${String(i + 1).padStart(2)} | ${line}`.trimEnd())
       .join('\n'),
   ).toMatchInlineSnapshot(`
-    " 1 | const __vite_ssr_import_0__ = await __vite_ssr_import__("node:path", {"importedNames":["dirname","join"]});const __vite_ssr_import_1__ = await __vite_ssr_import__("node:fs", {"importedNames":["default"]});const __vite_ssr_import_2__ = await __vite_ssr_import__("kleur/colors", {"importedNames":["red","green"]});debugger;
+    " 1 | Object.defineProperty(__vite_ssr_exports__, "join", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__.join }});Object.defineProperty(__vite_ssr_exports__, "red", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_2__.red }});const __vite_ssr_import_0__ = await __vite_ssr_import__("node:path", {"importedNames":["dirname","join"]});const __vite_ssr_import_1__ = await __vite_ssr_import__("node:fs", {"importedNames":["default"]});const __vite_ssr_import_2__ = await __vite_ssr_import__("kleur/colors", {"importedNames":["red","green"]});debugger;
      2 |
      3 |
      4 |
@@ -280,10 +271,9 @@ test('dynamic import', async () => {
   const result = await ssrTransformSimple(
     `export const i = () => import('./foo')`,
   )
-  expect(result?.code).toMatchInlineSnapshot(`
-    "const i = () => __vite_ssr_dynamic_import__('./foo')
-    Object.defineProperty(__vite_ssr_exports__, "i", { enumerable: true, configurable: true, get(){ return i }});"
-  `)
+  expect(result?.code).toMatchInlineSnapshot(
+    `"Object.defineProperty(__vite_ssr_exports__, "i", { enumerable: true, configurable: true, get(){ return i }});const i = () => __vite_ssr_dynamic_import__('./foo')"`,
+  )
   expect(result?.deps).toEqual([])
   expect(result?.dynamicDeps).toEqual(['./foo'])
 })
@@ -440,10 +430,9 @@ test('should declare variable for imported super class', async () => {
         `export class B extends Foo {}`,
     ),
   ).toMatchInlineSnapshot(`
-    "const __vite_ssr_import_0__ = await __vite_ssr_import__("./dependency", {"importedNames":["Foo"]});const Foo = __vite_ssr_import_0__.Foo;
+    "Object.defineProperty(__vite_ssr_exports__, "B", { enumerable: true, configurable: true, get(){ return B }});const __vite_ssr_import_0__ = await __vite_ssr_import__("./dependency", {"importedNames":["Foo"]});const Foo = __vite_ssr_import_0__.Foo;
     class A extends Foo {};
     class B extends Foo {}
-    Object.defineProperty(__vite_ssr_exports__, "B", { enumerable: true, configurable: true, get(){ return B }});
     Object.defineProperty(__vite_ssr_exports__, "default", { enumerable: true, configurable: true, value: A });"
   `)
 })
@@ -479,9 +468,8 @@ test('should handle default export variants', async () => {
       `export default class A {}\n` + `export class B extends A {}`,
     ),
   ).toMatchInlineSnapshot(`
-    "class A {};
+    "Object.defineProperty(__vite_ssr_exports__, "B", { enumerable: true, configurable: true, get(){ return B }});class A {};
     class B extends A {}
-    Object.defineProperty(__vite_ssr_exports__, "B", { enumerable: true, configurable: true, get(){ return B }});
     Object.defineProperty(__vite_ssr_exports__, "default", { enumerable: true, configurable: true, value: A });"
   `)
 })
@@ -919,12 +907,10 @@ export function fn1() {
         `,
     ),
   ).toMatchInlineSnapshot(`
-    "
+    "Object.defineProperty(__vite_ssr_exports__, "fn1", { enumerable: true, configurable: true, get(){ return fn1 }});Object.defineProperty(__vite_ssr_exports__, "fn2", { enumerable: true, configurable: true, get(){ return fn2 }});
     function fn1() {
+    };function fn2() {
     }
-    Object.defineProperty(__vite_ssr_exports__, "fn1", { enumerable: true, configurable: true, get(){ return fn1 }});;function fn2() {
-    }
-    Object.defineProperty(__vite_ssr_exports__, "fn2", { enumerable: true, configurable: true, get(){ return fn2 }});
             "
   `)
 })
@@ -1023,7 +1009,7 @@ export class Test {
 };`.trim()
 
   expect(await ssrTransformSimpleCode(code)).toMatchInlineSnapshot(`
-    "const __vite_ssr_import_0__ = await __vite_ssr_import__("foobar", {"importedNames":["foo","bar"]});
+    "Object.defineProperty(__vite_ssr_exports__, "Test", { enumerable: true, configurable: true, get(){ return Test }});const __vite_ssr_import_0__ = await __vite_ssr_import__("foobar", {"importedNames":["foo","bar"]});
     if (false) {
       const foo = 'foo';
       console.log(foo)
@@ -1047,8 +1033,7 @@ export class Test {
           console.log((0,__vite_ssr_import_0__.bar))
         }
       }
-    }
-    Object.defineProperty(__vite_ssr_exports__, "Test", { enumerable: true, configurable: true, get(){ return Test }});;;"
+    };;"
   `)
 })
 
@@ -1213,11 +1198,10 @@ export * as bar from './bar'
 console.log(bar)
   `),
   ).toMatchInlineSnapshot(`
-    "
+    "Object.defineProperty(__vite_ssr_exports__, "bar", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_1__ }});
     const __vite_ssr_import_0__ = await __vite_ssr_import__("./foo", {"importedNames":["foo"]});
     __vite_ssr_exports__.default = (0,__vite_ssr_import_0__.foo)();
-    const __vite_ssr_import_1__ = await __vite_ssr_import__("./bar");
-    Object.defineProperty(__vite_ssr_exports__, "bar", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_1__ }});;
+    const __vite_ssr_import_1__ = await __vite_ssr_import__("./bar");;
     console.log(bar)
       "
   `)

packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/README.md

@@ -0,0 +1,22 @@
+- `test1` - `test5`
+
+Cyclic import example based on https://github.com/vitejs/vite/issues/14048#issuecomment-2354774156
+
+```mermaid
+flowchart TD
+    B(dep1.js) -->|dep1| A(index.js)
+    A -->|dep1| C(dep2.js)
+    C -->|dep2| A
+    A -->|dep1, dep2| entry.js
+```
+
+---
+
+- `test6`
+
+```mermaid
+flowchart TD
+    A(dep1.js) -->|dep1| B
+    B(dep2.js) -->|dep2| A
+    A -->|dep1| C(index.js)
+```

packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/package.json

@@ -0,0 +1,3 @@
+{
+  "type": "module"
+}

packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/test1/dep1.js

@@ -0,0 +1 @@
+export const dep1 = { ok: true };

packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/test1/dep2.js

@@ -0,0 +1,2 @@
+import { dep1 } from "./index.js"
+export const dep2 = { ok: dep1.ok }

packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/test1/index.js

@@ -0,0 +1,5 @@
+import { dep1 } from './dep1.js'
+export { dep1 }
+
+import { dep2 } from './dep2.js'
+export { dep2 }

packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/test2/dep1.js

@@ -0,0 +1 @@
+export const dep1 = { ok: true };

packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/test2/dep2.js

@@ -0,0 +1,2 @@
+import { dep1 } from "./index.js"
+export const dep2 = { ok: dep1.ok }

packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/test2/index.js

@@ -0,0 +1,2 @@
+export { dep1 } from './dep1.js'
+export { dep2 } from "./dep2.js"

packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/test3/dep1.js

@@ -0,0 +1 @@
+export const dep1 = { ok: true };

packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/test3/dep2.js

@@ -0,0 +1,2 @@
+import { dep1 } from "./index.js"
+export const dep2 = { ok: dep1.ok }

packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/test3/index.js

@@ -0,0 +1,4 @@
+import { dep1 } from './dep1.js'
+import { dep2 } from './dep2.js'
+export { dep1 }
+export { dep2 }

packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/test4/dep1.js

@@ -0,0 +1 @@
+export const dep1 = { ok: true };

packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/test4/dep2.js

@@ -0,0 +1,2 @@
+import { dep1 } from "./index.js"
+export const dep2 = { ok: dep1.ok }

packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/test4/index.js

@@ -0,0 +1,2 @@
+export * from './dep1.js'
+export * from './dep2.js'

packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/test5/dep1.js

@@ -0,0 +1 @@
+export const dep1 = { ok: true };

packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/test5/dep2.js

@@ -0,0 +1,2 @@
+import { dep1 } from "./index.js"
+export const dep2 = { ok: dep1.ok }

packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/test5/index.js

@@ -0,0 +1,5 @@
+import { dep2 } from './dep2.js'
+export { dep2 }
+
+import { dep1 } from './dep1.js'
+export { dep1 }

packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/test6/dep1.js

@@ -0,0 +1,2 @@
+import { dep2 } from "./dep2.js"
+export const dep1 = "dep1: " + dep2

packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/test6/dep2.js

@@ -0,0 +1,2 @@
+import { dep1 } from "./dep1.js"
+export const dep2 = "dep2: " + dep1

packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/test6/index.js

@@ -0,0 +1,2 @@
+import { dep1 } from './dep1.js'
+export { dep1 }

packages/vite/src/node/ssr/runtime/__tests__/server-runtime.spec.ts

@@ -246,6 +246,42 @@ describe('module runner initialization', async () => {
     const mod = await runner.import('/fixtures/no-this/importer.js')
     expect(mod.result).toBe(undefined)
   })
+
+  it.for([
+    '/fixtures/cyclic2/test1/index.js',
+    '/fixtures/cyclic2/test2/index.js',
+    '/fixtures/cyclic2/test3/index.js',
+    '/fixtures/cyclic2/test4/index.js',
+  ] as const)(`cyclic %s`, async (entry, { runner }) => {
+    const mod = await runner.import(entry)
+    expect({ ...mod }).toEqual({
+      dep1: {
+        ok: true,
+      },
+      dep2: {
+        ok: true,
+      },
+    })
+  })
+
+  it(`cyclic invalid 1`, async ({ runner }) => {
+    // Node also fails but with a different message
+    //   $ node packages/vite/src/node/ssr/runtime/__tests__/fixtures/cyclic2/test5/index.js
+    //   ReferenceError: Cannot access 'dep1' before initialization
+    await expect(() =>
+      runner.import('/fixtures/cyclic2/test5/index.js'),
+    ).rejects.toMatchInlineSnapshot(
+      `[ReferenceError: Cannot access '__vite_ssr_import_1__' before initialization]`,
+    )
+  })
+
+  it(`cyclic invalid 2`, async ({ runner }) => {
+    await expect(() =>
+      runner.import('/fixtures/cyclic2/test6/index.js'),
+    ).rejects.toMatchInlineSnapshot(
+      `[ReferenceError: Cannot access 'dep1' before initialization]`,
+    )
+  })
 })
 
 describe('optimize-deps', async () => {