From 08855dd9905568431b2f6ecbd8442f7ce87e848c Mon Sep 17 00:00:00 2001 From: Ryan Tang <24728770+ryandiginomad@users.noreply.github.com> Date: Sun, 31 May 2026 23:18:30 +0800 Subject: [PATCH 1/2] feat(theme-check-vscode): surface orphaned files on startup On activation, check the theme for orphaned (dead) files and show a single dismissable notification with "Review" (opens the existing dead-code picker) and "Don't show again" actions, instead of requiring the user to run the dead-code command manually. - Gated by a new `themeCheck.checkOrphanedFilesOnBoot` setting (default on), mirroring the existing `themeCheck.preloadOnBoot` pattern - Reuses the whole-theme dead-code detection already exposed via ThemeGraphDeadCodeRequest - Extract fetchDeadCode + showDeadCodePicker from makeDeadCode so the startup check and the command share logic (no behavior change; covered by tests) - Wired into both node and browser activation, fire-and-forget so it never blocks or fails activation Closes #970 --- .changeset/orphaned-files-startup-prompt.md | 7 ++ packages/vscode-extension/package.json | 7 ++ .../vscode-extension/src/browser/extension.ts | 4 + .../src/common/commands.spec.ts | 94 ++++++++++++++ .../vscode-extension/src/common/commands.ts | 56 ++++++--- .../src/common/orphanedFilesOnBoot.spec.ts | 116 ++++++++++++++++++ .../src/common/orphanedFilesOnBoot.ts | 48 ++++++++ .../vscode-extension/src/node/extension.ts | 4 + 8 files changed, 319 insertions(+), 17 deletions(-) create mode 100644 .changeset/orphaned-files-startup-prompt.md create mode 100644 packages/vscode-extension/src/common/commands.spec.ts create mode 100644 packages/vscode-extension/src/common/orphanedFilesOnBoot.spec.ts create mode 100644 packages/vscode-extension/src/common/orphanedFilesOnBoot.ts diff --git a/.changeset/orphaned-files-startup-prompt.md b/.changeset/orphaned-files-startup-prompt.md new file mode 100644 index 000000000..40483bad2 --- /dev/null +++ b/.changeset/orphaned-files-startup-prompt.md @@ -0,0 +1,7 @@ +--- +'theme-check-vscode': minor +--- + +Surface orphaned (dead) files on startup with a notification + +When a theme has orphaned files, the VS Code extension now shows a single dismissable notification on startup with **Review** (opens the existing dead-code picker) and **Don't show again** actions, instead of requiring the user to run the dead-code command manually. Gated by the new `themeCheck.checkOrphanedFilesOnBoot` setting (default `true`). diff --git a/packages/vscode-extension/package.json b/packages/vscode-extension/package.json index 6a230bce9..adffc2dd7 100644 --- a/packages/vscode-extension/package.json +++ b/packages/vscode-extension/package.json @@ -165,6 +165,13 @@ ], "description": "When true, theme check preloads all the files from your theme for fast rename handling and theme graph generation.", "default": true + }, + "themeCheck.checkOrphanedFilesOnBoot": { + "type": [ + "boolean" + ], + "description": "When true, show a notification on startup when your theme has orphaned (dead) files.", + "default": true } } }, diff --git a/packages/vscode-extension/src/browser/extension.ts b/packages/vscode-extension/src/browser/extension.ts index 074c1447b..31a29214c 100644 --- a/packages/vscode-extension/src/browser/extension.ts +++ b/packages/vscode-extension/src/browser/extension.ts @@ -10,6 +10,7 @@ import LiquidFormatter from '../common/formatter'; import { vscodePrettierFormat } from './formatter'; import { documentSelectors } from '../common/constants'; import { makeDeadCode, openLocation } from '../common/commands'; +import { checkOrphanedFilesOnBoot } from '../common/orphanedFilesOnBoot'; import { createReferencesTreeView, setupContext, @@ -45,6 +46,9 @@ export async function activate(context: ExtensionContext) { createReferencesTreeView('shopify.themeGraph.dependencies', context, client, 'dependencies'), watchReferencesTreeViewConfig(), ); + + // Fire-and-forget: surfacing orphaned files must never block or fail activation. + checkOrphanedFilesOnBoot(client).catch((error) => console.error(error)); } } diff --git a/packages/vscode-extension/src/common/commands.spec.ts b/packages/vscode-extension/src/common/commands.spec.ts new file mode 100644 index 000000000..596ef8e83 --- /dev/null +++ b/packages/vscode-extension/src/common/commands.spec.ts @@ -0,0 +1,94 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { BaseLanguageClient } from 'vscode-languageclient'; + +const mocks = vi.hoisted(() => ({ + showInformationMessage: vi.fn(), + showQuickPick: vi.fn(), + showTextDocument: vi.fn(), + openTextDocument: vi.fn(() => Promise.resolve({})), + executeCommand: vi.fn(), + activeTextEditor: { + document: { uri: { toString: () => 'file:///theme/layout/theme.liquid' } }, + } as any, +})); + +vi.mock('vscode', () => ({ + window: { + get activeTextEditor() { + return mocks.activeTextEditor; + }, + showInformationMessage: mocks.showInformationMessage, + showQuickPick: mocks.showQuickPick, + showTextDocument: mocks.showTextDocument, + }, + workspace: { openTextDocument: mocks.openTextDocument }, + commands: { executeCommand: mocks.executeCommand }, + Uri: { parse: (s: string) => ({ toString: () => s }) }, + Position: class { + constructor( + public line: number, + public character: number, + ) {} + }, + Range: class { + constructor( + public start: any, + public end: any, + ) {} + }, +})); + +import { makeDeadCode } from './commands'; + +function makeClient(rootUri: string, deadCode: string[]): BaseLanguageClient { + return { + sendRequest: vi + .fn() + .mockResolvedValueOnce(rootUri) // ThemeGraphRootRequest + .mockResolvedValueOnce(deadCode), // ThemeGraphDeadCodeRequest + } as unknown as BaseLanguageClient; +} + +describe('makeDeadCode (characterization — behavior must survive refactor)', () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.activeTextEditor = { + document: { uri: { toString: () => 'file:///theme/layout/theme.liquid' } }, + }; + }); + + it('tells the user when there is no dead code', async () => { + const client = makeClient('file:///theme', []); + + await makeDeadCode(client)(); + + expect(mocks.showInformationMessage).toHaveBeenCalledWith('No dead code found.'); + expect(mocks.showQuickPick).not.toHaveBeenCalled(); + }); + + it('offers a quick pick of the orphaned files when dead code is found', async () => { + const client = makeClient('file:///theme', [ + 'file:///theme/snippets/unused-a.liquid', + 'file:///theme/snippets/unused-b.liquid', + ]); + + await makeDeadCode(client)(); + + expect(mocks.showInformationMessage).not.toHaveBeenCalled(); + expect(mocks.showQuickPick).toHaveBeenCalledTimes(1); + const [items, options] = mocks.showQuickPick.mock.calls[0]; + expect(items).toHaveLength(2); + expect(options).toMatchObject({ canPickMany: true }); + }); + + it('does nothing when there is no active editor', async () => { + mocks.activeTextEditor = undefined; + const client = makeClient('file:///theme', ['file:///theme/snippets/unused.liquid']); + + await makeDeadCode(client)(); + + expect(client.sendRequest).not.toHaveBeenCalled(); + expect(mocks.showInformationMessage).not.toHaveBeenCalled(); + expect(mocks.showQuickPick).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/vscode-extension/src/common/commands.ts b/packages/vscode-extension/src/common/commands.ts index 12c5f59e7..5318bd761 100644 --- a/packages/vscode-extension/src/common/commands.ts +++ b/packages/vscode-extension/src/common/commands.ts @@ -25,31 +25,53 @@ export function openLocation(ref: AugmentedLocation) { }); } +/** + * Fetches the theme root and the list of dead (orphaned) files for a given uri. + * The uri only needs to belong to the theme — the server resolves the root and + * computes dead code across the whole theme graph. + */ +export async function fetchDeadCode( + client: BaseLanguageClient, + uri: string, +): Promise<{ rootUri: string; deadCode: string[] }> { + const [rootUri, deadCode] = await Promise.all([ + client.sendRequest(ThemeGraphRootRequest.type, { uri }), + client.sendRequest(ThemeGraphDeadCodeRequest.type, { uri }), + ]); + return { rootUri, deadCode }; +} + +/** + * Presents the dead files as a multi-select quick pick and opens whatever the + * user selects. Does not depend on the active editor, so it can be driven from + * a startup check as well as the command. + */ +export async function showDeadCodePicker(rootUri: string, deadCode: string[]): Promise { + const relativePaths = deadCode.map((file) => path.relative(file, rootUri)); + const selectedFiles = await window.showQuickPick(relativePaths, { + canPickMany: true, + placeHolder: 'Select files to open', + }); + if (selectedFiles) { + selectedFiles.forEach((file) => { + const uri = path.join(rootUri, file); + workspace.openTextDocument(Uri.parse(uri)).then((doc) => { + window.showTextDocument(doc, { preview: false, preserveFocus: true, viewColumn: 2 }); + }); + }); + } +} + export function makeDeadCode(client: BaseLanguageClient) { return async function deadCode() { const uri = window.activeTextEditor?.document.uri.toString(); if (!uri) return; - const [rootUri, deadCode] = await Promise.all([ - client.sendRequest(ThemeGraphRootRequest.type, { uri }), - client.sendRequest(ThemeGraphDeadCodeRequest.type, { uri }), - ]); + const { rootUri, deadCode } = await fetchDeadCode(client, uri); if (deadCode.length === 0) { window.showInformationMessage('No dead code found.'); } else { - const relativePaths = deadCode.map((file) => path.relative(file, rootUri)); - const selectedFiles = await window.showQuickPick(relativePaths, { - canPickMany: true, - placeHolder: 'Select files to open', - }); - if (selectedFiles) { - selectedFiles.forEach((file) => { - const uri = path.join(rootUri, file); - workspace.openTextDocument(Uri.parse(uri)).then((doc) => { - window.showTextDocument(doc, { preview: false, preserveFocus: true, viewColumn: 2 }); - }); - }); - } + await showDeadCodePicker(rootUri, deadCode); } }; } diff --git a/packages/vscode-extension/src/common/orphanedFilesOnBoot.spec.ts b/packages/vscode-extension/src/common/orphanedFilesOnBoot.spec.ts new file mode 100644 index 000000000..73811819e --- /dev/null +++ b/packages/vscode-extension/src/common/orphanedFilesOnBoot.spec.ts @@ -0,0 +1,116 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { BaseLanguageClient } from 'vscode-languageclient'; + +const mocks = vi.hoisted(() => ({ + showInformationMessage: vi.fn(), + showQuickPick: vi.fn(), + showTextDocument: vi.fn(), + openTextDocument: vi.fn(() => Promise.resolve({})), + findFiles: vi.fn(), + getConfig: vi.fn(), + updateConfig: vi.fn(), +})); + +vi.mock('vscode', () => ({ + window: { + showInformationMessage: mocks.showInformationMessage, + showQuickPick: mocks.showQuickPick, + showTextDocument: mocks.showTextDocument, + }, + workspace: { + getConfiguration: () => ({ get: mocks.getConfig, update: mocks.updateConfig }), + findFiles: mocks.findFiles, + openTextDocument: mocks.openTextDocument, + }, + commands: { executeCommand: vi.fn() }, + ConfigurationTarget: { Global: 1 }, + Uri: { parse: (s: string) => ({ toString: () => s }) }, + Position: class { + constructor( + public line: number, + public character: number, + ) {} + }, + Range: class { + constructor( + public start: any, + public end: any, + ) {} + }, +})); + +import { checkOrphanedFilesOnBoot } from './orphanedFilesOnBoot'; + +function uriLike(s: string) { + return { toString: () => s } as any; +} + +/** sendRequest resolves root first, then dead code (matches fetchDeadCode's Promise.all order). */ +function clientWith(rootUri: string, deadCode: string[]): BaseLanguageClient { + return { + sendRequest: vi.fn().mockResolvedValueOnce(rootUri).mockResolvedValueOnce(deadCode), + } as unknown as BaseLanguageClient; +} + +describe('checkOrphanedFilesOnBoot', () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.getConfig.mockReturnValue(true); + }); + + it('notifies the user when the theme has orphaned files', async () => { + mocks.findFiles.mockResolvedValue([uriLike('file:///theme/.theme-check.yml')]); + const client = clientWith('file:///theme', ['file:///theme/snippets/unused.liquid']); + + await checkOrphanedFilesOnBoot(client); + + expect(mocks.showInformationMessage).toHaveBeenCalledTimes(1); + expect(mocks.showInformationMessage.mock.calls[0][0]).toContain('1 orphaned file'); + }); + + it('does not notify when there are no orphaned files', async () => { + mocks.findFiles.mockResolvedValue([uriLike('file:///theme/.theme-check.yml')]); + const client = clientWith('file:///theme', []); + + await checkOrphanedFilesOnBoot(client); + + expect(mocks.showInformationMessage).not.toHaveBeenCalled(); + }); + + it('does nothing when the setting is disabled', async () => { + mocks.getConfig.mockReturnValue(false); + const client = clientWith('file:///theme', ['file:///theme/snippets/unused.liquid']); + + await checkOrphanedFilesOnBoot(client); + + expect(mocks.findFiles).not.toHaveBeenCalled(); + expect(client.sendRequest).not.toHaveBeenCalled(); + expect(mocks.showInformationMessage).not.toHaveBeenCalled(); + }); + + it('disables the setting when the user picks "Don\'t show again"', async () => { + mocks.findFiles.mockResolvedValue([uriLike('file:///theme/.theme-check.yml')]); + mocks.showInformationMessage.mockResolvedValue("Don't show again"); + const client = clientWith('file:///theme', ['file:///theme/snippets/unused.liquid']); + + await checkOrphanedFilesOnBoot(client); + + expect(mocks.updateConfig).toHaveBeenCalledWith( + 'themeCheck.checkOrphanedFilesOnBoot', + false, + 1, // ConfigurationTarget.Global + ); + }); + + it('opens the dead-code picker when the user picks "Review"', async () => { + mocks.findFiles.mockResolvedValue([uriLike('file:///theme/.theme-check.yml')]); + mocks.showInformationMessage.mockResolvedValue('Review'); + mocks.showQuickPick.mockResolvedValue(undefined); + const client = clientWith('file:///theme', ['file:///theme/snippets/unused.liquid']); + + await checkOrphanedFilesOnBoot(client); + + expect(mocks.showQuickPick).toHaveBeenCalledTimes(1); + expect(mocks.updateConfig).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/vscode-extension/src/common/orphanedFilesOnBoot.ts b/packages/vscode-extension/src/common/orphanedFilesOnBoot.ts new file mode 100644 index 000000000..e595bdb61 --- /dev/null +++ b/packages/vscode-extension/src/common/orphanedFilesOnBoot.ts @@ -0,0 +1,48 @@ +import { window, workspace, ConfigurationTarget } from 'vscode'; +import { BaseLanguageClient } from 'vscode-languageclient'; +import { fetchDeadCode, showDeadCodePicker } from './commands'; + +export const CHECK_ORPHANED_ON_BOOT_SETTING = 'themeCheck.checkOrphanedFilesOnBoot'; + +const REVIEW = 'Review'; +const DONT_SHOW_AGAIN = "Don't show again"; + +/** + * On extension startup, surface orphaned (dead) files with a single dismissable + * notification instead of requiring the user to run the dead-code command. Gated + * by the `themeCheck.checkOrphanedFilesOnBoot` setting. Aggregates across every + * theme root in the workspace; "Review" opens the picker for the first root that + * has orphaned files. + */ +export async function checkOrphanedFilesOnBoot(client: BaseLanguageClient): Promise { + const enabled = workspace.getConfiguration().get(CHECK_ORPHANED_ON_BOOT_SETTING, true); + if (!enabled) return; + + const configFiles = await workspace.findFiles('**/.theme-check.yml', '**/node_modules/**'); + + let total = 0; + let firstHit: { rootUri: string; deadCode: string[] } | undefined; + for (const file of configFiles) { + const { rootUri, deadCode } = await fetchDeadCode(client, file.toString()); + if (deadCode.length > 0) { + total += deadCode.length; + if (!firstHit) firstHit = { rootUri, deadCode }; + } + } + + if (total === 0 || !firstHit) return; + + const choice = await window.showInformationMessage( + `Found ${total} orphaned file${total === 1 ? '' : 's'} in your theme.`, + REVIEW, + DONT_SHOW_AGAIN, + ); + + if (choice === REVIEW) { + await showDeadCodePicker(firstHit.rootUri, firstHit.deadCode); + } else if (choice === DONT_SHOW_AGAIN) { + await workspace + .getConfiguration() + .update(CHECK_ORPHANED_ON_BOOT_SETTING, false, ConfigurationTarget.Global); + } +} diff --git a/packages/vscode-extension/src/node/extension.ts b/packages/vscode-extension/src/node/extension.ts index 0bf9b99fc..f44908b06 100644 --- a/packages/vscode-extension/src/node/extension.ts +++ b/packages/vscode-extension/src/node/extension.ts @@ -17,6 +17,7 @@ import { watchReferencesTreeViewConfig, } from '../common/ReferencesProvider'; import { makeDeadCode, openLocation } from '../common/commands'; +import { checkOrphanedFilesOnBoot } from '../common/orphanedFilesOnBoot'; const sleep = (ms: number) => new Promise((res) => setTimeout(res, ms)); @@ -47,6 +48,9 @@ export async function activate(context: ExtensionContext) { createReferencesTreeView('shopify.themeGraph.dependencies', context, client, 'dependencies'), watchReferencesTreeViewConfig(), ); + + // Fire-and-forget: surfacing orphaned files must never block or fail activation. + checkOrphanedFilesOnBoot(client).catch((error) => console.error(error)); } } From 2d33a1fb77f680c48eb139242b652a0f76246b6a Mon Sep 17 00:00:00 2001 From: Ryan Tang <24728770+ryandiginomad@users.noreply.github.com> Date: Sat, 13 Jun 2026 22:04:59 +0800 Subject: [PATCH 2/2] fix(theme-check-vscode): cover all theme roots and prompt per root on boot Addresses review feedback on the orphaned-files startup check: - The startup scan only globbed `**/.theme-check.yml`, so theme app extensions (`shopify.extension.toml`) and configless themes (inferred from a `snippets` directory) never got the prompt even though they activate the extension. The glob now matches every root signal that `findRoot` recognises, and the server resolves each anchor to its root. - The notification aggregated the orphaned-file count across all themes but "Review" only opened the first theme's files. It now dedupes by the server-resolved root and prompts once per theme, so each count matches the files its own picker opens. --- .../src/common/orphanedFilesOnBoot.spec.ts | 135 ++++++++++++++++-- .../src/common/orphanedFilesOnBoot.ts | 53 +++++-- 2 files changed, 165 insertions(+), 23 deletions(-) diff --git a/packages/vscode-extension/src/common/orphanedFilesOnBoot.spec.ts b/packages/vscode-extension/src/common/orphanedFilesOnBoot.spec.ts index 73811819e..8ea334b47 100644 --- a/packages/vscode-extension/src/common/orphanedFilesOnBoot.spec.ts +++ b/packages/vscode-extension/src/common/orphanedFilesOnBoot.spec.ts @@ -45,10 +45,24 @@ function uriLike(s: string) { return { toString: () => s } as any; } -/** sendRequest resolves root first, then dead code (matches fetchDeadCode's Promise.all order). */ -function clientWith(rootUri: string, deadCode: string[]): BaseLanguageClient { +/** + * Builds a client whose ThemeGraphRootRequest/ThemeGraphDeadCodeRequest pair + * resolves per anchor uri. `byUri` maps an anchor uri to the root + dead code the + * server would return for it. Matches fetchDeadCode's Promise.all (root, deadCode). + */ +function clientWith( + byUri: Record, +): BaseLanguageClient { return { - sendRequest: vi.fn().mockResolvedValueOnce(rootUri).mockResolvedValueOnce(deadCode), + sendRequest: vi.fn((type: any, params: { uri: string }) => { + const hit = byUri[params.uri]; + // Disambiguate the two requests by the request type's `method` string. + const method: string = type?.method ?? ''; + if (method.toLowerCase().includes('deadcode')) { + return Promise.resolve(hit?.deadCode ?? []); + } + return Promise.resolve(hit?.rootUri ?? ''); + }), } as unknown as BaseLanguageClient; } @@ -58,9 +72,26 @@ describe('checkOrphanedFilesOnBoot', () => { mocks.getConfig.mockReturnValue(true); }); + it('globs for every theme-root signal, not just .theme-check.yml', async () => { + mocks.findFiles.mockResolvedValue([]); + const client = clientWith({}); + + await checkOrphanedFilesOnBoot(client); + + const globPattern = mocks.findFiles.mock.calls[0][0] as string; + expect(globPattern).toContain('.theme-check.yml'); + expect(globPattern).toContain('shopify.extension.toml'); + expect(globPattern).toContain('snippets'); + }); + it('notifies the user when the theme has orphaned files', async () => { mocks.findFiles.mockResolvedValue([uriLike('file:///theme/.theme-check.yml')]); - const client = clientWith('file:///theme', ['file:///theme/snippets/unused.liquid']); + const client = clientWith({ + 'file:///theme/.theme-check.yml': { + rootUri: 'file:///theme', + deadCode: ['file:///theme/snippets/unused.liquid'], + }, + }); await checkOrphanedFilesOnBoot(client); @@ -70,7 +101,9 @@ describe('checkOrphanedFilesOnBoot', () => { it('does not notify when there are no orphaned files', async () => { mocks.findFiles.mockResolvedValue([uriLike('file:///theme/.theme-check.yml')]); - const client = clientWith('file:///theme', []); + const client = clientWith({ + 'file:///theme/.theme-check.yml': { rootUri: 'file:///theme', deadCode: [] }, + }); await checkOrphanedFilesOnBoot(client); @@ -79,7 +112,7 @@ describe('checkOrphanedFilesOnBoot', () => { it('does nothing when the setting is disabled', async () => { mocks.getConfig.mockReturnValue(false); - const client = clientWith('file:///theme', ['file:///theme/snippets/unused.liquid']); + const client = clientWith({}); await checkOrphanedFilesOnBoot(client); @@ -91,7 +124,12 @@ describe('checkOrphanedFilesOnBoot', () => { it('disables the setting when the user picks "Don\'t show again"', async () => { mocks.findFiles.mockResolvedValue([uriLike('file:///theme/.theme-check.yml')]); mocks.showInformationMessage.mockResolvedValue("Don't show again"); - const client = clientWith('file:///theme', ['file:///theme/snippets/unused.liquid']); + const client = clientWith({ + 'file:///theme/.theme-check.yml': { + rootUri: 'file:///theme', + deadCode: ['file:///theme/snippets/unused.liquid'], + }, + }); await checkOrphanedFilesOnBoot(client); @@ -106,11 +144,92 @@ describe('checkOrphanedFilesOnBoot', () => { mocks.findFiles.mockResolvedValue([uriLike('file:///theme/.theme-check.yml')]); mocks.showInformationMessage.mockResolvedValue('Review'); mocks.showQuickPick.mockResolvedValue(undefined); - const client = clientWith('file:///theme', ['file:///theme/snippets/unused.liquid']); + const client = clientWith({ + 'file:///theme/.theme-check.yml': { + rootUri: 'file:///theme', + deadCode: ['file:///theme/snippets/unused.liquid'], + }, + }); await checkOrphanedFilesOnBoot(client); expect(mocks.showQuickPick).toHaveBeenCalledTimes(1); expect(mocks.updateConfig).not.toHaveBeenCalled(); }); + + it('deduplicates anchors that resolve to the same root', async () => { + // A theme with both a .theme-check.yml and a snippets dir yields two anchors + // that the server resolves to the same root — it should only prompt once. + mocks.findFiles.mockResolvedValue([ + uriLike('file:///theme/.theme-check.yml'), + uriLike('file:///theme/snippets/a.liquid'), + ]); + const client = clientWith({ + 'file:///theme/.theme-check.yml': { + rootUri: 'file:///theme', + deadCode: ['file:///theme/snippets/unused.liquid'], + }, + 'file:///theme/snippets/a.liquid': { + rootUri: 'file:///theme', + deadCode: ['file:///theme/snippets/unused.liquid'], + }, + }); + + await checkOrphanedFilesOnBoot(client); + + expect(mocks.showInformationMessage).toHaveBeenCalledTimes(1); + }); + + it('prompts per root in a multi-theme workspace, with each count matching its own picker', async () => { + // Two separate themes, each with its own orphaned files. The aggregated-count + // bug would say "Found 3 orphaned files" once and only open theme-a's subset. + mocks.findFiles.mockResolvedValue([ + uriLike('file:///theme-a/.theme-check.yml'), + uriLike('file:///theme-b/shopify.extension.toml'), + ]); + const client = clientWith({ + 'file:///theme-a/.theme-check.yml': { + rootUri: 'file:///theme-a', + deadCode: ['file:///theme-a/snippets/a1.liquid'], + }, + 'file:///theme-b/shopify.extension.toml': { + rootUri: 'file:///theme-b', + deadCode: ['file:///theme-b/snippets/b1.liquid', 'file:///theme-b/snippets/b2.liquid'], + }, + }); + // Pick "Review" on every notification. + mocks.showInformationMessage.mockResolvedValue('Review'); + mocks.showQuickPick.mockResolvedValue(undefined); + + await checkOrphanedFilesOnBoot(client); + + // One notification per theme with orphans. + expect(mocks.showInformationMessage).toHaveBeenCalledTimes(2); + const messages = mocks.showInformationMessage.mock.calls.map((c) => c[0]); + expect(messages.some((m: string) => m.includes('1 orphaned file'))).toBe(true); + expect(messages.some((m: string) => m.includes('2 orphaned files'))).toBe(true); + + // Review opened a picker for each theme — the picker for theme-b must show + // theme-b's two files, never theme-a's subset. + expect(mocks.showQuickPick).toHaveBeenCalledTimes(2); + }); + + it('only prompts for roots that actually have orphaned files', async () => { + mocks.findFiles.mockResolvedValue([ + uriLike('file:///theme-a/.theme-check.yml'), + uriLike('file:///theme-b/.theme-check.yml'), + ]); + const client = clientWith({ + 'file:///theme-a/.theme-check.yml': { + rootUri: 'file:///theme-a', + deadCode: ['file:///theme-a/snippets/a1.liquid'], + }, + 'file:///theme-b/.theme-check.yml': { rootUri: 'file:///theme-b', deadCode: [] }, + }); + + await checkOrphanedFilesOnBoot(client); + + expect(mocks.showInformationMessage).toHaveBeenCalledTimes(1); + expect(mocks.showInformationMessage.mock.calls[0][0]).toContain('1 orphaned file'); + }); }); diff --git a/packages/vscode-extension/src/common/orphanedFilesOnBoot.ts b/packages/vscode-extension/src/common/orphanedFilesOnBoot.ts index e595bdb61..def50db44 100644 --- a/packages/vscode-extension/src/common/orphanedFilesOnBoot.ts +++ b/packages/vscode-extension/src/common/orphanedFilesOnBoot.ts @@ -8,41 +8,64 @@ const REVIEW = 'Review'; const DONT_SHOW_AGAIN = "Don't show again"; /** - * On extension startup, surface orphaned (dead) files with a single dismissable + * Glob for any file that signals a theme root, mirroring `findRoot`'s notion of a + * root: a `.theme-check.yml`, a `shopify.extension.toml` (theme app extensions), + * or a `snippets` directory (configless themes / zipped themes / TAEs). Each match + * is just an anchor inside a theme — the server resolves it to the actual root. + */ +const ROOT_ANCHOR_GLOB = '**/{.theme-check.yml,shopify.extension.toml,snippets/*}'; + +/** + * On extension startup, surface orphaned (dead) files with a dismissable * notification instead of requiring the user to run the dead-code command. Gated - * by the `themeCheck.checkOrphanedFilesOnBoot` setting. Aggregates across every - * theme root in the workspace; "Review" opens the picker for the first root that - * has orphaned files. + * by the `themeCheck.checkOrphanedFilesOnBoot` setting. + * + * A workspace can contain more than one theme, so this prompts once per theme root + * that has orphaned files — each notification's count matches the files its own + * "Review" picker opens (no cross-theme aggregation). */ export async function checkOrphanedFilesOnBoot(client: BaseLanguageClient): Promise { const enabled = workspace.getConfiguration().get(CHECK_ORPHANED_ON_BOOT_SETTING, true); if (!enabled) return; - const configFiles = await workspace.findFiles('**/.theme-check.yml', '**/node_modules/**'); + const anchors = await workspace.findFiles(ROOT_ANCHOR_GLOB, '**/node_modules/**'); - let total = 0; - let firstHit: { rootUri: string; deadCode: string[] } | undefined; - for (const file of configFiles) { - const { rootUri, deadCode } = await fetchDeadCode(client, file.toString()); - if (deadCode.length > 0) { - total += deadCode.length; - if (!firstHit) firstHit = { rootUri, deadCode }; + // Resolve each anchor to its theme root + dead code, then dedupe by root so a + // theme matched by several anchors is only prompted once. + const byRoot = new Map(); + for (const anchor of anchors) { + const { rootUri, deadCode } = await fetchDeadCode(client, anchor.toString()); + if (rootUri && deadCode.length > 0 && !byRoot.has(rootUri)) { + byRoot.set(rootUri, deadCode); } } - if (total === 0 || !firstHit) return; + for (const [rootUri, deadCode] of byRoot) { + const shouldStop = await promptForRoot(rootUri, deadCode); + if (shouldStop) return; + } +} +/** + * Shows the notification for a single theme root. Returns `true` if the user opted + * out ("Don't show again"), so the caller can stop prompting the remaining roots. + */ +async function promptForRoot(rootUri: string, deadCode: string[]): Promise { + const count = deadCode.length; const choice = await window.showInformationMessage( - `Found ${total} orphaned file${total === 1 ? '' : 's'} in your theme.`, + `Found ${count} orphaned file${count === 1 ? '' : 's'} in your theme.`, REVIEW, DONT_SHOW_AGAIN, ); if (choice === REVIEW) { - await showDeadCodePicker(firstHit.rootUri, firstHit.deadCode); + await showDeadCodePicker(rootUri, deadCode); } else if (choice === DONT_SHOW_AGAIN) { await workspace .getConfiguration() .update(CHECK_ORPHANED_ON_BOOT_SETTING, false, ConfigurationTarget.Global); + return true; } + + return false; }