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: correctly resolve paths relative to root, when used outside of root directory #2687

Merged
merged 6 commits into from
Jan 21, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions packages/vite-node/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,12 +207,19 @@ export class ViteNodeRunner {
}

private async _resolveUrl(id: string, importee?: string): Promise<[url: string, fsPath: string]> {
if (!this.shouldResolveId(id))
return [id, id]
// we don't pass down importee here, because otherwise Vite doesn't resolve it correctly
// should be checked before normalization, because it removes this prefix
if (importee && id.startsWith(VALID_ID_PREFIX))
importee = undefined
id = normalizeRequestId(id, this.options.base)
// should be checked after normalization
// provide importer only for relative and absolute paths
// paths like "src/user" are valid for transformRequest, but they will be resolved incorrectly,
// if importer is provided - because they will be treated as relative to the importer instead of root
if (!id.startsWith('/') && !id.startsWith('./') && !id.startsWith('../'))
importee = undefined
if (!this.shouldResolveId(id))
return [id, id]
const { path, exists } = toFilePath(id, this.root)
if (!this.options.resolveId || exists)
return [id, path]
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/runtime/execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export class VitestRunner extends ViteNodeRunner {
const environment = getCurrentEnvironment()
// do not try and resolve node builtins in Node
// import('url') returns Node internal even if 'url' package is installed
return environment === 'node' ? !isNodeBuiltin(id) : true
return environment === 'node' ? !isNodeBuiltin(id) : !id.startsWith('node:')
}

async resolveUrl(id: string, importee?: string) {
Expand Down
4 changes: 2 additions & 2 deletions packages/vitest/src/runtime/loader.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { pathToFileURL } from 'node:url'
import { fileURLToPath, pathToFileURL } from 'node:url'
import { readFile } from 'node:fs/promises'
import { hasCJSSyntax, isNodeBuiltin } from 'mlly'
import { normalizeModuleId } from 'vite-node/utils'
Expand Down Expand Up @@ -64,7 +64,7 @@ export const resolve: Resolver = async (url, context, next) => {
}
else {
const { url: resolvedUrl, format } = await next(url, context, next)
filepath = new URL(resolvedUrl).pathname
filepath = fileURLToPath(resolvedUrl)
result = {
url: resolvedUrl,
format,
Expand Down
7 changes: 7 additions & 0 deletions test/core/test/imports.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ test('dynamic absolute with extension mport works', async () => {
expect(stringTimeoutMod).toBe(variableTimeoutMod)
})

test('dynamic baseUrl import works', async () => {
const staticMod = await import('./../src/timeout')
// @ts-expect-error there is no tsconfig in test/core to handle baseUrl
const dynamicMod = await import('src/timeout')
expect(staticMod).toBe(dynamicMod)
})

test('data with dynamic import works', async () => {
const dataUri = 'data:text/javascript;charset=utf-8,export default "hi"'
const { default: hi } = await import(dataUri)
Expand Down