Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ssr): tolerate circular imports #3950

Merged
merged 13 commits into from
Aug 2, 2021
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion packages/playground/ssr-react/__tests__/ssr-react.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { editFile, getColor, isBuild, untilUpdated } from '../../testUtils'
import { editFile, untilUpdated } from '../../testUtils'
import { port } from './serve'
import fetch from 'node-fetch'

Expand Down Expand Up @@ -46,3 +46,10 @@ test('client navigation', async () => {
)
await untilUpdated(() => page.textContent('h1'), 'changed')
})

test(`circular dependecies modules doesn't throw`, async () => {
await page.goto(url)
expect(await page.textContent('.circ-dep-init')).toMatch(
'circ-dep-init-a circ-dep-init-b'
)
})
9 changes: 9 additions & 0 deletions packages/playground/ssr-react/src/add.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { multiply } from './multiply'

export function add(a, b) {
return a + b
}

export function addAndMultiply(a, b, c) {
return multiply(add(a, b), c)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This test aim to find out wherever the modules with circular dependencies are correctly initialized
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from './module-a'
export { getValueAB } from './module-b'
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const valueA = 'circ-dep-init-a'
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { valueA } from './circular-dep-init'

export const valueB = 'circ-dep-init-b'
export const valueAB = valueA.concat(` ${valueB}`)

export function getValueAB() {
return valueAB
}
45 changes: 45 additions & 0 deletions packages/playground/ssr-react/src/forked-deadlock/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
This test aim to check for a particular type of circular dependency that causes tricky deadlocks, **deadlocks with forked imports stack**

```
A -> B means: B is imported by A and B has A in its stack
A ... B means: A is waiting for B to ssrLoadModule()
H -> X ... Y
H -> X -> Y ... B
H -> A ... B
H -> A -> B ... X
```

### Forked deadlock description:
```
[X] is waiting for [Y] to resolve
↑ ↳ is waiting for [A] to resolve
│ ↳ is waiting for [B] to resolve
│ ↳ is waiting for [X] to resolve
└────────────────────────────────────────────────────────────────────────┘
```

This may seems a traditional deadlock, but the thing that makes this special is the import stack of each module:
```
[X] stack:
[H]
```
```
[Y] stack:
[X]
[H]
```
```
[A] stack:
[H]
```
```
[B] stack:
[A]
[H]
```
Even if `[X]` is imported by `[B]`, `[B]` is not in `[X]`'s stack because it's imported by `[H]` in first place then it's stack is only composed by `[H]`. `[H]` **forks** the imports **stack** and this make hard to be found.

### Fix description
Vite, when imports `[X]`, should check whether `[X]` is already pending and if it is, it must check that, when it was imported in first place, the stack of `[X]` doesn't have any module in common with the current module; in this case `[B]` has the module `[H]` is common with `[X]` and i can assume that a deadlock is going to happen.

10 changes: 10 additions & 0 deletions packages/playground/ssr-react/src/forked-deadlock/common-module.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { stuckModuleExport } from './stuck-module'
import { deadlockfuseModuleExport } from './deadlock-fuse-module'

/**
* module H
*/
export function commonModuleExport() {
stuckModuleExport()
deadlockfuseModuleExport()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { fuseStuckBridgeModuleExport } from './fuse-stuck-bridge-module'

/**
* module A
*/
export function deadlockfuseModuleExport() {
fuseStuckBridgeModuleExport()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { stuckModuleExport } from './stuck-module'

/**
* module C
*/
export function fuseStuckBridgeModuleExport() {
stuckModuleExport()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { deadlockfuseModuleExport } from './deadlock-fuse-module'

/**
* module Y
*/
export function middleModuleExport() {
void deadlockfuseModuleExport
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { middleModuleExport } from './middle-module'

/**
* module X
*/
export function stuckModuleExport() {
middleModuleExport()
}
9 changes: 9 additions & 0 deletions packages/playground/ssr-react/src/multiply.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { add } from './add'

export function multiply(a, b) {
return a * b
}

export function multiplyAndAdd(a, b, c) {
return add(multiply(a, b), c)
}
11 changes: 10 additions & 1 deletion packages/playground/ssr-react/src/pages/About.jsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
import { addAndMultiply } from '../add'
import { multiplyAndAdd } from '../multiply'

export default function About() {
return <h1>About</h1>
return (
<>
<h1>About</h1>
<div>{addAndMultiply(1, 2, 3)}</div>
<div>{multiplyAndAdd(1, 2, 3)}</div>
</>
)
}
16 changes: 15 additions & 1 deletion packages/playground/ssr-react/src/pages/Home.jsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,17 @@
import { addAndMultiply } from '../add'
import { multiplyAndAdd } from '../multiply'
import { commonModuleExport } from '../forked-deadlock/common-module'
import { getValueAB } from '../circular-dep-init/circular-dep-init'

export default function Home() {
return <h1>Home</h1>
commonModuleExport()

return (
<>
<h1>Home</h1>
<div>{addAndMultiply(1, 2, 3)}</div>
<div>{multiplyAndAdd(1, 2, 3)}</div>
<div className="circ-dep-init">{getValueAB()}</div>
</>
)
}
72 changes: 40 additions & 32 deletions packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ test('default import', async () => {
)
).code
).toMatchInlineSnapshot(`
"const __vite_ssr_import_0__ = __vite_ssr_import__(\\"vue\\")
"const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\");
console.log(__vite_ssr_import_0__.default.bar)"
`)
})
Expand All @@ -26,7 +26,7 @@ test('named import', async () => {
)
).code
).toMatchInlineSnapshot(`
"const __vite_ssr_import_0__ = __vite_ssr_import__(\\"vue\\")
"const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\");
function foo() { return __vite_ssr_import_0__.ref(0) }"
`)
})
Expand All @@ -41,7 +41,7 @@ test('namespace import', async () => {
)
).code
).toMatchInlineSnapshot(`
"const __vite_ssr_import_0__ = __vite_ssr_import__(\\"vue\\")
"const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\");
function foo() { return __vite_ssr_import_0__.ref(0) }"
`)
})
Expand All @@ -50,24 +50,24 @@ test('export function declaration', async () => {
expect((await ssrTransform(`export function foo() {}`, null, null)).code)
.toMatchInlineSnapshot(`
"function foo() {}
Object.defineProperty(__vite_ssr_exports__, \\"foo\\", { enumerable: true, configurable: true, get(){ return foo }})"
Object.defineProperty(__vite_ssr_exports__, \\"foo\\", { enumerable: true, configurable: true, get(){ return foo }});"
`)
})

test('export class declaration', async () => {
expect((await ssrTransform(`export class foo {}`, null, null)).code)
.toMatchInlineSnapshot(`
"class foo {}
Object.defineProperty(__vite_ssr_exports__, \\"foo\\", { enumerable: true, configurable: true, get(){ return foo }})"
Object.defineProperty(__vite_ssr_exports__, \\"foo\\", { enumerable: true, configurable: true, get(){ return foo }});"
`)
})

test('export var declaration', async () => {
expect((await ssrTransform(`export const a = 1, b = 2`, null, null)).code)
.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 }})"
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 }});"
`)
})

Expand All @@ -77,8 +77,8 @@ test('export named', async () => {
.code
).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 }})"
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 }});"
`)
})

Expand All @@ -87,10 +87,10 @@ test('export named from', async () => {
(await ssrTransform(`export { ref, computed as c } from 'vue'`, null, null))
.code
).toMatchInlineSnapshot(`
"const __vite_ssr_import_0__ = __vite_ssr_import__(\\"vue\\")
"const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\");

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 }})"
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 }});"
`)
})

Expand All @@ -104,27 +104,35 @@ test('named exports of imported binding', async () => {
)
).code
).toMatchInlineSnapshot(`
"const __vite_ssr_import_0__ = __vite_ssr_import__(\\"vue\\")
"const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\");

Object.defineProperty(__vite_ssr_exports__, \\"createApp\\", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__.createApp }})"
Object.defineProperty(__vite_ssr_exports__, \\"createApp\\", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__.createApp }});"
`)
})

test('export * from', async () => {
expect((await ssrTransform(`export * from 'vue'`, null, null)).code)
.toMatchInlineSnapshot(`
"const __vite_ssr_import_0__ = __vite_ssr_import__(\\"vue\\")

__vite_ssr_exportAll__(__vite_ssr_import_0__)"
expect(
(
await ssrTransform(
`export * from 'vue'\n` + `export * from 'react'`,
null,
null
)
).code
).toMatchInlineSnapshot(`
"const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\");
__vite_ssr_exportAll__(__vite_ssr_import_0__);
const __vite_ssr_import_1__ = await __vite_ssr_import__(\\"react\\");
__vite_ssr_exportAll__(__vite_ssr_import_1__);"
`)
})

test('export * as from', async () => {
expect((await ssrTransform(`export * as foo from 'vue'`, null, null)).code)
.toMatchInlineSnapshot(`
"const __vite_ssr_import_0__ = __vite_ssr_import__(\\"vue\\")
"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__ }})"
Object.defineProperty(__vite_ssr_exports__, \\"foo\\", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__ }});"
`)
})

Expand All @@ -146,7 +154,7 @@ test('dynamic import', async () => {
.code
).toMatchInlineSnapshot(`
"const i = () => __vite_ssr_dynamic_import__('./foo')
Object.defineProperty(__vite_ssr_exports__, \\"i\\", { enumerable: true, configurable: true, get(){ return i }})"
Object.defineProperty(__vite_ssr_exports__, \\"i\\", { enumerable: true, configurable: true, get(){ return i }});"
`)
})

Expand All @@ -160,7 +168,7 @@ test('do not rewrite method definition', async () => {
)
).code
).toMatchInlineSnapshot(`
"const __vite_ssr_import_0__ = __vite_ssr_import__(\\"vue\\")
"const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\");
class A { fn() { __vite_ssr_import_0__.fn() } }"
`)
})
Expand All @@ -175,7 +183,7 @@ test('do not rewrite catch clause', async () => {
)
).code
).toMatchInlineSnapshot(`
"const __vite_ssr_import_0__ = __vite_ssr_import__(\\"./dependency\\")
"const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"./dependency\\");
try {} catch(error) {}"
`)
})
Expand All @@ -191,7 +199,7 @@ test('should declare variable for imported super class', async () => {
)
).code
).toMatchInlineSnapshot(`
"const __vite_ssr_import_0__ = __vite_ssr_import__(\\"./dependency\\")
"const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"./dependency\\");
const Foo = __vite_ssr_import_0__.Foo;
class A extends Foo {}"
`)
Expand All @@ -209,12 +217,12 @@ test('should declare variable for imported super class', async () => {
)
).code
).toMatchInlineSnapshot(`
"const __vite_ssr_import_0__ = __vite_ssr_import__(\\"./dependency\\")
"const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"./dependency\\");
const Foo = __vite_ssr_import_0__.Foo;
class A extends Foo {}
class B extends Foo {}
Object.defineProperty(__vite_ssr_exports__, \\"default\\", { enumerable: true, value: A })
Object.defineProperty(__vite_ssr_exports__, \\"B\\", { enumerable: true, configurable: true, get(){ return B }})"
Object.defineProperty(__vite_ssr_exports__, \\"default\\", { enumerable: true, value: A });
Object.defineProperty(__vite_ssr_exports__, \\"B\\", { enumerable: true, configurable: true, get(){ return B }});"
`)
})

Expand Down Expand Up @@ -246,7 +254,7 @@ test('should handle default export variants', async () => {
).toMatchInlineSnapshot(`
"function foo() {}
foo.prototype = Object.prototype;
Object.defineProperty(__vite_ssr_exports__, \\"default\\", { enumerable: true, value: foo })"
Object.defineProperty(__vite_ssr_exports__, \\"default\\", { enumerable: true, value: foo });"
`)
// default named classes
expect(
Expand All @@ -260,8 +268,8 @@ test('should handle default export variants', async () => {
).toMatchInlineSnapshot(`
"class A {}
class B extends A {}
Object.defineProperty(__vite_ssr_exports__, \\"default\\", { enumerable: true, value: A })
Object.defineProperty(__vite_ssr_exports__, \\"B\\", { enumerable: true, configurable: true, get(){ return B }})"
Object.defineProperty(__vite_ssr_exports__, \\"default\\", { enumerable: true, value: A });
Object.defineProperty(__vite_ssr_exports__, \\"B\\", { enumerable: true, configurable: true, get(){ return B }});"
`)
})

Expand All @@ -288,7 +296,7 @@ test('overwrite bindings', async () => {
)
).code
).toMatchInlineSnapshot(`
"const __vite_ssr_import_0__ = __vite_ssr_import__(\\"vue\\")
"const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"vue\\");
const a = { inject: __vite_ssr_import_0__.inject }
const b = { test: __vite_ssr_import_0__.inject }
function c() { const { test: inject } = { test: true }; console.log(inject) }
Expand Down
Loading