Skip to content

Commit

Permalink
fix: handle case where path starts with root but isn't contained inside
Browse files Browse the repository at this point in the history
Previously `getFsPath` would just replace the first occurrence of the
root path with the empty string (`path.replace(this.root, '')`). This
lead to incorrect behavior:

```js
const path = '/some/path/to/a/package-somewhere/index.js'
const root = '/some/path/to/a/package'
const result = path.replace(root, '')

// result _should_ be '/some/path/to/a/package-somewhere/index.js', but
// instead it's '-somewhere/index.js'
```

This fixes a bug I found while trying to write a test case for #1864.
  • Loading branch information
simon-abbott committed Aug 17, 2022
1 parent cc72d79 commit 1efdd2d
Show file tree
Hide file tree
Showing 11 changed files with 56 additions and 3 deletions.
16 changes: 16 additions & 0 deletions packages/vite-node/src/utils.ts
Expand Up @@ -14,6 +14,22 @@ export function mergeSlashes(str: string) {
return str.replace(/\/\//g, '/')
}

export function snipBase(path: string, base: string) {
if (!base.endsWith('/'))
base += '/'

const isAtFS = path.startsWith('/@fs/')
if (isAtFS && !base.startsWith('/@fs/'))
base = base.startsWith('/') ? `/@fs${base}` : `/@fs/${base}`

if (path.startsWith(base)) {
// Subtract 1 to ensure the result has a leading slash
const snipped = path.slice(base.length - 1)
return isAtFS ? `/@fs${snipped}` : snipped
}
return path
}

export function normalizeRequestId(id: string, base?: string): string {
if (base && id.startsWith(base))
id = `/${id.slice(base.length)}`
Expand Down
6 changes: 3 additions & 3 deletions packages/vitest/src/runtime/mocker.ts
@@ -1,7 +1,7 @@
import { existsSync, readdirSync } from 'fs'
import { isNodeBuiltin } from 'mlly'
import { basename, dirname, join, resolve } from 'pathe'
import { normalizeRequestId, toFilePath } from 'vite-node/utils'
import { normalizeRequestId, snipBase, toFilePath } from 'vite-node/utils'
import type { ModuleCacheMap } from 'vite-node/client'
import { getAllMockableProperties, getType, getWorkerState, isWindows, mergeSlashes, slash } from '../utils'
import { distDir } from '../constants'
Expand Down Expand Up @@ -142,14 +142,14 @@ export class VitestMocker {
}

public normalizePath(path: string) {
return normalizeRequestId(path.replace(this.root, ''), this.base).replace(/^\/@fs\//, isWindows ? '' : '/')
return normalizeRequestId(snipBase(path, this.root), this.base).replace(/^\/@fs\//, isWindows ? '' : '/')
}

public getFsPath(path: string, external: string | null) {
if (external)
return mergeSlashes(`/@fs/${path}`)

return normalizeRequestId(path.replace(this.root, ''))
return normalizeRequestId(snipBase(path, this.root))
}

public resolveMockPath(mockPath: string, external: string | null) {
Expand Down
5 changes: 5 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions test/path-resolution-target/.eslintignore
@@ -0,0 +1 @@
!dist/
1 change: 1 addition & 0 deletions test/path-resolution-target/.gitignore
@@ -0,0 +1 @@
!dist/
2 changes: 2 additions & 0 deletions test/path-resolution-target/README.md
@@ -0,0 +1,2 @@
This package is here just to be a sibling package to `path-resolution`. It
contains no tests of its own.
1 change: 1 addition & 0 deletions test/path-resolution-target/dist/index.d.ts
@@ -0,0 +1 @@
export function add(a: number, b: number): number
3 changes: 3 additions & 0 deletions test/path-resolution-target/dist/index.js
@@ -0,0 +1,3 @@
export function add(a, b) {
return a + b;
}
6 changes: 6 additions & 0 deletions test/path-resolution-target/package.json
@@ -0,0 +1,6 @@
{
"name": "@vitest/test-path-resolution-target",
"type": "module",
"private": true,
"main": "./dist/index.js"
}
1 change: 1 addition & 0 deletions test/path-resolution/package.json
Expand Up @@ -9,6 +9,7 @@
"devDependencies": {
"@edge-runtime/vm": "1.1.0-beta.26",
"@vitest/test-path-res-target": "workspace:*",
"@vitest/test-path-resolution-target": "workspace:*",
"@vitest/web-worker": "workspace:*",
"vitest": "workspace:*"
}
Expand Down
17 changes: 17 additions & 0 deletions test/path-resolution/test/starts-with.test.ts
@@ -0,0 +1,17 @@
import { expect, it, vitest } from 'vitest'

import { add } from '@vitest/test-path-resolution-target'

vitest.mock('@vitest/test-path-resolution-target')

it('should be mocked', () => {
expect(add).toHaveProperty('mock')
expect(add(2, 3)).toBeUndefined()
})

it('should import actual', async () => {
const { add } = await vitest.importActual('@vitest/test-path-resolution-target')

expect(add).not.toHaveProperty('mock')
expect(add(2, 3)).toBe(5)
})

0 comments on commit 1efdd2d

Please sign in to comment.