Skip to content

Commit

Permalink
fix: replace runner-side path normalization with fetchModule-side r…
Browse files Browse the repository at this point in the history
…esolve (#18361)
  • Loading branch information
hi-ogawa authored Dec 24, 2024
1 parent 8bfe247 commit 9f10261
Show file tree
Hide file tree
Showing 11 changed files with 67 additions and 68 deletions.
9 changes: 1 addition & 8 deletions docs/guide/api-environment-runtimes.md
Original file line number Diff line number Diff line change
Expand Up @@ -150,11 +150,10 @@ Module runner exposes `import` method. When Vite server triggers `full-reload` H

```js
import { ModuleRunner, ESModulesEvaluator } from 'vite/module-runner'
import { root, transport } from './rpc-implementation.js'
import { transport } from './rpc-implementation.js'

const moduleRunner = new ModuleRunner(
{
root,
transport,
},
new ESModulesEvaluator(),
Expand All @@ -180,10 +179,6 @@ type ModuleRunnerTransport = unknown

// ---cut---
interface ModuleRunnerOptions {
/**
* Root of the project
*/
root: string
/**
* A set of methods to communicate with the server.
*/
Expand Down Expand Up @@ -294,7 +289,6 @@ const transport = {

const runner = new ModuleRunner(
{
root: fileURLToPath(new URL('./', import.meta.url)),
transport,
},
new ESModulesEvaluator(),
Expand Down Expand Up @@ -362,7 +356,6 @@ import { ESModulesEvaluator, ModuleRunner } from 'vite/module-runner'

export const runner = new ModuleRunner(
{
root: fileURLToPath(new URL('./', import.meta.url)),
transport: {
async invoke(data) {
const response = await fetch(`http://my-vite-server/invoke`, {
Expand Down
6 changes: 0 additions & 6 deletions packages/vite/src/module-runner/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import type {
SSRImportMetadata,
} from './types'
import {
normalizeAbsoluteUrl,
posixDirname,
posixPathToFileHref,
posixResolve,
Expand Down Expand Up @@ -52,7 +51,6 @@ export class ModuleRunner {
})
private readonly transport: NormalizedModuleRunnerTransport
private readonly resetSourceMapSupport?: () => void
private readonly root: string
private readonly concurrentModuleNodePromises = new Map<
string,
Promise<EvaluatedModuleNode>
Expand All @@ -65,8 +63,6 @@ export class ModuleRunner {
public evaluator: ModuleEvaluator = new ESModulesEvaluator(),
private debug?: ModuleRunnerDebugger,
) {
const root = this.options.root
this.root = root[root.length - 1] === '/' ? root : `${root}/`
this.evaluatedModules = options.evaluatedModules ?? new EvaluatedModules()
this.transport = normalizeModuleRunnerTransport(options.transport)
if (options.hmr !== false) {
Expand Down Expand Up @@ -237,8 +233,6 @@ export class ModuleRunner {
url: string,
importer?: string,
): Promise<EvaluatedModuleNode> {
url = normalizeAbsoluteUrl(url, this.root)

let cached = this.concurrentModuleNodePromises.get(url)
if (!cached) {
const cachedModule = this.evaluatedModules.getModuleByUrl(url)
Expand Down
3 changes: 2 additions & 1 deletion packages/vite/src/module-runner/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,9 @@ export interface ModuleRunnerHmr {
export interface ModuleRunnerOptions {
/**
* Root of the project
* @deprecated not used and to be removed
*/
root: string
root?: string
/**
* A set of methods to communicate with the server.
*/
Expand Down
23 changes: 1 addition & 22 deletions packages/vite/src/module-runner/utils.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,5 @@
import * as pathe from 'pathe'
import { isWindows, slash } from '../shared/utils'

export function normalizeAbsoluteUrl(url: string, root: string): string {
url = slash(url)

// file:///C:/root/id.js -> C:/root/id.js
if (url.startsWith('file://')) {
// 8 is the length of "file:///"
url = decodeURI(url.slice(isWindows ? 8 : 7))
}

// strip root from the URL because fetchModule prefers a public served url path
// packages/vite/src/node/server/moduleGraph.ts:17
if (url.startsWith(root)) {
// /root/id.js -> /id.js
// C:/root/id.js -> /id.js
// 1 is to keep the leading slash
url = url.slice(root.length - 1)
}

return url
}
import { isWindows } from '../shared/utils'

export const decodeBase64 =
typeof atob !== 'undefined'
Expand Down
24 changes: 8 additions & 16 deletions packages/vite/src/node/ssr/fetchModule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,16 @@ export async function fetchModule(
return { externalize: url, type: 'builtin' }
}

if (isExternalUrl(url)) {
// handle file urls from not statically analyzable dynamic import
const isFileUrl = url.startsWith('file://')

if (isExternalUrl(url) && !isFileUrl) {
return { externalize: url, type: 'network' }
}

// if there is no importer, the file is an entry point
// entry points are always internalized
if (importer && url[0] !== '.' && url[0] !== '/') {
if (!isFileUrl && importer && url[0] !== '.' && url[0] !== '/') {
const { isProduction, root } = environment.config
const { externalConditions, dedupe, preserveSymlinks } =
environment.config.resolve
Expand Down Expand Up @@ -74,7 +77,7 @@ export async function fetchModule(

// this is an entry point module, very high chance it's not resolved yet
// for example: runner.import('./some-file') or runner.import('/some-file')
if (!importer) {
if (isFileUrl || !importer) {
const resolved = await environment.pluginContainer.resolveId(url)
if (!resolved) {
throw new Error(`[vite] cannot find entry point module '${url}'.`)
Expand All @@ -84,8 +87,8 @@ export async function fetchModule(

url = unwrapId(url)

let mod = await environment.moduleGraph.getModuleByUrl(url)
const cached = !!mod?.transformResult
const mod = await environment.moduleGraph.ensureEntryFromUrl(url)
const cached = !!mod.transformResult

// if url is already cached, we can just confirm it's also cached on the server
if (options.cached && cached) {
Expand All @@ -102,17 +105,6 @@ export async function fetchModule(
)
}

// module entry should be created by transformRequest
mod ??= await environment.moduleGraph.getModuleByUrl(url)

if (!mod) {
throw new Error(
`[vite] cannot find module '${url}' ${
importer ? ` imported from '${importer}'` : ''
}.`,
)
}

if (options.inlineSourceMap !== false) {
result = inlineSourceMap(mod, result, options.startOffset)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
import path from 'node:path'
import * as staticModule from './simple'
import { pathToFileURL } from 'node:url'

export const initialize = async () => {
const nameRelative = './simple'
const nameAbsolute = '/fixtures/simple'
const nameAbsoluteExtension = '/fixtures/simple.js'
const absolutePath = path.join(import.meta.dirname, "simple.js")
const fileUrl = pathToFileURL(absolutePath)
return {
dynamicProcessed: await import('./simple'),
dynamicRelative: await import(nameRelative),
dynamicAbsolute: await import(nameAbsolute),
dynamicAbsoluteExtension: await import(nameAbsoluteExtension),
dynamicAbsoluteFull: await import(path.join(import.meta.dirname, "simple.js")),
dynamicAbsoluteFull: await import((process.platform === 'win32' ? '/@fs/' : '') + absolutePath),
dynamicFileUrl: await import(fileUrl),
static: staticModule,
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// @ts-check

import { BroadcastChannel, parentPort } from 'node:worker_threads'
import { fileURLToPath } from 'node:url'
import { ESModulesEvaluator, ModuleRunner } from 'vite/module-runner'
import { createBirpc } from 'birpc'

Expand All @@ -20,7 +19,6 @@ const rpc = createBirpc({}, {

const runner = new ModuleRunner(
{
root: fileURLToPath(new URL('./', import.meta.url)),
transport: {
invoke(data) { return rpc.invoke(data) }
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// @ts-check

import { BroadcastChannel, parentPort } from 'node:worker_threads'
import { fileURLToPath } from 'node:url'
import { ESModulesEvaluator, ModuleRunner } from 'vite/module-runner'

if (!parentPort) {
Expand All @@ -24,7 +23,6 @@ const messagePortTransport = {

const runner = new ModuleRunner(
{
root: fileURLToPath(new URL('./', import.meta.url)),
transport: messagePortTransport,
},
new ESModulesEvaluator(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ describe('module runner initialization', async () => {
expect(modules.static).toBe(modules.dynamicAbsolute)
expect(modules.static).toBe(modules.dynamicAbsoluteExtension)
expect(modules.static).toBe(modules.dynamicAbsoluteFull)
expect(modules.static).toBe(modules.dynamicFileUrl)
})

it('correctly imports a virtual module', async ({ runner }) => {
Expand Down Expand Up @@ -263,3 +264,41 @@ describe('optimize-deps', async () => {
expect(mod.default.hello()).toMatchInlineSnapshot(`"world"`)
})
})

describe('resolveId absolute path entry', async () => {
const it = await createModuleRunnerTester({
plugins: [
{
name: 'test-resolevId',
enforce: 'pre',
resolveId(source) {
if (
source ===
posix.join(this.environment.config.root, 'fixtures/basic.js')
) {
return '\0virtual:basic'
}
},
load(id) {
if (id === '\0virtual:basic') {
return `export const name = "virtual:basic"`
}
},
},
],
})

it('ssrLoadModule', async ({ server }) => {
const mod = await server.ssrLoadModule(
posix.join(server.config.root, 'fixtures/basic.js'),
)
expect(mod.name).toMatchInlineSnapshot(`"virtual:basic"`)
})

it('runner', async ({ server, runner }) => {
const mod = await runner.import(
posix.join(server.config.root, 'fixtures/basic.js'),
)
expect(mod.name).toMatchInlineSnapshot(`"virtual:basic"`)
})
})
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, expect } from 'vitest'
import type { ModuleRunner } from 'vite/module-runner'
import type { ViteDevServer } from '../../..'
import { createModuleRunnerTester, editFile, resolvePath } from './utils'

describe('module runner initialization', async () => {
Expand All @@ -18,13 +18,13 @@ describe('module runner initialization', async () => {
return err
}
}
const serializeStack = (runner: ModuleRunner, err: Error) => {
return err.stack!.split('\n')[1].replace(runner.options.root, '<root>')
const serializeStack = (server: ViteDevServer, err: Error) => {
return err.stack!.split('\n')[1].replace(server.config.root, '<root>')
}
const serializeStackDeep = (runtime: ModuleRunner, err: Error) => {
const serializeStackDeep = (server: ViteDevServer, err: Error) => {
return err
.stack!.split('\n')
.map((s) => s.replace(runtime.options.root, '<root>'))
.map((s) => s.replace(server.config.root, '<root>'))
}

it('source maps are correctly applied to stack traces', async ({
Expand All @@ -35,15 +35,15 @@ describe('module runner initialization', async () => {
const topLevelError = await getError(() =>
runner.import('/fixtures/has-error.js'),
)
expect(serializeStack(runner, topLevelError)).toBe(
expect(serializeStack(server, topLevelError)).toBe(
' at <root>/fixtures/has-error.js:2:7',
)

const methodError = await getError(async () => {
const mod = await runner.import('/fixtures/throws-error-method.ts')
mod.throwError()
})
expect(serializeStack(runner, methodError)).toBe(
expect(serializeStack(server, methodError)).toBe(
' at Module.throwError (<root>/fixtures/throws-error-method.ts:6:9)',
)

Expand All @@ -60,17 +60,17 @@ describe('module runner initialization', async () => {
mod.throwError()
})

expect(serializeStack(runner, methodErrorNew)).toBe(
expect(serializeStack(server, methodErrorNew)).toBe(
' at Module.throwError (<root>/fixtures/throws-error-method.ts:11:9)',
)
})

it('deep stacktrace', async ({ runner }) => {
it('deep stacktrace', async ({ runner, server }) => {
const methodError = await getError(async () => {
const mod = await runner.import('/fixtures/has-error-deep.ts')
mod.main()
})
expect(serializeStackDeep(runner, methodError).slice(0, 3)).toEqual([
expect(serializeStackDeep(server, methodError).slice(0, 3)).toEqual([
'Error: crash',
' at crash (<root>/fixtures/has-error-deep.ts:2:9)',
' at Module.main (<root>/fixtures/has-error-deep.ts:6:3)',
Expand Down
1 change: 1 addition & 0 deletions packages/vite/src/node/ssr/runtime/__tests__/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export async function createModuleRunnerTester(
}
},
},
...(config.plugins ?? []),
],
...config,
})
Expand Down

0 comments on commit 9f10261

Please sign in to comment.