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: Module graph in workspace of UI package #3636

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from 11 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
2 changes: 1 addition & 1 deletion packages/ui/client/components/FileDetails.vue
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ debouncedWatch(
current,
async (c, o) => {
if (c && c.filepath !== o?.filepath) {
data.value = await client.rpc.getModuleGraph(c.filepath)
data.value = await client.rpc.getModuleGraph(c.filepath, c.projectName)
graph.value = getModuleGraph(data.value, c.filepath)
}
},
Expand Down
7 changes: 6 additions & 1 deletion packages/ui/client/components/TaskItem.vue
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
<script setup lang="ts">
import { getProjectNameColor } from '../utils/task'
import { isDark } from '../composables/dark'
import type { Task } from '#types'

const props = defineProps<{
Expand All @@ -23,7 +25,10 @@ const duration = computed(() => {
>
<StatusIcon :task="task" mr-2 />
<div flex items-end gap-2 :text="task?.result?.state === 'fail' ? 'red-500' : ''">
<span text-sm truncate font-light>{{ task.name }}</span>
<span text-sm truncate font-light>
<span v-if="task.type === 'suite' && task.projectName" :style="{ color: getProjectNameColor(task.projectName, isDark) }">|{{ task.projectName }}| </span>
{{ task.name }}
</span>
<span v-if="typeof duration === 'number'" text="xs" op20 style="white-space: nowrap">
{{ duration > 0 ? duration : '< 1' }}ms
</span>
Expand Down
21 changes: 21 additions & 0 deletions packages/ui/client/utils/task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,24 @@ export function caseInsensitiveMatch(target: string, str2: string) {
return false
return target.toLowerCase().includes(str2.toLowerCase())
}

export function getProjectNameColor(name: string | undefined, isDark: boolean) {
if (!name)
return ''
const index = name.split('').reduce((acc, v, idx) => acc + v.charCodeAt(0) + idx, 0)
const darkColors = [
'#0000FF', // blue
'#FFFF00', // yellow
'#00FFFF', // cyan
'#00FF00', // green
'#FF00FF', // magenta
]
const lightColors = [
'#330099', // blue
'#7c7c05', // yellow
'#087b7b', // cyan
'#025402', // green
'#FF00FF', // magenta
]
return isDark ? darkColors[index % darkColors.length] : lightColors[index % lightColors.length]
}
4 changes: 2 additions & 2 deletions packages/vitest/src/api/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ export function setup(vitestOrWorkspace: Vitest | WorkspaceProject, server?: Vit
return result
}
},
async getModuleGraph(id: string): Promise<ModuleGraphData> {
return getModuleGraph(ctx, id)
async getModuleGraph(id: string, workspaceName?: string): Promise<ModuleGraphData> {
return getModuleGraph(ctx, id, workspaceName)
},
updateSnapshot(file?: File) {
if (!file)
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/api/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export interface WebSocketHandlers {
getConfig(): ResolvedConfig
resolveSnapshotPath(testPath: string): string
resolveSnapshotRawPath(testPath: string, rawPath: string): string
getModuleGraph(id: string): Promise<ModuleGraphData>
getModuleGraph(id: string, workspaceName?: string): Promise<ModuleGraphData>
getTransformResult(id: string): Promise<TransformResultWithSource | undefined>
readFile(id: string): Promise<string | null>
writeFile(id: string, content: string, ensureDir?: boolean): Promise<void>
Expand Down
15 changes: 13 additions & 2 deletions packages/vitest/src/utils/graph.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { ModuleNode } from 'vite'
import type { ModuleGraphData, Vitest } from '../types'

export async function getModuleGraph(ctx: Vitest, id: string): Promise<ModuleGraphData> {
export async function getModuleGraph(ctx: Vitest, id: string, workspaceName?: string): Promise<ModuleGraphData> {
const graph: Record<string, string[]> = {}
const externalized = new Set<string>()
const inlined = new Set<string>()
Expand Down Expand Up @@ -29,7 +29,18 @@ export async function getModuleGraph(ctx: Vitest, id: string): Promise<ModuleGra
graph[id] = (await Promise.all(mods.map(m => get(m, seen)))).filter(Boolean) as string[]
return id
}
await get(ctx.server.moduleGraph.getModuleById(id))

const workspaceProject = ctx.projects.find(project => project.getName() === workspaceName)
const workspaceModules = workspaceProject?.ctx.getModuleProjects(id)
Copy link
Member

@sheremet-va sheremet-va Aug 16, 2023

Choose a reason for hiding this comment

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

This code can be removed, you already have access to ctx

const selectedModule = workspaceModules?.find(module => module.getName() === workspaceName)
Copy link
Member

Choose a reason for hiding this comment

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

You do the same thing you did on #L33. Why?

You need to find the project by name and fallback to ctx.getCoreWorkspaceProject().server || ctx.server

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sheremet-va getCoreWorkspaceProject() is coming as null for test/workspaces folder.
Screenshot 2023-08-08 213513

Upon investigating, I have found that the conditons that executes the createCoreWorkspace function in packages\vitest\src\node\core.ts aren't satisfying

Screenshot 2023-08-08 213414

You do the same thing you did on #L33. Why?

This might be hacky, but I have found only this way where we get the workspaceMod

Copy link
Member

@sheremet-va sheremet-va Aug 16, 2023

Choose a reason for hiding this comment

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

getCoreWorkspaceProject() is coming as null for test/workspaces folder.

Yes, it will be null if there is no core project. This is why I added || ctx.server. Core project is not created if main config file is not used by a workspace (so, when there is a global config and project configs and workspace specifies only project configs without manually using core project)

Core workspace project should be used AS THE LAST RESORT. Regular projects should be used if they are found by name.

This might be hacky, but I have found only this way where we get the workspaceMod

Then you must be doing something wrong?

Copy link
Member

Choose a reason for hiding this comment

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

Workspace project is just a project with the same name. If there is no such projects, just fallback to context which is always defined

let workspaceMod: ModuleNode | undefined
if (selectedModule) {
const { server, browser } = selectedModule
const mod = server.moduleGraph.getModuleById(id) || browser?.moduleGraph.getModuleById(id)
workspaceMod = mod
}

await get(ctx.server.moduleGraph.getModuleById(id) || workspaceMod)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't you fallback to ctx.server.moduleGraph only if you didn't find the workspaceMod? This line looks suspicious to me

return {
graph,
externalized: Array.from(externalized),
Expand Down