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.

Sign up for GitHub

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

dev: adapt to vitest@1.2 #213

Merged
merged 6 commits into from
Feb 16, 2024
Merged

dev: adapt to vitest@1.2 #213

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9a0b252
dev: adapt to vitest@1.2.0
keroxp Jan 17, 2024
670d3d5
a
keroxp Jan 17, 2024
067280e
fix import error
keroxp Jan 17, 2024
a93b0f0
revert vitest version to latest
keroxp Jan 17, 2024
9eed0e8
bump vitest to 1.2.2
keroxp Feb 16, 2024
c512b97
treat 'custom' task as 'test'
keroxp Feb 16, 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
19 changes: 9 additions & 10 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,8 @@
},
"scripts": {
"vscode:prepublish": "yarn run compile",
"compile": "tsup ./src/extension.ts --external vscode --minify",
"watch": "tsup ./src/extension.ts --external vscode --watch --sourcemap",
"compile": "tsup ./src/extension.ts --external vscode,lightningcss,esbuild --minify",
"watch": "tsup ./src/extension.ts --external vscode,lightningcss,esbuild --watch --sourcemap",
"test": "vitest run",
"lint": "eslint .",
"lint:fix": "eslint . --fix"
Expand All @@ -172,10 +172,10 @@
"@types/node": "^18.11.18",
"@types/semver": "^7.3.9",
"@types/vscode": "^1.77.0",
"@types/ws": "^8.5.3",
"@typescript-eslint/eslint-plugin": "^5.12.1",
"@typescript-eslint/parser": "^5.12.1",
"@vitest/ws-client": "^0.12.6",
"@types/ws": "^8.5.10",
"@typescript-eslint/eslint-plugin": "^6.19.0",
"@typescript-eslint/parser": "^6.19.0",
"@vitest/ws-client": "^1.2.0",
"@vscode/test-electron": "^2.1.2",
"@vue/reactivity": "^3.2.33",
"@vueuse/core": "^8.4.2",
Expand All @@ -190,11 +190,10 @@
"prettier": "^2.5.1",
"semver": "^7.3.5",
"tree-kill": "^1.2.2",
"tsup": "^5.12.7",
"typescript": "^4.9.5",
"vite": "^2.8.6",
"tsup": "^8.0.1",
"typescript": "^5.3.3",
"vitest": "latest",
"ws": "^8.6.0"
"ws": "^8.16.0"
},
"lint-staged": {
"*.{js,ts,tsx,vue,md}": [
Expand Down
16 changes: 13 additions & 3 deletions src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,22 @@ import * as vscode from 'vscode'
import semver from 'semver'
import type { WorkspaceConfiguration, WorkspaceFolder } from 'vscode'
import type { ResolvedConfig } from 'vitest'
import { configDefaults } from 'vitest/config'
import { isDefinitelyVitestEnv, mayBeVitestEnv } from './pure/isVitestEnv'
import { getVitestCommand, getVitestVersion, isNodeAvailable } from './pure/utils'
import { log } from './log'
export const extensionId = 'zxch3n.vitest-explorer'

// Copied from https://github.com/vitest-dev/vitest/blob/main/packages/vitest/src/defaults.ts
// "import { configDefaults } from 'vitest'" throws unexpected URL error
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This statement throws "Invalid URL" error during activation (from node_modules/vite/dist/node-cjs/publicUtils.cjs. That file includes codes like below:

const { version: version$2 } = JSON.parse(fs$1.readFileSync(new URL('../../package.json', (typeof document === 'undefined' ? require('u' + 'rl').pathToFileURL(__filename).href : (_documentCurrentScript && _documentCurrentScript.src || new URL('node-cjs/publicUtils.cjs', document.baseURI).href)))).toString());

It won't work in the bundled dist/extension.js. I couldn't find any solution and gave up. Are anyone knows something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect Vite to not be bundled with this extension, maybe it can be externalized/tree-shaken?

Copy link
Member

@sheremet-va sheremet-va Feb 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vitest/config doesn't actually import anything from Vite, so I'm not sure why it even gets bundled here 🤔 (Oh, now I see it re-exports mergeConfig). I think we should just update the bundler here to remove the mergeConfig API from vitest/config

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like tsup/esbuild's default tree-shaking is not doing much and keeps unused require("vite"). Actually tsup supports rollup-based tree-shaking https://tsup.egoist.dev/#tree-shaking, so this config seems to work but rebuild slows down a bit.

// tsup.config.ts
import { defineConfig } from "tsup"

export default defineConfig({
  entry: ["./src/extension.ts"],
  external: ["vscode", "vite"],
  treeshake: true,
})

Personally, just inlining defaultInclude and defaultExclude is equally fine as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tsup also supports plugins where we can manually remove vite import

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is ideal to import the original config from vitest. I think it's problem of vitest bundle. new URL with relative path resolution won't work imported bundle. Anyway, can it be acceptable?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's problem of vitest bundle

it is not the problem of Vitest bundle, Vitest doesn't use new URL, this is the code from Vite, and Vite should not be bundled for the extension. To remove vite import, you can write a small esbuild plugin that strips it out

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, let's keep hardcoded config for now

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I wonder if require would work:

require('vitest/config', { paths: [workspaceDir] })

const defaultInclude = ['**/*.{test,spec}.?(c|m)[jt]s?(x)']
const defaultExclude = [
'**/node_modules/**',
'**/dist/**',
'**/cypress/**',
'**/.{idea,git,cache,output,temp}/**',
'**/{karma,rollup,webpack,vite,vitest,jest,ava,babel,nyc,cypress,tsup,build,eslint,prettier}.config.*',
]

export function getConfigValue<T>(
rootConfig: WorkspaceConfiguration,
folderConfig: WorkspaceConfiguration,
Expand Down Expand Up @@ -42,8 +52,8 @@ export function getConfig(workspaceFolder?: WorkspaceFolder | vscode.Uri | strin
export function getCombinedConfig(config: ResolvedConfig, workspaceFolder?: WorkspaceFolder | vscode.Uri | string) {
const vitestConfig = getConfig(workspaceFolder)
return {
exclude: vitestConfig.exclude?.concat(config.exclude) || configDefaults.exclude,
include: vitestConfig.include?.concat(config.include) || configDefaults.include,
exclude: vitestConfig.exclude?.concat(config.exclude) || defaultExclude,
include: vitestConfig.include?.concat(config.include) || defaultInclude,
}
}

Expand Down
1 change: 0 additions & 1 deletion src/discover.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ export class TestFileDiscoverer extends vscode.Disposable {
workspaceFolder.uri,
include,
)
const workspacePath = workspaceFolder.uri.fsPath
const filter = (v: vscode.Uri) =>
exclude.every(x => !minimatch(path.relative(workspacePath, v.fsPath), x, { dot: true }))

Expand Down
2 changes: 1 addition & 1 deletion src/pure/ApiProcess.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export class ApiProcess {

log.info('Start api process at port', port)
log.info('[RUN]', `${this.vitest.cmd} ${this.vitest.args.join(' ')}`)
const cwd = sanitizeFilePath(this.workspace, false)
const cwd = sanitizeFilePath(this.workspace)
log.info('[RUN.cwd]', cwd)

const logs = [] as string[]
Expand Down
2 changes: 1 addition & 1 deletion src/pure/isVitestEnv.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { existsSync } from 'fs'
import path = require('path')
import path from 'node:path'
import { readFile, readdir } from 'fs-extra'
import type { WorkspaceFolder } from 'vscode'
import { getVitestPath } from './utils'
Expand Down
6 changes: 3 additions & 3 deletions src/pure/runner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export class TestRunner {
const command = vitestCommand.cmd
const args = [
...vitestCommand.args,
...(testFile ? testFile.map(f => sanitizeFilePath(f, true)) : []),
...(testFile ? testFile.map(f => sanitizeFilePath(f)) : []),
] as string[]
if (updateSnapshot)
args.push('--update')
Expand All @@ -90,11 +90,11 @@ export class TestRunner {
args.push('-t', testNamePattern.replace(/[$^+?()[\]"]/g, '\\$&'))
}

const workspacePath = sanitizeFilePath(this.workspacePath, false)
const workspacePath = sanitizeFilePath(this.workspacePath)
const outputs: string[] = []
const env = { ...process.env, ...workspaceEnv }
let testResultFiles = [] as File[]
const output = await runVitestWithApi({ cmd: sanitizeFilePath(command, false), args }, this.workspacePath, {
const output = await runVitestWithApi({ cmd: sanitizeFilePath(command), args }, this.workspacePath, {
log: (line) => {
log.info(`${filterColorFormatOutput(line.trimEnd())}\r\n`)
outputs.push(filterColorFormatOutput(line))
Expand Down
8 changes: 3 additions & 5 deletions src/pure/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,13 @@ export function getVitestPath(projectRoot: string): string | undefined {
return

if (existsSync(path.resolve(node_modules, 'vitest', 'vitest.mjs')))
return sanitizeFilePath(path.resolve(node_modules, 'vitest', 'vitest.mjs'), false)
return sanitizeFilePath(path.resolve(node_modules, 'vitest', 'vitest.mjs'))

const suffixes = ['.js', '', '.cmd']
for (const suffix of suffixes) {
if (existsSync(path.resolve(node_modules, '.bin', `vitest${suffix}`))) {
return sanitizeFilePath(
path.resolve(node_modules, '.bin', `vitest${suffix}`),
false,
)
}
}
Expand Down Expand Up @@ -64,7 +63,7 @@ export function getVitestCommand(
return {
cmd: 'node',
args: [
sanitizeFilePath(path.resolve(node_modules, 'vitest', 'vitest.mjs'), false),
sanitizeFilePath(path.resolve(node_modules, 'vitest', 'vitest.mjs')),
],
}
}
Expand Down Expand Up @@ -176,9 +175,8 @@ const capitalizeDriveLetter = (path: string) => {
const replaceDoubleSlashes = (string: string) => string.replace(/\\/g, '/')

export function sanitizeFilePath(path: string) {
if (isWindows) {
if (isWindows)
return replaceDoubleSlashes(capitalizeDriveLetter(path))
}

return path
}
Expand Down
8 changes: 4 additions & 4 deletions src/pure/watch/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import WebSocket from 'ws'
import { computed, effect, reactive, ref, shallowRef } from '@vue/reactivity'
import type { ResolvedConfig, Task, TaskResult, WebSocketEvents } from 'vitest'
import type { ResolvedConfig, Task, TaskResultPack, WebSocketEvents } from 'vitest'
import { log } from '../../log'
import { createClient } from './ws-client'

Expand Down Expand Up @@ -64,14 +64,14 @@ export function buildWatchClient(
// otherwise those record will not be recorded to client.state
const loadingPromise = client.waitForConnection().then(async () => {
const files = await client.rpc.getFiles()
const idResultPairs: [string, TaskResult][] = []
const idResultPairs: TaskResultPack[] = []
let isRunning = files.length === 0
files && travel(files)
function travel(tasks: Task[]) {
for (const task of tasks) {
if (task.type === 'test') {
if (task.type === 'test' || task.type === 'custom') {
if (task.result)
idResultPairs.push([task.id, task.result])
idResultPairs.push([task.id, task.result, task.meta])
else if (task.mode === 'run')
isRunning = true
}
Expand Down
6 changes: 3 additions & 3 deletions src/pure/watch/vitestConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,19 @@ async function connectAndFetchConfig(
if (!handled.has(ws)) {
handled.add(ws)
ws.addEventListener('open', () => {
log.info('WS Opened')
log.info('pure/WS Opened')
client.rpc.getConfig().then((_config) => {
client.dispose()
resolve(_config)
})
})

ws.addEventListener('error', (e) => {
console.error('WS ERROR', e)
console.error('pure/WS ERROR ', e)
})

ws.addEventListener('close', () => {
log.info('WS Close')
log.info('pure/WS Close')
})
}
})
Expand Down
5 changes: 4 additions & 1 deletion src/pure/watch/ws-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export interface VitestClientOptions {
export interface VitestClient {
ws: WebSocket
state: StateManager
rpc: BirpcReturn<WebSocketHandlers>
rpc: BirpcReturn<WebSocketHandlers, WebSocketEvents>
waitForConnection(): Promise<void>
reconnect(): Promise<void>
dispose(): void
Expand Down Expand Up @@ -138,6 +138,9 @@ export function createClient(url: string, options: VitestClientOptions = {}) {
ctx.state.collectFiles(files)
handlers.onCollected?.(files)
},
onCancel(reason) {
handlers.onCancel?.(reason)
},
onTaskUpdate(packs) {
ctx.state.updateTasks(packs)
handlers.onTaskUpdate?.(packs)
Expand Down
19 changes: 5 additions & 14 deletions src/watch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,14 +281,6 @@ export class TestWatcher extends Disposable {
}

function getSourceFilepathAndLocationFromStack(stack: ParsedStack): { sourceFilepath?: string; line: number; column: number } {
if (stack.sourcePos) {
return {
sourceFilepath: stack.sourcePos.source?.replace(/\//g, path.sep),
line: stack.sourcePos.line,
column: stack.sourcePos.column,
}
}

return {
sourceFilepath: stack.file.replace(/\//g, path.sep),
line: stack.line,
Expand Down Expand Up @@ -362,8 +354,8 @@ export function syncTestStatusToVsCode(
) {
const set = new Set(vscode)
for (const task of vitest) {
const data = matchTask(task, set, task.type)
if (task.type === 'test') {
const data = matchTask(task, set)
if (task.type === 'test' || task.type === 'custom') {
if (task.result == null) {
if (finished) {
finishedTest && finishedTest.add(data.item)
Expand All @@ -385,7 +377,7 @@ export function syncTestStatusToVsCode(
case 'fail':
run.failed(
data.item,
testMessageForTestError(data.item, task.result.error),
task.result.errors?.map(i => testMessageForTestError(data.item, i)) ?? [],
task.result.duration,
)
finishedTest && finishedTest.add(data.item)
Expand Down Expand Up @@ -414,14 +406,13 @@ export function syncTestStatusToVsCode(
function matchTask(
task: Task,
candidates: Set<TestDescribe | TestCase>,
type: 'suite' | 'test',
): TestDescribe | TestCase {
let ans: (TestDescribe | TestCase) | undefined
for (const candidate of candidates) {
if (type === 'suite' && !(candidate instanceof TestDescribe))
if (task.type === 'suite' && !(candidate instanceof TestDescribe))
continue

if (type === 'test' && !(candidate instanceof TestCase))
if (task.type === 'test' && !(candidate instanceof TestCase))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about type custom?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what's type='custom'. It should be treated as which one? It seems to be newly introduced in v1.

continue

if (candidate.pattern === task.name) {
Expand Down
1 change: 1 addition & 0 deletions tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"sourceMap": true,
"rootDir": "src",
"esModuleInterop": true,
"skipLibCheck": true,
"strict": true /* enable all strict type-checking options */
/* Additional Checks */
// "noImplicitReturns": true, /* Report error when not all code paths in function return a value. */
Expand Down
Loading