From 474b31836ca45f393a9ec1bf4820f95ef2d0a0c1 Mon Sep 17 00:00:00 2001 From: AdrianGonz97 <31664583+AdrianGonz97@users.noreply.github.com> Date: Sat, 29 Aug 2026 02:45:07 +0000 Subject: [PATCH 1/2] minor code cleanup --- packages/sv/src/cli/add.ts | 348 ++++++++++++------------- packages/sv/src/cli/create.ts | 9 +- packages/sv/src/core/common.ts | 20 +- packages/sv/src/core/config.ts | 37 ++- packages/sv/src/core/engine.ts | 2 +- packages/sv/src/core/fetch-packages.ts | 50 ++-- packages/sv/src/core/formatFiles.ts | 2 +- packages/sv/src/core/tests/setup.ts | 2 +- packages/sv/src/create/index.ts | 18 +- packages/sv/src/create/utils.ts | 6 +- 10 files changed, 264 insertions(+), 230 deletions(-) diff --git a/packages/sv/src/cli/add.ts b/packages/sv/src/cli/add.ts index 75df142b0..f76162ee9 100644 --- a/packages/sv/src/cli/add.ts +++ b/packages/sv/src/cli/add.ts @@ -16,6 +16,7 @@ import { type LoadedAddon, type OptionValues, type SetupResult, + createLoadedAddon, getErrorHint } from '../core/config.ts'; import { applyAddons, orderAddons, setupAddons } from '../core/engine.ts'; @@ -30,7 +31,6 @@ import { } from '../core/package-manager.ts'; import { verifyCleanWorkingDirectory, verifyUnsupportedAddons } from '../core/verifiers.ts'; import { createWorkspace, type Workspace } from '../core/workspace.ts'; -import { noDownloadCheckOption, noInstallOption } from './create.ts'; const officialAddons = Object.values(_officialAddons); const addonOptions = getAddonOptionFlags(); @@ -44,98 +44,6 @@ const OptionsSchema = v.strictObject({ }); type Options = v.InferOutput; -/** - * Classifies addon inputs into AddonReferences with source information. - */ -export function classifyAddons(inputs: AddonInput[], cwd: string): AddonReference[] { - const seen = new Map(); - const invalidAddons: string[] = []; - - for (const input of inputs) { - const official = officialAddons.find( - (a) => a.id === input.specifier || a.alias === input.specifier - ); - - if (official) { - const source: AddonSource = { kind: 'official', id: official.id }; - seen.set(official.id, { - specifier: input.specifier, - options: input.options, - source - }); - } else if (input.specifier.startsWith('file:')) { - const relativePath = input.specifier.slice(5).trim(); - if (!relativePath) { - invalidAddons.push('file:'); - continue; - } - const filePath = path.resolve(cwd, relativePath); - const source: AddonSource = { kind: 'file', path: filePath }; - seen.set(input.specifier, { - specifier: input.specifier, - options: input.options, - source - }); - } else { - // npm package - normalize and extract name/tag - const normalized = input.specifier.startsWith('@') - ? input.specifier.includes('/') - ? input.specifier - : input.specifier + '/sv' - : input.specifier; - - // Split name and tag: @scope/name@version or name@version - let packageName: string; - let tag: string; - if (normalized.startsWith('@')) { - // Scoped: @scope/name or @scope/name@version - const slashIndex = normalized.indexOf('/'); - const afterSlash = normalized.slice(slashIndex + 1); - const [name, version = 'latest'] = afterSlash.split('@'); - packageName = normalized.slice(0, slashIndex + 1) + name; - tag = version; - } else { - // Unscoped: name or name@version - const [name, version = 'latest'] = normalized.split('@'); - packageName = name; - tag = version; - } - - const npmUrl = `https://www.npmjs.com/package/${packageName}`; - const registryUrl = `https://registry.npmjs.org/${packageName}/${tag}`; - const source: AddonSource = { kind: 'npm', packageName, tag, npmUrl, registryUrl }; - seen.set(input.specifier, { - specifier: input.specifier, - options: input.options, - source - }); - } - } - - if (invalidAddons.length > 0) { - common.errorAndExit( - `Invalid add-ons specified: ${invalidAddons.map((id) => color.command(id)).join(', ')}\n` + - `${color.optional('Check the documentation for valid add-on specifiers:')} ${color.website('https://svelte.dev/docs/cli/sv-add')}` - ); - } - - return Array.from(seen.values()); -} - -/** - * Creates a LoadedAddon from an AddonDefinition (for official addons) - */ -export function createLoadedAddon(addon: AddonDefinition): LoadedAddon { - return { - reference: { - specifier: addon.id, - options: [], - source: { kind: 'official', id: addon.id } - }, - addon - }; -} - // infers the workspace cwd if a `package.json` resides in a parent directory const defaultPkgPath = pkg.up(); const defaultCwd = defaultPkgPath ? path.dirname(defaultPkgPath) : undefined; @@ -146,8 +54,8 @@ export const add = new Command('add') ) .option('-C, --cwd ', 'path to working directory', defaultCwd) .option('--no-git-check', 'even if some files are dirty, no prompt will be shown') - .addOption(noDownloadCheckOption) - .addOption(noInstallOption) + .addOption(common.cliOptions.noDownloadCheck) + .addOption(common.cliOptions.noInstall) .addOption(installOption) .configureHelp({ ...common.helpConfig, @@ -226,6 +134,84 @@ export const add = new Command('add') }); }); +/** + * Classifies addon inputs into AddonReferences with source information. + */ +export function classifyAddons(inputs: AddonInput[], cwd: string): AddonReference[] { + const seen = new Map(); + const invalidAddons: string[] = []; + + for (const input of inputs) { + const official = officialAddons.find( + (a) => a.id === input.specifier || a.alias === input.specifier + ); + + if (official) { + const source: AddonSource = { kind: 'official', id: official.id }; + seen.set(official.id, { + specifier: input.specifier, + options: input.options, + source + }); + } else if (input.specifier.startsWith('file:')) { + const relativePath = input.specifier.slice(5).trim(); + if (!relativePath) { + invalidAddons.push('file:'); + continue; + } + const filePath = path.resolve(cwd, relativePath); + const source: AddonSource = { kind: 'file', path: filePath }; + seen.set(input.specifier, { + specifier: input.specifier, + options: input.options, + source + }); + } else { + // npm package - normalize and extract name/tag + const normalized = input.specifier.startsWith('@') + ? input.specifier.includes('/') + ? input.specifier + : input.specifier + '/sv' + : input.specifier; + + // Split name and tag: @scope/name@version or name@version + let packageName: string; + let tag: string; + if (normalized.startsWith('@')) { + // Scoped: @scope/name or @scope/name@version + const slashIndex = normalized.indexOf('/'); + const afterSlash = normalized.slice(slashIndex + 1); + const [name, version = 'latest'] = afterSlash.split('@'); + packageName = normalized.slice(0, slashIndex + 1) + name; + tag = version; + } else { + // Unscoped: name or name@version + const [name, version = 'latest'] = normalized.split('@'); + packageName = name; + tag = version; + } + + const npmUrl = `https://www.npmjs.com/package/${packageName}`; + const registryUrl = `https://registry.npmjs.org/${packageName}/${tag}`; + const source: AddonSource = { kind: 'npm', packageName, tag, npmUrl, registryUrl }; + seen.set(input.specifier, { + specifier: input.specifier, + options: input.options, + source + }); + } + } + + if (invalidAddons.length > 0) { + common.errorAndExit( + `Invalid add-ons specified: ${invalidAddons.map((id) => color.command(id)).join(', ')}\n` + + `${color.optional('Check the documentation for valid add-on specifiers:')} ${color.website('https://svelte.dev/docs/cli/sv-add')}` + ); + } + + return Array.from(seen.values()); +} + /** * Resolves all addons (official and community). * Returns LoadedAddon[] with addon code loaded. @@ -269,6 +255,92 @@ export async function resolveAddons( return loaded; } +export async function resolveNonOfficialAddons( + refs: AddonReference[], + downloadCheck: boolean +): Promise { + const selectedAddons: AddonDefinition[] = []; + const { start, stop } = p.spinner(); + + try { + start(`Resolving ${refs.map((r) => color.addon(r.specifier)).join(', ')} packages`); + + const pkgs = await Promise.all( + refs.map(async (ref) => { + if (ref.source.kind === 'official') { + throw new Error(`Unexpected official addon in non-official resolver: ${ref.specifier}`); + } + return await getPackageJSON(ref); + }) + ); + stop('Resolved community add-on packages'); + + // Display version compatibility warnings + for (const { warning } of pkgs) { + if (warning) { + p.log.warn(warning); + } + } + + p.log.warn( + 'Svelte maintainers have not reviewed community add-ons for malicious code! Use at your discretion.' + ); + + const paddingName = common.getPadding(pkgs.map(({ pkg }) => pkg.name)); + const paddingVersion = common.getPadding(pkgs.map(({ pkg }) => `(v${pkg.version})`)); + + const packageInfos = pkgs.map(({ pkg, repo: _repo }) => { + const name = color.warning(pkg.name.padEnd(paddingName)); + const version = color.dim(`(v${pkg.version})`.padEnd(paddingVersion)); + const repo = color.dim(`(${_repo})`); + return `${name} ${version} ${repo}`; + }); + p.log.message(packageInfos.join('\n')); + + if (downloadCheck) { + const confirm = await p.confirm({ message: 'Would you like to continue?' }); + if (confirm !== true) { + p.cancel('Operation cancelled.'); + process.exit(1); + } + } + + start('Downloading community add-on packages'); + const downloadResults = await Promise.allSettled( + pkgs.map(async (opts) => downloadPackage(opts)) + ); + + // Separate successes and failures + const failures: Array<{ ref: AddonReference; error: string }> = []; + for (let i = 0; i < downloadResults.length; i++) { + const result = downloadResults[i]; + if (result.status === 'fulfilled') { + selectedAddons.push(result.value); + } else { + failures.push({ + ref: refs[i], + error: result.reason instanceof Error ? result.reason.message : 'Unknown error' + }); + } + } + + if (failures.length > 0) { + const failedList = failures.map((f) => color.addon(f.ref.specifier)).join(', '); + const hints = failures + .map((f) => `${f.ref.specifier}: ${getErrorHint(f.ref.source)}`) + .join('\n'); + const errorMsg = `Failed to resolve ${failedList}\n${color.optional(failures.map((f) => f.error).join('\n'))}\n\n${hints}`; + throw new Error(errorMsg); + } + stop('Downloaded community add-on packages'); + } catch (err) { + stop('Failed to download community add-on packages'); + const msg = err instanceof Error ? err.message : 'Unknown error'; + common.errorAndExit(msg); + } + return selectedAddons; +} + export async function promptAddonQuestions({ options, loadedAddons, @@ -954,89 +1026,3 @@ function getOptionChoices(details: AddonDefinition) { } return { choices, groups, groupDefaults }; } - -export async function resolveNonOfficialAddons( - refs: AddonReference[], - downloadCheck: boolean -): Promise { - const selectedAddons: AddonDefinition[] = []; - const { start, stop } = p.spinner(); - - try { - start(`Resolving ${refs.map((r) => color.addon(r.specifier)).join(', ')} packages`); - - const pkgs = await Promise.all( - refs.map(async (ref) => { - if (ref.source.kind === 'official') { - throw new Error(`Unexpected official addon in non-official resolver: ${ref.specifier}`); - } - return await getPackageJSON(ref); - }) - ); - stop('Resolved community add-on packages'); - - // Display version compatibility warnings - for (const { warning } of pkgs) { - if (warning) { - p.log.warn(warning); - } - } - - p.log.warn( - 'Svelte maintainers have not reviewed community add-ons for malicious code! Use at your discretion.' - ); - - const paddingName = common.getPadding(pkgs.map(({ pkg }) => pkg.name)); - const paddingVersion = common.getPadding(pkgs.map(({ pkg }) => `(v${pkg.version})`)); - - const packageInfos = pkgs.map(({ pkg, repo: _repo }) => { - const name = color.warning(pkg.name.padEnd(paddingName)); - const version = color.dim(`(v${pkg.version})`.padEnd(paddingVersion)); - const repo = color.dim(`(${_repo})`); - return `${name} ${version} ${repo}`; - }); - p.log.message(packageInfos.join('\n')); - - if (downloadCheck) { - const confirm = await p.confirm({ message: 'Would you like to continue?' }); - if (confirm !== true) { - p.cancel('Operation cancelled.'); - process.exit(1); - } - } - - start('Downloading community add-on packages'); - const downloadResults = await Promise.allSettled( - pkgs.map(async (opts) => downloadPackage(opts)) - ); - - // Separate successes and failures - const failures: Array<{ ref: AddonReference; error: string }> = []; - for (let i = 0; i < downloadResults.length; i++) { - const result = downloadResults[i]; - if (result.status === 'fulfilled') { - selectedAddons.push(result.value); - } else { - failures.push({ - ref: refs[i], - error: result.reason instanceof Error ? result.reason.message : 'Unknown error' - }); - } - } - - if (failures.length > 0) { - const failedList = failures.map((f) => color.addon(f.ref.specifier)).join(', '); - const hints = failures - .map((f) => `${f.ref.specifier}: ${getErrorHint(f.ref.source)}`) - .join('\n'); - const errorMsg = `Failed to resolve ${failedList}\n${color.optional(failures.map((f) => f.error).join('\n'))}\n\n${hints}`; - throw new Error(errorMsg); - } - stop('Downloaded community add-on packages'); - } catch (err) { - stop('Failed to download community add-on packages'); - const msg = err instanceof Error ? err.message : 'Unknown error'; - common.errorAndExit(msg); - } - return selectedAddons; -} diff --git a/packages/sv/src/cli/create.ts b/packages/sv/src/cli/create.ts index 8be4923c9..a53e97fb6 100644 --- a/packages/sv/src/cli/create.ts +++ b/packages/sv/src/cli/create.ts @@ -61,11 +61,6 @@ const addonNameOption = new Option( '--addon-name ', 'name for the addon package (e.g. @/ or )' ); -export const noDownloadCheckOption = new Option( - '--no-download-check', - 'skip all download confirmation prompts' -); -export const noInstallOption = new Option('--no-install', 'skip installing dependencies'); const ProjectPathSchema = v.optional(v.string()); const OptionsSchema = v.strictObject({ @@ -94,10 +89,10 @@ export const create = new Command('create') .addOption(noAddonsOption) .addOption(addOption) .addOption(addonNameOption) - .addOption(noInstallOption) + .addOption(common.cliOptions.noInstall) .option('--from-playground ', 'create a project from the svelte playground') .option('--no-dir-check', 'even if the folder is not empty, no prompt will be shown') - .addOption(noDownloadCheckOption) + .addOption(common.cliOptions.noDownloadCheck) .addOption(installOption) .configureHelp({ ...common.helpConfig, diff --git a/packages/sv/src/core/common.ts b/packages/sv/src/core/common.ts index 9af512ae0..b602c3207 100644 --- a/packages/sv/src/core/common.ts +++ b/packages/sv/src/core/common.ts @@ -8,11 +8,29 @@ import { type AgentName, resolveCommandArray } from '@sveltejs/sv-utils'; -import type { Argument, Command, Help, HelpConfiguration, Option } from 'commander'; +import { Option, type Argument, type Command, type Help, type HelpConfiguration } from 'commander'; +import * as v from 'valibot'; import pkg from '../../package.json' with { type: 'json' }; import type { LoadedAddon, Verification } from './config.ts'; import { UnsupportedError } from './errors.ts'; +const StringRecordSchema = v.record(v.string(), v.string()); +export const PackageJSONSchema = v.looseObject({ + name: v.string(), + version: v.string(), + peerDependencies: v.optional(StringRecordSchema), + dependencies: v.optional(StringRecordSchema), + devDependencies: v.optional(StringRecordSchema), + repository: v.optional(v.union([v.string(), v.looseObject({ url: v.optional(v.string()) })])), + dist: v.optional(v.looseObject({ tarball: v.optional(v.string()) })) +}); +export type PackageJSON = v.InferOutput; + +export const cliOptions = { + noDownloadCheck: new Option('--no-download-check', 'skip all download confirmation prompts'), + noInstall: new Option('--no-install', 'skip installing dependencies') +}; + // a file whose whole content is a single @import (e.g. CLAUDE.md -> @../AGENTS.md) const RX_IMPORT_ONLY = /^\s*@\S+\s*$/; diff --git a/packages/sv/src/core/config.ts b/packages/sv/src/core/config.ts index 738d476d6..d6002d265 100644 --- a/packages/sv/src/core/config.ts +++ b/packages/sv/src/core/config.ts @@ -129,12 +129,19 @@ export type SetupOptions> = { }; /** - * The entry point for your addon, It will hold every thing! (options, setup, run, nextSteps, ...) + * The entry point for your add-on. * - * For dynamic options added via `addOption` in setup, use the generic to get strong typing: + * ```ts + * const addon = defineAddon({ id: 'my-addon', options, run }); + * ``` + * + * If your add-on adds dynamic options via `addOption` during setup, pass their + * types as a type argument: * ```ts * const addon = defineAddon<{ extra: boolean }>()({ ... }); - * addon.options.extra.default // boolean + * // Take note of the extra call here: 👆 👆 + * // This works around Typescript's lack of partial type arguments + * addon.options.extra.default; // boolean * ``` */ export function defineAddon( @@ -148,11 +155,13 @@ export function defineAddon>(): < options: Args; } ) => Addon, Id, SetupValues>; -export function defineAddon(...args: any[]): any { - if (args.length === 0) { - return (config: any) => config; +export function defineAddon( + config?: AddonDefinition +): AddonDefinition | ((config: AddonDefinition) => AddonDefinition) { + if (config === undefined) { + return (c) => c; } - return args[0]; + return config; } // ============================================================================ @@ -268,6 +277,20 @@ export type SetupResult = { export type AddonDefinition = Addon>, Id>; +/** + * Creates a LoadedAddon from an AddonDefinition (for official addons) + */ +export function createLoadedAddon(addon: AddonDefinition): LoadedAddon { + return { + reference: { + specifier: addon.id, + options: [], + source: { kind: 'official', id: addon.id } + }, + addon + }; +} + type MaybePromise = Promise | T; export type Verification = { diff --git a/packages/sv/src/core/engine.ts b/packages/sv/src/core/engine.ts index a4f920ea4..34fd22f9b 100644 --- a/packages/sv/src/core/engine.ts +++ b/packages/sv/src/core/engine.ts @@ -14,9 +14,9 @@ import { minimizeDiff } from '@sveltejs/sv-utils'; import { exec } from 'tinyexec'; -import { createLoadedAddon } from '../cli/add.ts'; import { filePaths } from './common.ts'; import { + createLoadedAddon, getErrorHint, type Addon, type AddonDefinition, diff --git a/packages/sv/src/core/fetch-packages.ts b/packages/sv/src/core/fetch-packages.ts index 013f4cd5f..5f9e6edd6 100644 --- a/packages/sv/src/core/fetch-packages.ts +++ b/packages/sv/src/core/fetch-packages.ts @@ -6,14 +6,28 @@ import { fileURLToPath } from 'node:url'; import { createGunzip } from 'node:zlib'; import { color, coerceVersion, downloadJson, dedent } from '@sveltejs/sv-utils'; import { unpackTar } from 'modern-tar/fs'; +import * as v from 'valibot'; import pkg from '../../package.json' with { type: 'json' }; import * as common from './common.ts'; +import { PackageJSONSchema, type PackageJSON } from './common.ts'; import type { AddonDefinition, AddonReference } from './config.ts'; // path to the `node_modules` directory of `sv` const NODE_MODULES = fileURLToPath(new URL('../../node_modules', import.meta.url)); -function verifyPackage(addonPkg: Record, specifier: string): string | undefined { +type PackageBlocklist = { npm_names: string[] }; + +function parsePackageJSON(value: unknown, specifier: string): PackageJSON { + const result = v.safeParse(PackageJSONSchema, value); + if (!result.success) { + throw new Error( + `Invalid add-on package specified: '${specifier}' has invalid package metadata` + ); + } + return result.output; +} + +function verifyPackage(addonPkg: PackageJSON, specifier: string): string | undefined { const peerDeps = { ...addonPkg.peerDependencies }; const deps = { ...addonPkg.dependencies }; @@ -75,7 +89,7 @@ function copyDirectorySync(src: string, dest: string) { } } -type DownloadOptions = { path?: string; pkg: any }; +type DownloadOptions = { path?: string; pkg: PackageJSON }; /** * Downloads and installs the package into the `node_modules` of `sv`. * @returns the details of the downloaded addon @@ -102,10 +116,14 @@ export async function downloadPackage(options: DownloadOptions): Promise, name: st const files = getSharedFiles(); const pkg_file = path.join(cwd, filePaths.packageJson); - const pkg = /** @type {any} */ JSON.parse(fs.readFileSync(pkg_file, 'utf-8')); + const pkg: PackageJSON = JSON.parse(fs.readFileSync(pkg_file, 'utf-8')); sort_files(files).forEach((file) => { const include = file.include.every((condition) => matches_condition(condition, options)); @@ -97,7 +97,7 @@ function write_common_files(cwd: string, options: Omit, name: st if (exclude || !include) return; if (file.name === filePaths.packageJson) { - const new_pkg = JSON.parse(file.contents); + const new_pkg: PackageJSON = JSON.parse(file.contents); merge(pkg, new_pkg); } else { const dest = path.join(cwd, file.name); @@ -147,17 +147,15 @@ function merge(target: any, source: any) { } } -function sort_keys(obj: Record) { +function sort_keys(obj?: Record) { if (!obj) return; - const sorted: Record = {}; - Object.keys(obj) + return Object.keys(obj) .sort() - .forEach((key) => { + .reduce>((sorted, key) => { sorted[key] = obj[key]; - }); - - return sorted; + return sorted; + }, {}); } /** diff --git a/packages/sv/src/create/utils.ts b/packages/sv/src/create/utils.ts index b666d86dd..cd34448dc 100644 --- a/packages/sv/src/create/utils.ts +++ b/packages/sv/src/create/utils.ts @@ -1,15 +1,15 @@ import fs from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; +import { isNodeError } from '../core/common.ts'; import type { Common } from './index.ts'; export function mkdirp(dir: string): void { try { fs.mkdirSync(dir, { recursive: true }); } catch (err) { - const e: any = err; - if (e.code === 'EEXIST') return; - throw e; + if (isNodeError(err) && err.code === 'EEXIST') return; + throw err; } } From b6f1cbe66a602769abe9c522b161ea734f44d313 Mon Sep 17 00:00:00 2001 From: AdrianGonz97 <31664583+AdrianGonz97@users.noreply.github.com> Date: Sat, 29 Aug 2026 03:34:07 +0000 Subject: [PATCH 2/2] fix inconsistent tailwindcss test --- packages/sv/src/addons/tests/tailwindcss/test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/sv/src/addons/tests/tailwindcss/test.ts b/packages/sv/src/addons/tests/tailwindcss/test.ts index 2a44112ac..c9494852c 100644 --- a/packages/sv/src/addons/tests/tailwindcss/test.ts +++ b/packages/sv/src/addons/tests/tailwindcss/test.ts @@ -15,13 +15,14 @@ const { test, prepareServer, testCases } = setupTest( test.concurrent.for(testCases)( 'tailwindcss $kind.type $variant', - async (testCase, { page, ...ctx }) => { + async (testCase, { page, expect: vExpect, ...ctx }) => { const cwd = ctx.cwd(testCase); // ...add test files addFixture(cwd, testCase.variant); - const { close } = await prepareServer({ cwd, page }); + // we'll pass in vitest's `expect` instance to satisfy our `requireAssertion: true` requirement (it would otherwise _occasionally_ fail without it) + const { close } = await prepareServer({ cwd, page, expect: vExpect }); // kill server process when we're done ctx.onTestFinished(async () => await close());