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

fix: consider deep imports in isBuiltIn #5248

Merged
merged 1 commit into from Oct 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion packages/playground/ssr-vue/__tests__/ssr-vue.spec.ts
Expand Up @@ -146,4 +146,9 @@ test('client navigation', async () => {
test('import.meta.url', async () => {
await page.goto(url)
expect(await page.textContent('.protocol')).toEqual('file:')
})
})

test('deep import built-in module', async () => {
await page.goto(url)
expect(await page.textContent('.file-message')).toMatch('fs/promises')
})
2 changes: 1 addition & 1 deletion packages/playground/ssr-vue/server.js
Expand Up @@ -66,7 +66,7 @@ async function createServer(
render = require('./dist/server/entry-server.js').render
}

const [appHtml, preloadLinks] = await render(url, manifest)
const [appHtml, preloadLinks] = await render(url, manifest, __dirname)

const html = template
.replace(`<!--preload-links-->`, preloadLinks)
Expand Down
13 changes: 11 additions & 2 deletions packages/playground/ssr-vue/src/entry-server.js
@@ -1,7 +1,8 @@
import { createApp } from './main'
import { renderToString } from 'vue/server-renderer'
import path from 'path'

export async function render(url, manifest) {
export async function render(url, manifest, rootDir) {
const { app, router } = createApp()

// set the router to the desired URL before rendering
Expand All @@ -13,7 +14,15 @@ export async function render(url, manifest) {
// itself on ctx.modules. After the render, ctx.modules would contain all the
// components that have been instantiated during this render call.
const ctx = {}
const html = await renderToString(app, ctx)
let html = await renderToString(app, ctx)

// for testing. Use deep import built-in module. PR #5248
const fs =
process.versions.node.split('.')[0] >= '14'
? await import('fs/promises')
: (await import('fs')).promises
const msg = await fs.readFile(path.resolve(rootDir, './src/message'), 'utf-8')
html += `<p class="file-message">msg read via deep import built-in module: ${msg}</p>`

// the SSR manifest generated by Vite contains module -> chunk/asset mapping
// which we can then use to determine what files need to be preloaded for this
Expand Down
1 change: 1 addition & 0 deletions packages/playground/ssr-vue/src/message
@@ -0,0 +1 @@
"fs/promises"
2 changes: 2 additions & 0 deletions packages/vite/src/node/utils.ts
Expand Up @@ -38,6 +38,8 @@ export const normalizeId = (id: string): string =>
id.replace(/(\s*>\s*)/g, ' > ')

export function isBuiltin(id: string): boolean {
const deepMatch = id.match(deepImportRE)
id = deepMatch ? deepMatch[1] || deepMatch[2] : id
return builtins.includes(id)
}
patak-dev marked this conversation as resolved.
Show resolved Hide resolved

Expand Down