Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
36 changes: 23 additions & 13 deletions packages/runtime-core/src/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -983,6 +983,28 @@ export function registerRuntimeCompiler(_compile: any): void {
// dev only
export const isRuntimeOnly = (): boolean => !compile

/**
* @internal
*/
export function getResolvedCompilerOptions(
instance: ComponentInternalInstance,
): CompilerOptions {
const Component = instance.type as ComponentOptions
const { isCustomElement, compilerOptions } = instance.appContext.config
const { delimiters, compilerOptions: componentCompilerOptions } = Component

return extend(
extend(
{
isCustomElement,
delimiters,
},
compilerOptions,
),
componentCompilerOptions,
)
}

export function finishComponentSetup(
instance: ComponentInternalInstance,
isSSR: boolean,
Expand Down Expand Up @@ -1014,19 +1036,7 @@ export function finishComponentSetup(
if (__DEV__) {
startMeasure(instance, `compile`)
}
const { isCustomElement, compilerOptions } = instance.appContext.config
const { delimiters, compilerOptions: componentCompilerOptions } =
Component
const finalCompilerOptions: CompilerOptions = extend(
extend(
{
isCustomElement,
delimiters,
},
compilerOptions,
),
componentCompilerOptions,
)
const finalCompilerOptions = getResolvedCompilerOptions(instance)
if (__COMPAT__) {
// pass runtime compat config into the compiler
finalCompilerOptions.compatConfig = Object.create(globalCompatConfig)
Expand Down
3 changes: 3 additions & 0 deletions packages/runtime-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,7 @@ export { transformVNodeArgs } from './vnode'
import {
createComponentInstance,
getComponentPublicInstance,
getResolvedCompilerOptions,
setupComponent,
} from './component'
import { renderComponentRoot } from './componentRenderUtils'
Expand All @@ -414,6 +415,7 @@ const _ssrUtils: {
ensureValidVNode: typeof ensureValidVNode
pushWarningContext: typeof pushWarningContext
popWarningContext: typeof popWarningContext
getResolvedCompilerOptions: typeof getResolvedCompilerOptions
} = {
createComponentInstance,
setupComponent,
Expand All @@ -425,6 +427,7 @@ const _ssrUtils: {
ensureValidVNode,
pushWarningContext,
popWarningContext,
getResolvedCompilerOptions,
}

/**
Expand Down
23 changes: 23 additions & 0 deletions packages/server-renderer/__tests__/webStream.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,26 @@ test('pipeToWebWritable', async () => {

expect(res).toBe(`<!--[--><div>parent</div><div>async</div><!--]-->`)
})

test('pipeToWebWritable error handling', async () => {
const App = {
ssrRender() {
throw new Error('ssr render error')
},
}

let abortedReason: any
const writable = new WritableStream({
abort(reason) {
abortedReason = reason
},
})

pipeToWebWritable(createApp(App), {}, writable)

// Wait for the error to propagate
await new Promise(resolve => setTimeout(resolve, 10))

expect(abortedReason).toBeInstanceOf(Error)
expect(abortedReason.message).toBe('ssr render error')
Comment on lines +82 to +88
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Flaky wait — prefer event-driven synchronization over a fixed 10 ms timeout.

setTimeout(resolve, 10) is a race; under CI load (slow machines, GC, microtask backlog) the abort may not have propagated yet, producing false negatives (abortedReason still undefined). Use a promise resolved from inside abort():

🧪 Proposed fix
   let abortedReason: any
+  let resolveAbort!: () => void
+  const aborted = new Promise<void>(r => (resolveAbort = r))
   const writable = new WritableStream({
     abort(reason) {
       abortedReason = reason
+      resolveAbort()
     },
   })

   pipeToWebWritable(createApp(App), {}, writable)

-  // Wait for the error to propagate
-  await new Promise(resolve => setTimeout(resolve, 10))
+  await aborted

   expect(abortedReason).toBeInstanceOf(Error)
   expect(abortedReason.message).toBe('ssr render error')
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pipeToWebWritable(createApp(App), {}, writable)
// Wait for the error to propagate
await new Promise(resolve => setTimeout(resolve, 10))
expect(abortedReason).toBeInstanceOf(Error)
expect(abortedReason.message).toBe('ssr render error')
let abortedReason: any
let resolveAbort!: () => void
const aborted = new Promise<void>(r => (resolveAbort = r))
const writable = new WritableStream({
abort(reason) {
abortedReason = reason
resolveAbort()
},
})
pipeToWebWritable(createApp(App), {}, writable)
await aborted
expect(abortedReason).toBeInstanceOf(Error)
expect(abortedReason.message).toBe('ssr render error')
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/server-renderer/__tests__/webStream.spec.ts` around lines 82 - 88,
The test uses a fixed 10ms setTimeout to wait for the abort which is flaky;
replace the timeout with an event-driven promise that resolves when the
Writable's abort handler runs so the test waits exactly for the abort.
Specifically, in the test that calls pipeToWebWritable(createApp(App), {},
writable) replace the sleep with a Promise whose resolver is called from the
writable.abort (or the assigned abort callback) where you already set
abortedReason; ensure the promise resolves inside that abort handler so the
subsequent expects (abortedReason and its message) run only after abort() has
executed.

})
17 changes: 2 additions & 15 deletions packages/server-renderer/src/helpers/ssrCompile.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import {
type ComponentInternalInstance,
type ComponentOptions,

Check failure on line 3 in packages/server-renderer/src/helpers/ssrCompile.ts

View workflow job for this annotation

GitHub Actions / test / lint-and-test-dts

'ComponentOptions' is declared but its value is never read.
ssrUtils,
warn,
} from 'vue'
import { compile } from '@vue/compiler-ssr'
import { NO, extend, generateCodeFrame, isFunction } from '@vue/shared'

Check failure on line 8 in packages/server-renderer/src/helpers/ssrCompile.ts

View workflow job for this annotation

GitHub Actions / test / lint-and-test-dts

'extend' is declared but its value is never read.
import type { CompilerError, CompilerOptions } from '@vue/compiler-core'

Check failure on line 9 in packages/server-renderer/src/helpers/ssrCompile.ts

View workflow job for this annotation

GitHub Actions / test / lint-and-test-dts

'CompilerOptions' is declared but never used.
import type { PushFn } from '../render'

import * as Vue from 'vue'
Expand All @@ -32,21 +33,7 @@
)
}

// TODO: This is copied from runtime-core/src/component.ts and should probably be refactored
const Component = instance.type as ComponentOptions
const { isCustomElement, compilerOptions } = instance.appContext.config
const { delimiters, compilerOptions: componentCompilerOptions } = Component

const finalCompilerOptions: CompilerOptions = extend(
extend(
{
isCustomElement,
delimiters,
},
compilerOptions,
),
componentCompilerOptions,
)
const finalCompilerOptions = ssrUtils.getResolvedCompilerOptions(instance)

finalCompilerOptions.isCustomElement =
finalCompilerOptions.isCustomElement || NO
Expand Down
37 changes: 24 additions & 13 deletions packages/server-renderer/src/renderToStream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
const { isVNode } = ssrUtils

export interface SimpleReadable {
push(chunk: string | null): void
push(chunk: string | null): void | Promise<void>
destroy(err: any): void
}

Expand All @@ -29,26 +29,37 @@
item = await item
}
if (isString(item)) {
stream.push(item)
const res = stream.push(item)
if (isPromise(res)) await res
} else {
await unrollBuffer(item, stream)
}
}
} else {
// sync buffer can be more efficiently unrolled without unnecessary await
// ticks
unrollBufferSync(buffer, stream)
const res = unrollBufferSync(buffer, stream)
if (isPromise(res)) await res
}
}

function unrollBufferSync(buffer: SSRBuffer, stream: SimpleReadable) {
function unrollBufferSync(
buffer: SSRBuffer,
stream: SimpleReadable,
): void | Promise<void> {
for (let i = 0; i < buffer.length; i++) {
let item = buffer[i]
if (isString(item)) {
stream.push(item)
const res = stream.push(item)
if (isPromise(res)) {
// if the stream is async, we can't unroll it syncly anymore
// this can happen if a sync buffer is being pushed to an async stream
return res.then(() => unrollBufferSync(buffer.slice(i + 1), stream))
}
} else {
// since this is a sync buffer, child buffers are never promises
unrollBufferSync(item as SSRBuffer, stream)
const res = unrollBufferSync(item as SSRBuffer, stream)
if (isPromise(res)) {
return res.then(() => unrollBufferSync(buffer.slice(i + 1), stream))
}
}
}
}
Expand All @@ -73,7 +84,8 @@
// provide the ssr context to the tree
input.provide(ssrContextKey, context)

Promise.resolve(renderComponentVNode(vnode))
Promise.resolve()
.then(() => renderComponentVNode(vnode))
.then(buffer => unrollBuffer(buffer, stream))
.then(() => resolveTeleports(context))
.then(() => {
Expand Down Expand Up @@ -120,7 +132,7 @@
)
}

return renderToSimpleStream(input, context, stream)

Check failure on line 135 in packages/server-renderer/src/renderToStream.ts

View workflow job for this annotation

GitHub Actions / test / lint-and-test-dts

Argument of type 'Readable' is not assignable to parameter of type 'SimpleReadable'.

Check failure on line 135 in packages/server-renderer/src/renderToStream.ts

View workflow job for this annotation

GitHub Actions / test / lint-and-test-dts

Type 'SimpleReadable' is missing the following properties from type 'Readable': readableAborted, readable, readableDidRead, readableEncoding, and 50 more.
}

export function pipeToNodeWritable(
Expand Down Expand Up @@ -205,10 +217,9 @@
}
},
destroy(err) {
// TODO better error handling?
// eslint-disable-next-line no-console
console.log(err)
writer.close()
writer.abort(err).catch(() => {
// ignore errors from aborting an already closed/errored stream
})
},
})
}
Loading