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: replace runner-side path normalization with fetchModule-side resolve #18361

Merged
merged 42 commits into from
Dec 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
0f1ca5b
fix(runner): normalize absolute path in fetchModule
hi-ogawa Oct 16, 2024
1913865
test: add test
hi-ogawa Oct 16, 2024
de0a2f8
wip: windows
hi-ogawa Oct 16, 2024
5e8e7ec
wip: windows
hi-ogawa Oct 16, 2024
2c3170c
refactor: move normalization to fetchModule
hi-ogawa Oct 16, 2024
3726ec3
chore: remove root
hi-ogawa Oct 16, 2024
f001b7d
chore: cleanup
hi-ogawa Oct 16, 2024
8f8bc80
Merge branch 'main' into fix-normalize-url-in-fetchModule
hi-ogawa Oct 21, 2024
4bf3d0a
test: resolve file url via plugin
hi-ogawa Oct 21, 2024
f35cacb
fix: no support `file://`
hi-ogawa Oct 21, 2024
d9e5dbb
Merge branch 'main' into fix-normalize-url-in-fetchModule
hi-ogawa Oct 21, 2024
b1236bf
debug
hi-ogawa Oct 21, 2024
d4059cd
chore: comment
hi-ogawa Oct 21, 2024
0a50356
test: refactor out fs mock
hi-ogawa Oct 21, 2024
8e510e0
test: revert isolate
hi-ogawa Oct 21, 2024
75ab26e
Merge branch 'main' into fix-normalize-url-in-fetchModule
hi-ogawa Oct 22, 2024
42a5eb4
chore: comment
hi-ogawa Oct 22, 2024
975f597
Merge branch 'main' into fix-normalize-url-in-fetchModule
hi-ogawa Oct 29, 2024
27060f8
cherry-pick https://github.com/vitejs/vite/pull/18421
hi-ogawa Oct 22, 2024
768c935
test: cleanup
hi-ogawa Oct 29, 2024
d5ed954
chore: cleanup
hi-ogawa Oct 29, 2024
08cd7d9
test: tweak
hi-ogawa Oct 29, 2024
ad91d63
chore: remove root
hi-ogawa Oct 29, 2024
0855eb7
ci: debug preview-release
hi-ogawa Oct 29, 2024
5350765
ci: more debug
hi-ogawa Oct 29, 2024
2793b78
ci: tweak
hi-ogawa Oct 29, 2024
2c3c408
Merge branch 'main' into fix-normalize-url-in-fetchModule
hi-ogawa Oct 29, 2024
b3e1e4f
Merge branch 'main' into fix-normalize-url-in-fetchModule
hi-ogawa Oct 30, 2024
446fe6f
fix: deal with isExternalUrl
hi-ogawa Oct 30, 2024
5a44730
test: revive resolveId test
hi-ogawa Oct 30, 2024
cfc4901
chore: cleanup
hi-ogawa Oct 30, 2024
61679ab
chore: remove more
hi-ogawa Oct 30, 2024
a6d6e77
chore: more cleanup
hi-ogawa Oct 30, 2024
0f39deb
Merge branch 'main' into fix-normalize-url-in-fetchModule
hi-ogawa Dec 10, 2024
06d820e
chore: revert some
hi-ogawa Dec 10, 2024
4e04d5c
chore: cleanup
hi-ogawa Dec 10, 2024
92f53eb
docs: update
hi-ogawa Dec 10, 2024
bdfbe10
chore: optional root
hi-ogawa Dec 10, 2024
901170c
docs: remove root
hi-ogawa Dec 10, 2024
feb14b0
chore: remove internal usages of root
hi-ogawa Dec 10, 2024
ff8b33c
chore: remove root in worker examples
hi-ogawa Dec 10, 2024
b1a1722
chore: keep this one though
hi-ogawa Dec 10, 2024
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: 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 @@ -165,6 +165,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 @@ -250,3 +251,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
Loading