feat: skip package manager prompt when nothing needs installing by jycouet · Pull Request #1195 · sveltejs/cli

jycouet · 2026-07-25T17:05:03Z

sv add now only asks which package manager to use when an add-on actually adds or widens a dependency in package.json (e.g. sv add mcp=ide:claude-code+setup:remote needs nothing installed). Files are still formatted when the install is skipped for that reason.

Adds isRangeWithin to @sveltejs/sv-utils, so an existing stricter range (^9.2.0 vs a requested ^9.0.0) is kept and doesn't trigger an install.

pkg-svelte-dev · 2026-07-25T17:05:06Z

Install the latest version of sv from 57085d3:

pnpm add https://pkg.svelte.dev/sv/c/57085d36e0863c0b5711cc05237e2355f1285307

Open in pkg.svelte.dev: https://pkg.svelte.dev/repos/cli/pr/1195

changeset-bot · 2026-07-25T17:05:07Z

🦋 Changeset detected

Latest commit: 57085d3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@sveltejs/sv-utils	Patch
sv	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

svelte-docs-bot · 2026-07-25T17:11:40Z

Preview: https://svelte-dev-git-preview-cli-1195-svelte.vercel.app/

sacrosanctic · 2026-07-25T23:16:20Z

Darn, I was working on the issue too, but yours look better.

sacrosanctic · 2026-07-25T23:32:55Z

+	let packageManager: AgentName | null | undefined = null;
+	if (options.install !== false && installNeeded) {
+		packageManager =
+			options.install === true ? await packageManagerPrompt(options.cwd) : options.install;
+	}


Suggested change

let packageManager: AgentName | null | undefined = null;

if (options.install !== false && installNeeded) {

packageManager =

options.install === true ? await packageManagerPrompt(options.cwd) : options.install;

}

let packageManager: AgentName | null | undefined = null;

if (options.install === true) {

// prompt user when `package.json` has changed

if (installNeeded) {

packageManager = await packageManagerPrompt(options.cwd);

}

} else if (options.install === false) {

// user choose not to install

} else {

packageManager = options.install;

}

sacrosanctic · 2026-07-25T23:34:45Z

+	// still format when the install was skipped only because no new dependency was added
+	if (packageManager || (!installNeeded && options.install !== false)) {


really hard to understand this condition

AdrianGonz97 · 2026-07-31T00:13:00Z

+/**
+ * Returns `true` when every version matching `range` also matches `target`,
+ * e.g. `^9.2.0` is within `^9.0.0` but `^8.0.0` is not.
+ * Unparseable inputs (`latest`, `workspace:*`, ...) return `false`.
+ */
+export function isRangeWithin(range: string, target: string): boolean {
+	try {
+		return isRangeSubset(range, target);
+	} catch {
+		return false;
+	}
+}


the param names are a bit too ambiguous. i think sticking with isRangeSubset's naming makes the most sense.

Suggested change

/**

* Returns `true` when every version matching `range` also matches `target`,

* e.g. `^9.2.0` is within `^9.0.0` but `^8.0.0` is not.

* Unparseable inputs (`latest`, `workspace:*`, ...) return `false`.

*/

export function isRangeWithin(range: string, target: string): boolean {

try {

return isRangeSubset(range, target);

} catch {

return false;

}

}

/**

* Returns `true` when every version matching `subset` also matches `superset`,

* e.g. `^9.2.0` is within `^9.0.0` but `^8.0.0` is not.

* Unparseable inputs (`latest`, `workspace:*`, ...) return `false`.

*/

export function isRangeWithin(subset: string, superset: string): boolean {

try {

return isRangeSubset(subset, superset);

} catch {

return false;

}

}

AdrianGonz97 · 2026-07-31T00:13:38Z

 export function isVersionUnsupportedBelow(
 	versionStr: string,
 	belowStr: string


drive-by change

Suggested change

export function isVersionUnsupportedBelow(

versionStr: string,

belowStr: string

export function isVersionUnsupportedBelow(

version: string,

below: string

AdrianGonz97 · 2026-07-31T00:15:40Z

+describe('isRangeWithin', () => {
+	const combinations = [
+		{ range: '^9.0.0', target: '^9.0.0', expected: true },
+		{ range: '^9.2.0', target: '^9.0.0', expected: true },
+		{ range: '9.2.0', target: '^9.0.0', expected: true },
+		{ range: '^9.0.0', target: '^9.2.0', expected: false },
+		{ range: '^8.0.0', target: '^9.0.0', expected: false },
+		{ range: '*', target: '^9.0.0', expected: false },
+		{ range: 'latest', target: '^9.0.0', expected: false },
+		{ range: 'workspace:^9.0.0', target: '^9.0.0', expected: false }
+	] as const;
+	it.each(combinations)(
+		'($range within $target) should be $expected',
+		({ range, target, expected }) => {
+			expect(isRangeWithin(range, target)).toEqual(expected);
+		}
+	);
+});


...continues the optional name change

Suggested change

describe('isRangeWithin', () => {

const combinations = [

{ range: '^9.0.0', target: '^9.0.0', expected: true },

{ range: '^9.2.0', target: '^9.0.0', expected: true },

{ range: '9.2.0', target: '^9.0.0', expected: true },

{ range: '^9.0.0', target: '^9.2.0', expected: false },

{ range: '^8.0.0', target: '^9.0.0', expected: false },

{ range: '*', target: '^9.0.0', expected: false },

{ range: 'latest', target: '^9.0.0', expected: false },

{ range: 'workspace:^9.0.0', target: '^9.0.0', expected: false }

] as const;

it.each(combinations)(

'($range within $target) should be $expected',

({ range, target, expected }) => {

expect(isRangeWithin(range, target)).toEqual(expected);

}

);

});

describe('isRangeWithin', () => {

const combinations = [

{ subset: '^9.0.0', superset: '^9.0.0', expected: true },

{ subset: '^9.2.0', superset: '^9.0.0', expected: true },

{ subset: '9.2.0', superset: '^9.0.0', expected: true },

{ subset: '^9.0.0', superset: '^9.2.0', expected: false },

{ subset: '^8.0.0', superset: '^9.0.0', expected: false },

{ subset: '*', superset: '^9.0.0', expected: false },

{ subset: 'latest', superset: '^9.0.0', expected: false },

{ subset: 'workspace:^9.0.0', superset: '^9.0.0', expected: false }

] as const;

it.each(combinations)(

'($subset within $superset) should be $expected',

({ subset, superset, expected }) => {

expect(isRangeWithin(subset, superset)).toEqual(expected);

}

);

});

AdrianGonz97 · 2026-07-31T00:35:54Z

+/** Returns `true` when `package.json` gained or widened a dependency, i.e. an install is required. */
 function updatePackages(
 	dependencies: Array<{ pkg: string; version: string; dev: boolean }>,
 	sv: SvApi
-) {
-	if (dependencies.length === 0) return;
+): boolean {
+	if (dependencies.length === 0) return false;

+	let installNeeded = false;
 	const pkgPath = filePaths.packageJson;


this is very much a nitpick, but returning an object should remove the need for the comment here

Suggested change

/** Returns `true` when `package.json` gained or widened a dependency, i.e. an install is required. */

function updatePackages(

dependencies: Array<{ pkg: string; version: string; dev: boolean }>,

sv: SvApi

) {

if (dependencies.length === 0) return;

): boolean {

if (dependencies.length === 0) return false;

let installNeeded = false;

const pkgPath = filePaths.packageJson;

/** Returns `true` when `package.json` gained or widened a dependency, i.e. an install is required. */

function updatePackages(

dependencies: Array<{ pkg: string; version: string; dev: boolean }>,

sv: SvApi

): { installNeeded: boolean } {

let installNeeded = false;

if (dependencies.length === 0) return { installNeeded };

const pkgPath = filePaths.packageJson;

AdrianGonz97 · 2026-07-31T00:36:19Z

 		})
 	);
+
+	return installNeeded;


a follow-up from the previous suggestion

Suggested change

return installNeeded;

return { installNeeded };

AdrianGonz97 · 2026-07-31T00:36:40Z

 		sv,
 		finalize: () => {
-			updatePackages(dependencies, sv);
+			const installNeeded = updatePackages(dependencies, sv);


another follow-up

Suggested change

const installNeeded = updatePackages(dependencies, sv);

const { installNeeded } = updatePackages(dependencies, sv);

AdrianGonz97 · 2026-07-31T00:53:33Z

 	if (packageManager) {
 		workspace.packageManager = packageManager;
 		await installDependencies(packageManager, options.cwd);
+	}
+
+	// still format when the install was skipped only because no new dependency was added
+	if (packageManager || (!installNeeded && options.install !== false)) {
 		await formatFiles({
-			packageManager,
+			packageManager: packageManager ?? workspace.packageManager,
 			cwd: options.cwd,
 			filesToFormat,
 			strategy: 'files-only'


im not exactly sure when this changed, but iirc, we used to guard formatting files behind whether we ran the deps install ourselves because we couldn't guarantee that the formatter was installed locally. if it wasn't, then it would result in an error when attempting to format.

I think that seems to be the case now. for example:

create a new project

install prettier and eslint with their respective add-ons

delete node_modules

try running the eslint add-on

feat: skip package manager prompt when nothing needs installing

a883eb7

jycouet force-pushed the feat/install-when-needed branch from e072bab to a883eb7 Compare July 25, 2026 17:07

jycouet added 2 commits July 25, 2026 20:26

fix(test): restore SV_VERSION_REGEX clobbered during rebase

17bceef

fix: narrow packageManager type in add flow

57085d3

sacrosanctic reviewed Jul 25, 2026

View reviewed changes

AdrianGonz97 reviewed Jul 31, 2026

View reviewed changes

-	let packageManager: AgentName | null | undefined = null;
-	if (options.install !== false && installNeeded) {
-		packageManager =
-			options.install === true ? await packageManagerPrompt(options.cwd) : options.install;
-	}
+	let packageManager: AgentName | null | undefined = null;
+	if (options.install === true) {
+		// prompt user when `package.json` has changed
+		if (installNeeded) {
+			packageManager = await packageManagerPrompt(options.cwd);
+		}
+	} else if (options.install === false) {
+		// user choose not to install
+	} else {
+		packageManager = options.install;
+	}

		// still format when the install was skipped only because no new dependency was added
		if (packageManager \|\| (!installNeeded && options.install !== false)) {

	const installNeeded = updatePackages(dependencies, sv);
	const { installNeeded } = updatePackages(dependencies, sv);

Uh oh!

Uh oh!

Conversation

jycouet commented Jul 25, 2026

Uh oh!

pkg-svelte-dev Bot commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot Bot commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

svelte-docs-bot Bot commented Jul 25, 2026

Uh oh!

sacrosanctic commented Jul 25, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pkg-svelte-dev Bot commented Jul 25, 2026 •

edited

Loading

changeset-bot Bot commented Jul 25, 2026 •

edited

Loading