From 45f801ce80887108c7485d8d5ea118ac3a85cdd4 Mon Sep 17 00:00:00 2001 From: Xu Yiming Date: Wed, 25 Dec 2024 22:29:43 +0800 Subject: [PATCH 01/10] refactor: Separate `createScriptRunnerTask` and `createInstallationTask` from `createTask` --- extensions/npm/src/npmView.ts | 4 +- extensions/npm/src/scriptHover.ts | 4 +- extensions/npm/src/tasks.ts | 81 +++++++++++++++++-------------- 3 files changed, 49 insertions(+), 40 deletions(-) diff --git a/extensions/npm/src/npmView.ts b/extensions/npm/src/npmView.ts index e041b43f0919e..34113f4155326 100644 --- a/extensions/npm/src/npmView.ts +++ b/extensions/npm/src/npmView.ts @@ -13,7 +13,7 @@ import { } from 'vscode'; import { readScripts } from './readScripts'; import { - createTask, getPackageManager, getTaskName, isAutoDetectionEnabled, isWorkspaceFolder, INpmTaskDefinition, + createInstallationTask, getPackageManager, getTaskName, isAutoDetectionEnabled, isWorkspaceFolder, INpmTaskDefinition, NpmTaskProvider, startDebugging, ITaskWithLocation, @@ -181,7 +181,7 @@ export class NpmScriptsTreeDataProvider implements TreeDataProvider { if (!uri) { return; } - const task = await createTask(await getPackageManager(this.context, selection.folder.workspaceFolder.uri, true), 'install', ['install'], selection.folder.workspaceFolder, uri, undefined, []); + const task = await createInstallationTask(await getPackageManager(this.context, selection.folder.workspaceFolder.uri, true), selection.folder.workspaceFolder, uri); tasks.executeTask(task); } diff --git a/extensions/npm/src/scriptHover.ts b/extensions/npm/src/scriptHover.ts index b3a87e0519821..46f076b517efb 100644 --- a/extensions/npm/src/scriptHover.ts +++ b/extensions/npm/src/scriptHover.ts @@ -12,7 +12,7 @@ import { } from 'vscode'; import { INpmScriptInfo, readScripts } from './readScripts'; import { - createTask, + createScriptRunnerTask, getPackageManager, startDebugging } from './tasks'; @@ -114,7 +114,7 @@ export class NpmScriptHoverProvider implements HoverProvider { const documentUri = args.documentUri; const folder = workspace.getWorkspaceFolder(documentUri); if (folder) { - const task = await createTask(await getPackageManager(this.context, folder.uri), script, ['run', script], folder, documentUri); + const task = await createScriptRunnerTask(await getPackageManager(this.context, folder.uri), script, folder, documentUri); await tasks.executeTask(task); } } diff --git a/extensions/npm/src/tasks.ts b/extensions/npm/src/tasks.ts index 6f1a1fbe649cf..2edb9ff9ccfb6 100644 --- a/extensions/npm/src/tasks.ts +++ b/extensions/npm/src/tasks.ts @@ -71,11 +71,10 @@ export class NpmTaskProvider implements TaskProvider { } else { packageJsonUri = _task.scope.uri.with({ path: _task.scope.uri.path + '/package.json' }); } - const cmd = [kind.script]; - if (kind.script !== INSTALL_SCRIPT) { - cmd.unshift('run'); + if (kind.script === INSTALL_SCRIPT) { + return createInstallationTask(await getPackageManager(this.context, _task.scope.uri), _task.scope, packageJsonUri); } - return createTask(await getPackageManager(this.context, _task.scope.uri), kind, cmd, _task.scope, packageJsonUri); + return createScriptRunnerTask(await getPackageManager(this.context, _task.scope.uri), kind.script, _task.scope, packageJsonUri); } return undefined; } @@ -281,12 +280,12 @@ async function provideNpmScriptsForFolder(context: ExtensionContext, packageJson const packageManager = await getPackageManager(context, folder.uri, showWarning); for (const { name, value, nameRange } of scripts.scripts) { - const task = await createTask(packageManager, name, ['run', name], folder!, packageJsonUri, value, undefined); + const task = await createScriptRunnerTask(packageManager, name, folder!, packageJsonUri, value); result.push({ task, location: new Location(packageJsonUri, nameRange) }); } if (!workspace.getConfiguration('npm', folder).get('scriptExplorerExclude', []).find(e => e.includes(INSTALL_SCRIPT))) { - result.push({ task: await createTask(packageManager, INSTALL_SCRIPT, [INSTALL_SCRIPT], folder, packageJsonUri, 'install dependencies from package', []) }); + result.push({ task: await createInstallationTask(packageManager, folder, packageJsonUri, 'install dependencies from package') }); } return result; } @@ -298,45 +297,39 @@ export function getTaskName(script: string, relativePath: string | undefined) { return script; } -export async function createTask(packageManager: string, script: INpmTaskDefinition | string, cmd: string[], folder: WorkspaceFolder, packageJsonUri: Uri, scriptValue?: string, matcher?: any): Promise { - let kind: INpmTaskDefinition; - if (typeof script === 'string') { - kind = { type: 'npm', script: script }; - } else { - kind = script; - } - - function getCommandLine(cmd: string[]): (string | ShellQuotedString)[] { - const result: (string | ShellQuotedString)[] = new Array(cmd.length); - for (let i = 0; i < cmd.length; i++) { - if (/\s/.test(cmd[i])) { - result[i] = { value: cmd[i], quoting: cmd[i].includes('--') ? ShellQuoting.Weak : ShellQuoting.Strong }; - } else { - result[i] = cmd[i]; - } - } - if (workspace.getConfiguration('npm', folder.uri).get('runSilent')) { - result.unshift('--silent'); +function getCommandLine(scope: Uri, cmd: string[]): (string | ShellQuotedString)[] { + const result: (string | ShellQuotedString)[] = new Array(cmd.length); + for (let i = 0; i < cmd.length; i++) { + if (/\s/.test(cmd[i])) { + result[i] = { value: cmd[i], quoting: cmd[i].includes('--') ? ShellQuoting.Weak : ShellQuoting.Strong }; + } else { + result[i] = cmd[i]; } - return result; } - - function getRelativePath(packageJsonUri: Uri): string { - const rootUri = folder.uri; - const absolutePath = packageJsonUri.path.substring(0, packageJsonUri.path.length - 'package.json'.length); - return absolutePath.substring(rootUri.path.length + 1); + if (workspace.getConfiguration('npm', scope).get('runSilent')) { + result.unshift('--silent'); } + return result; +} + +function getRelativePath(rootUri: Uri, packageJsonUri: Uri): string { + const absolutePath = packageJsonUri.path.substring(0, packageJsonUri.path.length - 'package.json'.length); + return absolutePath.substring(rootUri.path.length + 1); +} - const relativePackageJson = getRelativePath(packageJsonUri); +export async function createScriptRunnerTask(packageManager: string, script: string, folder: WorkspaceFolder, packageJsonUri: Uri, scriptValue?: string): Promise { + const kind: INpmTaskDefinition = { type: 'npm', script }; + + const relativePackageJson = getRelativePath(folder.uri, packageJsonUri); if (relativePackageJson.length && !kind.path) { kind.path = relativePackageJson.substring(0, relativePackageJson.length - 1); } - const taskName = getTaskName(kind.script, relativePackageJson); + const taskName = getTaskName(script, relativePackageJson); const cwd = path.dirname(packageJsonUri.fsPath); - const task = new Task(kind, folder, taskName, 'npm', new ShellExecution(packageManager, getCommandLine(cmd), { cwd: cwd }), matcher); + const task = new Task(kind, folder, taskName, 'npm', new ShellExecution(packageManager, getCommandLine(folder.uri, ['run', script]), { cwd: cwd })); task.detail = scriptValue; - const lowerCaseTaskName = kind.script.toLowerCase(); + const lowerCaseTaskName = script.toLowerCase(); if (isBuildTask(lowerCaseTaskName)) { task.group = TaskGroup.Build; } else if (isTestTask(lowerCaseTaskName)) { @@ -350,6 +343,22 @@ export async function createTask(packageManager: string, script: INpmTaskDefinit return task; } +export async function createInstallationTask(packageManager: string, folder: WorkspaceFolder, packageJsonUri: Uri, scriptValue?: string): Promise { + const kind: INpmTaskDefinition = { type: 'npm', script: INSTALL_SCRIPT }; + + const relativePackageJson = getRelativePath(folder.uri, packageJsonUri); + if (relativePackageJson.length && !kind.path) { + kind.path = relativePackageJson.substring(0, relativePackageJson.length - 1); + } + const taskName = getTaskName(INSTALL_SCRIPT, relativePackageJson); + const cwd = path.dirname(packageJsonUri.fsPath); + const task = new Task(kind, folder, taskName, 'npm', new ShellExecution(packageManager, getCommandLine(folder.uri, [INSTALL_SCRIPT]), { cwd: cwd })); + task.detail = scriptValue; + task.group = TaskGroup.Clean; + + return task; +} + export function getPackageJsonUriFromTask(task: Task): Uri | null { if (isWorkspaceFolder(task.scope)) { @@ -403,7 +412,7 @@ export async function runScript(context: ExtensionContext, script: string, docum const uri = document.uri; const folder = workspace.getWorkspaceFolder(uri); if (folder) { - const task = await createTask(await getPackageManager(context, folder.uri), script, ['run', script], folder, uri); + const task = await createScriptRunnerTask(await getPackageManager(context, folder.uri), script, folder, uri); tasks.executeTask(task); } } From a5f2eac6fd35e148fbf69a12491d81299606fc4b Mon Sep 17 00:00:00 2001 From: Xu Yiming Date: Thu, 26 Dec 2024 03:21:18 +0800 Subject: [PATCH 02/10] feat: Add `npm.scriptRunner` --- extensions/npm/README.md | 3 +- extensions/npm/package.json | 20 ++++++ extensions/npm/package.nls.json | 18 ++++-- extensions/npm/src/npmMain.ts | 10 ++- extensions/npm/src/npmScriptLens.ts | 6 +- extensions/npm/src/npmView.ts | 9 +-- extensions/npm/src/scriptHover.ts | 4 +- extensions/npm/src/tasks.ts | 95 +++++++++++++++++------------ 8 files changed, 109 insertions(+), 56 deletions(-) diff --git a/extensions/npm/README.md b/extensions/npm/README.md index 215ca927ff434..acc7d9872389b 100644 --- a/extensions/npm/README.md +++ b/extensions/npm/README.md @@ -34,7 +34,8 @@ The extension fetches data from and context.subscriptions.push(vscode.commands.registerCommand('npm.refresh', () => { invalidateScriptCaches(); })); + context.subscriptions.push(vscode.commands.registerCommand('npm.scriptRunner', (args) => { + if (args instanceof vscode.Uri) { + return getScriptRunner(args, context, true); + } + return ''; + })); context.subscriptions.push(vscode.commands.registerCommand('npm.packageManager', (args) => { if (args instanceof vscode.Uri) { - return getPackageManager(context, args); + return getPackageManager(args, context, true); } return ''; })); diff --git a/extensions/npm/src/npmScriptLens.ts b/extensions/npm/src/npmScriptLens.ts index c8e506904f882..fc7f2a854444d 100644 --- a/extensions/npm/src/npmScriptLens.ts +++ b/extensions/npm/src/npmScriptLens.ts @@ -15,8 +15,8 @@ import { workspace, l10n } from 'vscode'; -import { findPreferredPM } from './preferred-pm'; import { readScripts } from './readScripts'; +import { getScriptRunner } from './tasks'; const enum Constants { @@ -87,7 +87,7 @@ export class NpmScriptLensProvider implements CodeLensProvider, Disposable { } if (this.lensLocation === 'all') { - const packageManager = await findPreferredPM(Uri.joinPath(document.uri, '..').fsPath); + const scriptRunner = await getScriptRunner(Uri.joinPath(document.uri, '..')); return tokens.scripts.map( ({ name, nameRange }) => new CodeLens( @@ -95,7 +95,7 @@ export class NpmScriptLensProvider implements CodeLensProvider, Disposable { { title, command: 'extension.js-debug.createDebuggerTerminal', - arguments: [`${packageManager.name} run ${name}`, workspace.getWorkspaceFolder(document.uri), { cwd }], + arguments: [`${scriptRunner} run ${name}`, workspace.getWorkspaceFolder(document.uri), { cwd }], }, ), ); diff --git a/extensions/npm/src/npmView.ts b/extensions/npm/src/npmView.ts index 34113f4155326..027d7f60e5487 100644 --- a/extensions/npm/src/npmView.ts +++ b/extensions/npm/src/npmView.ts @@ -13,9 +13,10 @@ import { } from 'vscode'; import { readScripts } from './readScripts'; import { - createInstallationTask, getPackageManager, getTaskName, isAutoDetectionEnabled, isWorkspaceFolder, INpmTaskDefinition, + createInstallationTask, getTaskName, isAutoDetectionEnabled, isWorkspaceFolder, INpmTaskDefinition, NpmTaskProvider, startDebugging, + detectPackageManager, ITaskWithLocation, INSTALL_SCRIPT } from './tasks'; @@ -150,8 +151,8 @@ export class NpmScriptsTreeDataProvider implements TreeDataProvider { } private async runScript(script: NpmScript) { - // Call getPackageManager to trigger the multiple lock files warning. - await getPackageManager(this.context, script.getFolder().uri); + // Call detectPackageManager to trigger the multiple lock files warning. + await detectPackageManager(script.getFolder().uri, this.context, true); tasks.executeTask(script.task); } @@ -181,7 +182,7 @@ export class NpmScriptsTreeDataProvider implements TreeDataProvider { if (!uri) { return; } - const task = await createInstallationTask(await getPackageManager(this.context, selection.folder.workspaceFolder.uri, true), selection.folder.workspaceFolder, uri); + const task = await createInstallationTask(this.context, selection.folder.workspaceFolder, uri); tasks.executeTask(task); } diff --git a/extensions/npm/src/scriptHover.ts b/extensions/npm/src/scriptHover.ts index 46f076b517efb..33f2346e81588 100644 --- a/extensions/npm/src/scriptHover.ts +++ b/extensions/npm/src/scriptHover.ts @@ -13,7 +13,7 @@ import { import { INpmScriptInfo, readScripts } from './readScripts'; import { createScriptRunnerTask, - getPackageManager, startDebugging + startDebugging } from './tasks'; @@ -114,7 +114,7 @@ export class NpmScriptHoverProvider implements HoverProvider { const documentUri = args.documentUri; const folder = workspace.getWorkspaceFolder(documentUri); if (folder) { - const task = await createScriptRunnerTask(await getPackageManager(this.context, folder.uri), script, folder, documentUri); + const task = await createScriptRunnerTask(this.context, script, folder, documentUri); await tasks.executeTask(task); } } diff --git a/extensions/npm/src/tasks.ts b/extensions/npm/src/tasks.ts index 2edb9ff9ccfb6..c699cd9925c5f 100644 --- a/extensions/npm/src/tasks.ts +++ b/extensions/npm/src/tasks.ts @@ -72,9 +72,9 @@ export class NpmTaskProvider implements TaskProvider { packageJsonUri = _task.scope.uri.with({ path: _task.scope.uri.path + '/package.json' }); } if (kind.script === INSTALL_SCRIPT) { - return createInstallationTask(await getPackageManager(this.context, _task.scope.uri), _task.scope, packageJsonUri); + return createInstallationTask(this.context, _task.scope, packageJsonUri); } - return createScriptRunnerTask(await getPackageManager(this.context, _task.scope.uri), kind.script, _task.scope, packageJsonUri); + return createScriptRunnerTask(this.context, kind.script, _task.scope, packageJsonUri); } return undefined; } @@ -125,27 +125,43 @@ export function isWorkspaceFolder(value: any): value is WorkspaceFolder { return value && typeof value !== 'number'; } -export async function getPackageManager(extensionContext: ExtensionContext, folder: Uri, showWarning: boolean = true): Promise { - let packageManagerName = workspace.getConfiguration('npm', folder).get('packageManager', 'npm'); - - if (packageManagerName === 'auto') { - const { name, multipleLockFilesDetected: multiplePMDetected } = await findPreferredPM(folder.fsPath); - packageManagerName = name; - const neverShowWarning = 'npm.multiplePMWarning.neverShow'; - if (showWarning && multiplePMDetected && !extensionContext.globalState.get(neverShowWarning)) { - const multiplePMWarning = l10n.t('Using {0} as the preferred package manager. Found multiple lockfiles for {1}. To resolve this issue, delete the lockfiles that don\'t match your preferred package manager or change the setting "npm.packageManager" to a value other than "auto".', packageManagerName, folder.fsPath); - const neverShowAgain = l10n.t("Do not show again"); - const learnMore = l10n.t("Learn more"); - window.showInformationMessage(multiplePMWarning, learnMore, neverShowAgain).then(result => { - switch (result) { - case neverShowAgain: extensionContext.globalState.update(neverShowWarning, true); break; - case learnMore: env.openExternal(Uri.parse('https://docs.npmjs.com/cli/v9/configuring-npm/package-lock-json')); - } - }); - } +export async function getScriptRunner(folder: Uri, context?: ExtensionContext, showWarning?: boolean): Promise { + let scriptRunner = workspace.getConfiguration('npm', folder).get('scriptRunner', 'npm'); + + if (scriptRunner === 'auto') { + scriptRunner = await detectPackageManager(folder, context, showWarning); + } + + return scriptRunner; +} + +export async function getPackageManager(folder: Uri, context?: ExtensionContext, showWarning?: boolean): Promise { + let packageManager = workspace.getConfiguration('npm', folder).get('packageManager', 'npm'); + + if (packageManager === 'auto') { + packageManager = await detectPackageManager(folder, context, showWarning); } - return packageManagerName; + return packageManager; +} + +export async function detectPackageManager(folder: Uri, extensionContext?: ExtensionContext, showWarning: boolean = false): Promise { + const { name, multipleLockFilesDetected: multiplePMDetected } = await findPreferredPM(folder.fsPath); + const neverShowWarning = 'npm.multiplePMWarning.neverShow'; + if (showWarning && multiplePMDetected && extensionContext && !extensionContext.globalState.get(neverShowWarning)) { + // todo: add text for npm.scriptRunner? + const multiplePMWarning = l10n.t('Using {0} as the preferred package manager. Found multiple lockfiles for {1}. To resolve this issue, delete the lockfiles that don\'t match your preferred package manager or change the setting "npm.packageManager" to a value other than "auto".', name, folder.fsPath); + const neverShowAgain = l10n.t("Do not show again"); + const learnMore = l10n.t("Learn more"); + window.showInformationMessage(multiplePMWarning, learnMore, neverShowAgain).then(result => { + switch (result) { + case neverShowAgain: extensionContext.globalState.update(neverShowWarning, true); break; + case learnMore: env.openExternal(Uri.parse('https://docs.npmjs.com/cli/v9/configuring-npm/package-lock-json')); + } + }); + } + + return name; } export async function hasNpmScripts(): Promise { @@ -169,15 +185,13 @@ export async function hasNpmScripts(): Promise { } } -async function detectNpmScripts(context: ExtensionContext, showWarning: boolean): Promise { +async function* findNpmPackages(): AsyncGenerator { - const emptyTasks: ITaskWithLocation[] = []; - const allTasks: ITaskWithLocation[] = []; const visitedPackageJsonFiles: Set = new Set(); const folders = workspace.workspaceFolders; if (!folders) { - return emptyTasks; + return; } try { for (const folder of folders) { @@ -186,14 +200,12 @@ async function detectNpmScripts(context: ExtensionContext, showWarning: boolean) const paths = await workspace.findFiles(relativePattern, '**/{node_modules,.vscode-test}/**'); for (const path of paths) { if (!isExcluded(folder, path) && !visitedPackageJsonFiles.has(path.fsPath)) { - const tasks = await provideNpmScriptsForFolder(context, path, showWarning); + yield path; visitedPackageJsonFiles.add(path.fsPath); - allTasks.push(...tasks); } } } } - return allTasks; } catch (error) { return Promise.reject(error); } @@ -227,7 +239,12 @@ export async function detectNpmScriptsForFolder(context: ExtensionContext, folde export async function provideNpmScripts(context: ExtensionContext, showWarning: boolean): Promise { if (!cachedTasks) { - cachedTasks = await detectNpmScripts(context, showWarning); + const allTasks: ITaskWithLocation[] = []; + for await (const path of findNpmPackages()) { + const tasks = await provideNpmScriptsForFolder(context, path, showWarning); + allTasks.push(...tasks); + } + cachedTasks = allTasks; } return cachedTasks; } @@ -277,15 +294,13 @@ async function provideNpmScriptsForFolder(context: ExtensionContext, packageJson const result: ITaskWithLocation[] = []; - const packageManager = await getPackageManager(context, folder.uri, showWarning); - for (const { name, value, nameRange } of scripts.scripts) { - const task = await createScriptRunnerTask(packageManager, name, folder!, packageJsonUri, value); + const task = await createScriptRunnerTask(context, name, folder!, packageJsonUri, value, showWarning); result.push({ task, location: new Location(packageJsonUri, nameRange) }); } if (!workspace.getConfiguration('npm', folder).get('scriptExplorerExclude', []).find(e => e.includes(INSTALL_SCRIPT))) { - result.push({ task: await createInstallationTask(packageManager, folder, packageJsonUri, 'install dependencies from package') }); + result.push({ task: await createInstallationTask(context, folder, packageJsonUri, 'install dependencies from package', showWarning) }); } return result; } @@ -317,7 +332,8 @@ function getRelativePath(rootUri: Uri, packageJsonUri: Uri): string { return absolutePath.substring(rootUri.path.length + 1); } -export async function createScriptRunnerTask(packageManager: string, script: string, folder: WorkspaceFolder, packageJsonUri: Uri, scriptValue?: string): Promise { +export async function createScriptRunnerTask(context: ExtensionContext, script: string, folder: WorkspaceFolder, packageJsonUri: Uri, scriptValue?: string, showWarning = true): Promise { + const scriptRunner = await getScriptRunner(folder.uri, context, showWarning); const kind: INpmTaskDefinition = { type: 'npm', script }; const relativePackageJson = getRelativePath(folder.uri, packageJsonUri); @@ -326,7 +342,7 @@ export async function createScriptRunnerTask(packageManager: string, script: str } const taskName = getTaskName(script, relativePackageJson); const cwd = path.dirname(packageJsonUri.fsPath); - const task = new Task(kind, folder, taskName, 'npm', new ShellExecution(packageManager, getCommandLine(folder.uri, ['run', script]), { cwd: cwd })); + const task = new Task(kind, folder, taskName, 'npm', new ShellExecution(scriptRunner, getCommandLine(folder.uri, ['run', script]), { cwd: cwd })); task.detail = scriptValue; const lowerCaseTaskName = script.toLowerCase(); @@ -343,7 +359,8 @@ export async function createScriptRunnerTask(packageManager: string, script: str return task; } -export async function createInstallationTask(packageManager: string, folder: WorkspaceFolder, packageJsonUri: Uri, scriptValue?: string): Promise { +export async function createInstallationTask(context: ExtensionContext, folder: WorkspaceFolder, packageJsonUri: Uri, scriptValue?: string, showWarning = true): Promise { + const packageManager = await getPackageManager(folder.uri, context, showWarning); const kind: INpmTaskDefinition = { type: 'npm', script: INSTALL_SCRIPT }; const relativePackageJson = getRelativePath(folder.uri, packageJsonUri); @@ -412,15 +429,17 @@ export async function runScript(context: ExtensionContext, script: string, docum const uri = document.uri; const folder = workspace.getWorkspaceFolder(uri); if (folder) { - const task = await createScriptRunnerTask(await getPackageManager(context, folder.uri), script, folder, uri); + const task = await createScriptRunnerTask(context, script, folder, uri); tasks.executeTask(task); } } export async function startDebugging(context: ExtensionContext, scriptName: string, cwd: string, folder: WorkspaceFolder) { + const scriptRunner = await getScriptRunner(folder.uri, context, true); + commands.executeCommand( 'extension.js-debug.createDebuggerTerminal', - `${await getPackageManager(context, folder.uri)} run ${scriptName}`, + `${scriptRunner} run ${scriptName}`, folder, { cwd }, ); From 18620904e6a54647c3c4da17c8af4f849458c546 Mon Sep 17 00:00:00 2001 From: Xu Yiming Date: Thu, 26 Dec 2024 20:42:53 +0800 Subject: [PATCH 03/10] feat: Add Node.js as script runner --- extensions/npm/README.md | 2 +- extensions/npm/package.json | 28 +++++++++++++++------------- extensions/npm/package.nls.json | 1 + extensions/npm/src/npmScriptLens.ts | 2 +- extensions/npm/src/tasks.ts | 4 ++-- 5 files changed, 20 insertions(+), 17 deletions(-) diff --git a/extensions/npm/README.md b/extensions/npm/README.md index acc7d9872389b..d5538706019be 100644 --- a/extensions/npm/README.md +++ b/extensions/npm/README.md @@ -35,7 +35,7 @@ The extension fetches data from and Date: Sat, 28 Dec 2024 17:55:56 +0800 Subject: [PATCH 04/10] refactor: Refactor `isPrePostScript` --- extensions/npm/src/tasks.ts | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/extensions/npm/src/tasks.ts b/extensions/npm/src/tasks.ts index c360c6ad25093..b6db1680e411f 100644 --- a/extensions/npm/src/tasks.ts +++ b/extensions/npm/src/tasks.ts @@ -103,22 +103,18 @@ function isTestTask(name: string): boolean { } return false; } +const preScripts: Set = new Set([ + 'est', /* test typo? */ 'install', 'pack', 'pack', 'publish', 'restart', + 'shrinkwrap', 'stop', 'uninstall', 'version' +]); -function isPrePostScript(name: string): boolean { - const prePostScripts: Set = new Set([ - 'preuninstall', 'postuninstall', 'prepack', 'postpack', 'preinstall', 'postinstall', - 'prepack', 'postpack', 'prepublish', 'postpublish', 'preversion', 'postversion', - 'prestop', 'poststop', 'prerestart', 'postrestart', 'preshrinkwrap', 'postshrinkwrap', - 'pretest', 'postest', 'prepublishOnly' - ]); - - const prepost = ['pre' + name, 'post' + name]; - for (const knownScript of prePostScripts) { - if (knownScript === prepost[0] || knownScript === prepost[1]) { - return true; - } - } - return false; +const postScripts: Set = new Set([ + 'install', 'pack', 'pack', 'publish', 'publishOnly', 'restart', 'shrinkwrap', + 'stop', 'test', 'uninstall', 'version' +]); + +function canHavePrePostScript(name: string): boolean { + return preScripts.has(name) || postScripts.has(name); } export function isWorkspaceFolder(value: any): value is WorkspaceFolder { @@ -350,7 +346,7 @@ export async function createScriptRunnerTask(context: ExtensionContext, script: task.group = TaskGroup.Build; } else if (isTestTask(lowerCaseTaskName)) { task.group = TaskGroup.Test; - } else if (isPrePostScript(lowerCaseTaskName)) { + } else if (canHavePrePostScript(lowerCaseTaskName)) { task.group = TaskGroup.Clean; // hack: use Clean group to tag pre/post scripts } else if (scriptValue && isDebugScript(scriptValue)) { // todo@connor4312: all scripts are now debuggable, what is a 'debug script'? From 8b1e237e306c4c931b85b7974fb9ac0d440e6cb8 Mon Sep 17 00:00:00 2001 From: Xu Yiming Date: Tue, 7 Jan 2025 21:38:29 +0800 Subject: [PATCH 05/10] refactor: Extract `get*Command` --- extensions/npm/src/npmScriptLens.ts | 18 +++++++------ extensions/npm/src/tasks.ts | 42 ++++++++++++++++++++--------- 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/extensions/npm/src/npmScriptLens.ts b/extensions/npm/src/npmScriptLens.ts index f02502e8f990d..84dfeca86aa0e 100644 --- a/extensions/npm/src/npmScriptLens.ts +++ b/extensions/npm/src/npmScriptLens.ts @@ -16,7 +16,7 @@ import { l10n } from 'vscode'; import { readScripts } from './readScripts'; -import { getScriptRunner } from './tasks'; +import { getRunScriptCommand } from './tasks'; const enum Constants { @@ -87,18 +87,20 @@ export class NpmScriptLensProvider implements CodeLensProvider, Disposable { } if (this.lensLocation === 'all') { - const scriptRunner = await getScriptRunner(Uri.joinPath(document.uri, '..')); - return tokens.scripts.map( - ({ name, nameRange }) => - new CodeLens( + const folder = Uri.joinPath(document.uri, '..'); + return Promise.all(tokens.scripts.map( + async ({ name, nameRange }) => { + const runScriptCommand = await getRunScriptCommand(name, folder); + return new CodeLens( nameRange, { title, command: 'extension.js-debug.createDebuggerTerminal', - arguments: [`${scriptRunner} ${scriptRunner === 'node' ? '--run' : 'run'} ${name}`, workspace.getWorkspaceFolder(document.uri), { cwd }], + arguments: [runScriptCommand.join(' '), workspace.getWorkspaceFolder(document.uri), { cwd }], }, - ), - ); + ); + }, + )); } return []; diff --git a/extensions/npm/src/tasks.ts b/extensions/npm/src/tasks.ts index b6db1680e411f..bb0f7f7b388e6 100644 --- a/extensions/npm/src/tasks.ts +++ b/extensions/npm/src/tasks.ts @@ -308,7 +308,7 @@ export function getTaskName(script: string, relativePath: string | undefined) { return script; } -function getCommandLine(scope: Uri, cmd: string[]): (string | ShellQuotedString)[] { +function escapeCommandLine(cmd: string[]): (string | ShellQuotedString)[] { const result: (string | ShellQuotedString)[] = new Array(cmd.length); for (let i = 0; i < cmd.length; i++) { if (/\s/.test(cmd[i])) { @@ -317,9 +317,6 @@ function getCommandLine(scope: Uri, cmd: string[]): (string | ShellQuotedString) result[i] = cmd[i]; } } - if (workspace.getConfiguration('npm', scope).get('runSilent')) { - result.unshift('--silent'); - } return result; } @@ -328,8 +325,17 @@ function getRelativePath(rootUri: Uri, packageJsonUri: Uri): string { return absolutePath.substring(rootUri.path.length + 1); } -export async function createScriptRunnerTask(context: ExtensionContext, script: string, folder: WorkspaceFolder, packageJsonUri: Uri, scriptValue?: string, showWarning = true): Promise { - const scriptRunner = await getScriptRunner(folder.uri, context, showWarning); +export async function getRunScriptCommand(script: string, folder: Uri, context?: ExtensionContext, showWarning = true): Promise { + const scriptRunner = await getScriptRunner(folder, context, showWarning); + const result = [scriptRunner, scriptRunner === 'node' ? '--run' : 'run']; + if (workspace.getConfiguration('npm', folder).get('runSilent')) { + result.push('--silent'); + } + result.push(script); + return result; +} + +export async function createScriptRunnerTask(context: ExtensionContext, script: string, folder: WorkspaceFolder, packageJsonUri: Uri, scriptValue?: string, showWarning?: boolean): Promise { const kind: INpmTaskDefinition = { type: 'npm', script }; const relativePackageJson = getRelativePath(folder.uri, packageJsonUri); @@ -338,7 +344,9 @@ export async function createScriptRunnerTask(context: ExtensionContext, script: } const taskName = getTaskName(script, relativePackageJson); const cwd = path.dirname(packageJsonUri.fsPath); - const task = new Task(kind, folder, taskName, 'npm', new ShellExecution(scriptRunner, getCommandLine(folder.uri, [scriptRunner === 'node' ? '--run' : 'run', script]), { cwd: cwd })); + const args = await getRunScriptCommand(script, folder.uri, context, showWarning); + const scriptRunner = args.shift()!; + const task = new Task(kind, folder, taskName, 'npm', new ShellExecution(scriptRunner, escapeCommandLine(args), { cwd: cwd })); task.detail = scriptValue; const lowerCaseTaskName = script.toLowerCase(); @@ -355,8 +363,16 @@ export async function createScriptRunnerTask(context: ExtensionContext, script: return task; } -export async function createInstallationTask(context: ExtensionContext, folder: WorkspaceFolder, packageJsonUri: Uri, scriptValue?: string, showWarning = true): Promise { - const packageManager = await getPackageManager(folder.uri, context, showWarning); +async function getInstallDependenciesCommand(folder: Uri, context?: ExtensionContext, showWarning = true): Promise { + const packageManager = await getPackageManager(folder, context, showWarning); + const result = [packageManager, INSTALL_SCRIPT]; + if (workspace.getConfiguration('npm', folder).get('runSilent')) { + result.push('--silent'); + } + return result; +} + +export async function createInstallationTask(context: ExtensionContext, folder: WorkspaceFolder, packageJsonUri: Uri, scriptValue?: string, showWarning?: boolean): Promise { const kind: INpmTaskDefinition = { type: 'npm', script: INSTALL_SCRIPT }; const relativePackageJson = getRelativePath(folder.uri, packageJsonUri); @@ -365,7 +381,9 @@ export async function createInstallationTask(context: ExtensionContext, folder: } const taskName = getTaskName(INSTALL_SCRIPT, relativePackageJson); const cwd = path.dirname(packageJsonUri.fsPath); - const task = new Task(kind, folder, taskName, 'npm', new ShellExecution(packageManager, getCommandLine(folder.uri, [INSTALL_SCRIPT]), { cwd: cwd })); + const args = await getInstallDependenciesCommand(folder.uri, context, showWarning); + const packageManager = args.shift()!; + const task = new Task(kind, folder, taskName, 'npm', new ShellExecution(packageManager, escapeCommandLine(args), { cwd: cwd })); task.detail = scriptValue; task.group = TaskGroup.Clean; @@ -431,11 +449,11 @@ export async function runScript(context: ExtensionContext, script: string, docum } export async function startDebugging(context: ExtensionContext, scriptName: string, cwd: string, folder: WorkspaceFolder) { - const scriptRunner = await getScriptRunner(folder.uri, context, true); + const runScriptCommand = await getRunScriptCommand(scriptName, folder.uri, context, true); commands.executeCommand( 'extension.js-debug.createDebuggerTerminal', - `${scriptRunner} ${scriptRunner === 'node' ? '--run' : 'run'} ${scriptName}`, + runScriptCommand.join(' '), folder, { cwd }, ); From beb90bf77488f5b5cf490ffbbe7bbee9738071ac Mon Sep 17 00:00:00 2001 From: Xu Yiming Date: Thu, 9 Jan 2025 18:45:16 +0800 Subject: [PATCH 06/10] fix: Typo --- extensions/npm/src/tasks.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/npm/src/tasks.ts b/extensions/npm/src/tasks.ts index bb0f7f7b388e6..154880a9c41da 100644 --- a/extensions/npm/src/tasks.ts +++ b/extensions/npm/src/tasks.ts @@ -104,8 +104,8 @@ function isTestTask(name: string): boolean { return false; } const preScripts: Set = new Set([ - 'est', /* test typo? */ 'install', 'pack', 'pack', 'publish', 'restart', - 'shrinkwrap', 'stop', 'uninstall', 'version' + 'install', 'pack', 'pack', 'publish', 'restart', 'shrinkwrap', + 'stop', 'test', 'uninstall', 'version' ]); const postScripts: Set = new Set([ From 4f08efb3a53e97715598161ea5f0e74a3b9d911c Mon Sep 17 00:00:00 2001 From: Xu Yiming Date: Thu, 9 Jan 2025 18:47:19 +0800 Subject: [PATCH 07/10] style: Remove no-op `catch`es --- extensions/npm/src/tasks.ts | 68 +++++++++++++++---------------------- 1 file changed, 28 insertions(+), 40 deletions(-) diff --git a/extensions/npm/src/tasks.ts b/extensions/npm/src/tasks.ts index 154880a9c41da..a32e8cc2dc0fe 100644 --- a/extensions/npm/src/tasks.ts +++ b/extensions/npm/src/tasks.ts @@ -165,20 +165,16 @@ export async function hasNpmScripts(): Promise { if (!folders) { return false; } - try { - for (const folder of folders) { - if (isAutoDetectionEnabled(folder) && !excludeRegex.test(Utils.basename(folder.uri))) { - const relativePattern = new RelativePattern(folder, '**/package.json'); - const paths = await workspace.findFiles(relativePattern, '**/node_modules/**'); - if (paths.length > 0) { - return true; - } + for (const folder of folders) { + if (isAutoDetectionEnabled(folder) && !excludeRegex.test(Utils.basename(folder.uri))) { + const relativePattern = new RelativePattern(folder, '**/package.json'); + const paths = await workspace.findFiles(relativePattern, '**/node_modules/**'); + if (paths.length > 0) { + return true; } } - return false; - } catch (error) { - return Promise.reject(error); } + return false; } async function* findNpmPackages(): AsyncGenerator { @@ -189,21 +185,17 @@ async function* findNpmPackages(): AsyncGenerator { if (!folders) { return; } - try { - for (const folder of folders) { - if (isAutoDetectionEnabled(folder) && !excludeRegex.test(Utils.basename(folder.uri))) { - const relativePattern = new RelativePattern(folder, '**/package.json'); - const paths = await workspace.findFiles(relativePattern, '**/{node_modules,.vscode-test}/**'); - for (const path of paths) { - if (!isExcluded(folder, path) && !visitedPackageJsonFiles.has(path.fsPath)) { - yield path; - visitedPackageJsonFiles.add(path.fsPath); - } + for (const folder of folders) { + if (isAutoDetectionEnabled(folder) && !excludeRegex.test(Utils.basename(folder.uri))) { + const relativePattern = new RelativePattern(folder, '**/package.json'); + const paths = await workspace.findFiles(relativePattern, '**/{node_modules,.vscode-test}/**'); + for (const path of paths) { + if (!isExcluded(folder, path) && !visitedPackageJsonFiles.has(path.fsPath)) { + yield path; + visitedPackageJsonFiles.add(path.fsPath); } } } - } catch (error) { - return Promise.reject(error); } } @@ -212,25 +204,21 @@ export async function detectNpmScriptsForFolder(context: ExtensionContext, folde const folderTasks: IFolderTaskItem[] = []; - try { - if (excludeRegex.test(Utils.basename(folder))) { - return folderTasks; - } - const relativePattern = new RelativePattern(folder.fsPath, '**/package.json'); - const paths = await workspace.findFiles(relativePattern, '**/node_modules/**'); - - const visitedPackageJsonFiles: Set = new Set(); - for (const path of paths) { - if (!visitedPackageJsonFiles.has(path.fsPath)) { - const tasks = await provideNpmScriptsForFolder(context, path, true); - visitedPackageJsonFiles.add(path.fsPath); - folderTasks.push(...tasks.map(t => ({ label: t.task.name, task: t.task }))); - } - } + if (excludeRegex.test(Utils.basename(folder))) { return folderTasks; - } catch (error) { - return Promise.reject(error); } + const relativePattern = new RelativePattern(folder.fsPath, '**/package.json'); + const paths = await workspace.findFiles(relativePattern, '**/node_modules/**'); + + const visitedPackageJsonFiles: Set = new Set(); + for (const path of paths) { + if (!visitedPackageJsonFiles.has(path.fsPath)) { + const tasks = await provideNpmScriptsForFolder(context, path, true); + visitedPackageJsonFiles.add(path.fsPath); + folderTasks.push(...tasks.map(t => ({ label: t.task.name, task: t.task }))); + } + } + return folderTasks; } export async function provideNpmScripts(context: ExtensionContext, showWarning: boolean): Promise { From 63411ed325157ffe59d21388f6c6ac80054ba88f Mon Sep 17 00:00:00 2001 From: Xu Yiming Date: Thu, 9 Jan 2025 18:48:19 +0800 Subject: [PATCH 08/10] fix: `node --run` doesn't support `--silent` --- extensions/npm/src/tasks.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/extensions/npm/src/tasks.ts b/extensions/npm/src/tasks.ts index a32e8cc2dc0fe..720b5e822e2cb 100644 --- a/extensions/npm/src/tasks.ts +++ b/extensions/npm/src/tasks.ts @@ -315,12 +315,17 @@ function getRelativePath(rootUri: Uri, packageJsonUri: Uri): string { export async function getRunScriptCommand(script: string, folder: Uri, context?: ExtensionContext, showWarning = true): Promise { const scriptRunner = await getScriptRunner(folder, context, showWarning); - const result = [scriptRunner, scriptRunner === 'node' ? '--run' : 'run']; - if (workspace.getConfiguration('npm', folder).get('runSilent')) { - result.push('--silent'); + + if (scriptRunner === 'node') { + return ['node', '--run', script]; + } else { + const result = [scriptRunner, 'run']; + if (workspace.getConfiguration('npm', folder).get('runSilent')) { + result.push('--silent'); + } + result.push(script); + return result; } - result.push(script); - return result; } export async function createScriptRunnerTask(context: ExtensionContext, script: string, folder: WorkspaceFolder, packageJsonUri: Uri, scriptValue?: string, showWarning?: boolean): Promise { From f29ff1406d07ed82d51ccbcb81976a78de2d6cd5 Mon Sep 17 00:00:00 2001 From: Xu Yiming Date: Thu, 9 Jan 2025 18:49:00 +0800 Subject: [PATCH 09/10] refactor: Use `.map` in `escapeCommandLine` --- extensions/npm/src/tasks.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/extensions/npm/src/tasks.ts b/extensions/npm/src/tasks.ts index 720b5e822e2cb..20046db8817ee 100644 --- a/extensions/npm/src/tasks.ts +++ b/extensions/npm/src/tasks.ts @@ -297,15 +297,13 @@ export function getTaskName(script: string, relativePath: string | undefined) { } function escapeCommandLine(cmd: string[]): (string | ShellQuotedString)[] { - const result: (string | ShellQuotedString)[] = new Array(cmd.length); - for (let i = 0; i < cmd.length; i++) { - if (/\s/.test(cmd[i])) { - result[i] = { value: cmd[i], quoting: cmd[i].includes('--') ? ShellQuoting.Weak : ShellQuoting.Strong }; + return cmd.map(arg => { + if (/\s/.test(arg)) { + return { value: arg, quoting: arg.includes('--') ? ShellQuoting.Weak : ShellQuoting.Strong }; } else { - result[i] = cmd[i]; + return arg; } - } - return result; + }); } function getRelativePath(rootUri: Uri, packageJsonUri: Uri): string { From 822e38da152af94d3b7472eab3c5eb91f83820f8 Mon Sep 17 00:00:00 2001 From: Xu Yiming Date: Fri, 7 Feb 2025 19:46:29 +0800 Subject: [PATCH 10/10] chore: Remove TODO Upstream reviewer is ok with current state --- extensions/npm/src/tasks.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/extensions/npm/src/tasks.ts b/extensions/npm/src/tasks.ts index 20046db8817ee..0c1d74e514a12 100644 --- a/extensions/npm/src/tasks.ts +++ b/extensions/npm/src/tasks.ts @@ -145,7 +145,6 @@ export async function detectPackageManager(folder: Uri, extensionContext?: Exten const { name, multipleLockFilesDetected: multiplePMDetected } = await findPreferredPM(folder.fsPath); const neverShowWarning = 'npm.multiplePMWarning.neverShow'; if (showWarning && multiplePMDetected && extensionContext && !extensionContext.globalState.get(neverShowWarning)) { - // todo: add text for npm.scriptRunner? const multiplePMWarning = l10n.t('Using {0} as the preferred package manager. Found multiple lockfiles for {1}. To resolve this issue, delete the lockfiles that don\'t match your preferred package manager or change the setting "npm.packageManager" to a value other than "auto".', name, folder.fsPath); const neverShowAgain = l10n.t("Do not show again"); const learnMore = l10n.t("Learn more");