fix: relax community add-on requirements (alternate)

.changeset/tired-comics-strive.md

@@ -0,0 +1,5 @@
+---
+'sv': patch
+---
+
+fix(addon): relax dependency fields restriction on community add-ons

documentation/docs/30-add-ons/99-community.md

@@ -148,9 +148,11 @@ export default setupGlobal({ TEST_DIR });
 
 Community add-ons are bundled with [tsdown](https://tsdown.dev/) into a single file. Everything is bundled except `sv`. (It is a peer dependency provided at runtime.)
 
+`sv` ships its own copy of [`@sveltejs/sv-utils`](sv-utils), so an add-on that leaves it unbundled will still load. Nothing verifies the version: your add-on runs against whatever `sv` provides, and following its breaking changes is up to you. Bundle it to stay on a version you control.
+
 ### `package.json`
 
-Your add-on must have `sv` as a peer dependency and **no** `dependencies` in `package.json`:
+Your add-on must have `sv` as a peer dependency. Any `dependencies` declared will **not** be available at runtime, everything must be bundled:
 
 ```jsonc
 {
@@ -164,7 +166,7 @@ Your add-on must have `sv` as a peer dependency and **no** `dependencies` in `pa
 	"publishConfig": {
 		"access": "public"
 	},
-	// cannot have dependencies
+	// packages declared here will not be available during runtime, it must be bundled
 	"dependencies": {},
 	"peerDependencies": {
 		// minimum version required to run by this add-on

packages/sv/src/cli/tests/snapshots/@my-org/sv/CONTRIBUTING.md

@@ -45,4 +45,4 @@ npm publish
 
 ## Things to be aware of
 
-Community add-ons must have `sv` as a `peerDependency` and should **not** have any `dependencies`. Everything else (including `@sveltejs/sv-utils`) is bundled at build time by tsdown.
+Community add-ons must have `sv` as a `peerDependency`. Any `dependencies` declared in `package.json` will not be available at runtime. Everything else (including `@sveltejs/sv-utils`) is bundled at build time by tsdown.

packages/sv/src/core/common.ts

@@ -275,7 +275,14 @@ export function updateReadme(projectPath: string, command: string) {
 }
 
 export function errorAndExit(message: string) {
-	p.log.error(message);
+	const [firstLine, ...restLines] = message.split('\n');
+
+	p.log.error(firstLine);
+	// Fixes issue where the first line of the error message is not the same color as the rest of the lines
+	for (const line of restLines) {
+		p.log.message(color.optional(line), { spacing: 0 });
+	}
+
 	p.log.message();
 	p.cancel('Operation failed.');
 	process.exit(1);

packages/sv/src/core/fetch-packages.ts

@@ -4,7 +4,7 @@ import path from 'node:path';
 import { pipeline } from 'node:stream/promises';
 import { fileURLToPath } from 'node:url';
 import { createGunzip } from 'node:zlib';
-import { color, coerceVersion, downloadJson, dedent } from '@sveltejs/sv-utils';
+import { color, coerceVersion, downloadJson } from '@sveltejs/sv-utils';
 import { unpackTar } from 'modern-tar/fs';
 import pkg from '../../package.json' with { type: 'json' };
 import * as common from './common.ts';
@@ -15,7 +15,6 @@ const NODE_MODULES = fileURLToPath(new URL('../../node_modules', import.meta.url
 
 function verifyPackage(addonPkg: Record<string, any>, specifier: string): string | undefined {
 	const peerDeps = { ...addonPkg.peerDependencies };
-	const deps = { ...addonPkg.dependencies };
 
 	// valid addons should always have `sv` as a peerDependency
 	const addonSvVersion = peerDeps['sv'];
@@ -25,13 +24,6 @@ function verifyPackage(addonPkg: Record<string, any>, specifier: string): string
 		);
 	}
 
-	// addons should not have any dependencies (everything should be bundled)
-	if (Object.keys(deps).length > 0) {
-		throw new Error(
-			`Invalid add-on package detected: '${specifier}'\nCommunity add-ons should not have any 'dependencies'. Use 'peerDependencies' for 'sv' and bundle everything else`
-		);
-	}
-
 	// Check version compatibility and warn if there's a major version mismatch
 	const addon = coerceVersion(addonSvVersion);
 	const sv_major = coerceVersion(pkg.version).major;
@@ -102,17 +94,18 @@ export async function downloadPackage(options: DownloadOptions): Promise<AddonDe
 		// Try to create a symlink, but fall back to copying on Windows if it fails with EPERM
 		try {
 			fs.symlinkSync(options.path, dest, 'dir');
-		} catch (error: any) {
+		} catch (error) {
+			const code = errorCode(error);
 			// On Windows, symlinks may fail with EPERM if admin privileges aren't available
 			// In that case, fall back to copying the directory
-			if (platform() === 'win32' && (error.code === 'EPERM' || error.code === 'EACCES')) {
+			if (platform() === 'win32' && (code === 'EPERM' || code === 'EACCES')) {
 				copyDirectorySync(options.path, dest);
 			} else {
 				throw error;
 			}
 		}
 
-		return await importAddonCode(pkg.name, pkg.version);
+		return await importAddonCode(pkg.name, pkg.version, pkg.exports);
 	}
 
 	const tarballUrl: string = pkg.dist.tarball;
@@ -130,41 +123,60 @@ export async function downloadPackage(options: DownloadOptions): Promise<AddonDe
 		unpackTar(path.join(NODE_MODULES, pkg.name), { strip: 1 })
 	);
 
-	return await importAddonCode(pkg.name, pkg.version);
+	return await importAddonCode(pkg.name, pkg.version, pkg.exports);
 }
 
-async function importAddonCode(pkgName: string, pkgVersion: string): Promise<AddonDefinition> {
+export async function importAddonCode(
+	pkgName: string,
+	pkgVersion: string,
+	exports?: PackageExports
+): Promise<AddonDefinition> {
 	const issues: string[] = [];
+	let unresolvedModule = false;
 
-	let details: AddonDefinition | undefined;
-	try {
-		({ default: details } = await import(`${pkgName}/sv`));
-	} catch {
-		issues.push(`'/sv' export not found`);
-	}
+	// only probe `/sv` when the package actually maps it, otherwise the probe itself
+	// fails and reports a missing module that the author never declared
+	const candidates = hasSvExport(exports) ? [`${pkgName}/sv`, pkgName] : [pkgName];
 
-	if (!details) {
+	for (const specifier of candidates) {
 		try {
-			({ default: details } = await import(pkgName));
-		} catch {
-			issues.push(`default export not found`);
+			const details: AddonDefinition | undefined = (await import(specifier)).default;
+			if (details) return details;
+
+			issues.push(`'${specifier}' resolved but has no default export`);
+		} catch (e) {
+			const code = errorCode(e);
+			// ESM entry points report `ERR_MODULE_NOT_FOUND`, CJS ones report `MODULE_NOT_FOUND`
+			if (code === 'ERR_MODULE_NOT_FOUND' || code === 'MODULE_NOT_FOUND') {
+				unresolvedModule = true;
+			}
+			issues.push(`'${specifier}' failed to load: ${e instanceof Error ? e.message : e}`);
 		}
 	}
 
-	if (!details && issues.length > 0) {
-		throw new Error(
-			dedent`
-				Failed to load add-on '${pkgName}@${pkgVersion}':
-				${issues.map((i) => `- ${i}`).join('\n')}
+	const hint = unresolvedModule
+		? `\nThis usually means the add-on has dependencies that are not bundled.\n`
+		: '';
 
-				Please report this to the add-on author.
-				`
-		);
-	}
+	throw new Error(
+		`Failed to load add-on '${pkgName}@${pkgVersion}':\n- ${issues.join('\n- ')}\n${hint}\n` +
+			`Please report this to the add-on author.`
+	);
+}
 
-	return details!;
+/** `exports` is only consulted to pick entry points, never to reject a package. */
+export function hasSvExport(exports?: PackageExports): boolean {
+	if (typeof exports !== 'object' || exports === null || Array.isArray(exports)) return false;
+	return Boolean(exports['./sv']);
 }
 
+function errorCode(err: unknown): string | undefined {
+	return err instanceof Error ? (err as NodeJS.ErrnoException).code : undefined;
+}
+
+/** Values are nested condition objects, and the field itself may be a string or an array. */
+type PackageExports = string | string[] | Record<string, unknown>;
+
 type PackageJSON = {
 	name: string;
 	version: string;

packages/sv/src/core/tests/fetch-packages.ts

@@ -0,0 +1,156 @@
+import fs from 'node:fs';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { afterAll, describe, expect, it } from 'vitest';
+import { hasSvExport, importAddonCode } from '../fetch-packages.ts';
+
+// add-ons are imported by bare specifier, so fixtures have to live where `sv` resolves from
+const NODE_MODULES = fileURLToPath(new URL('../../../node_modules', import.meta.url));
+const PREFIX = 'sv-fixture-addon-';
+
+type Fixture = { exports?: unknown; main?: string; files: Record<string, string> };
+
+let counter = 0;
+function writeFixture(fixture: Fixture): string {
+	const name = `${PREFIX}${counter++}`;
+	const dir = path.join(NODE_MODULES, name);
+
+	for (const [file, contents] of Object.entries(fixture.files)) {
+		const filePath = path.join(dir, file);
+		fs.mkdirSync(path.dirname(filePath), { recursive: true });
+		fs.writeFileSync(filePath, contents);
+	}
+
+	const pkg: Record<string, unknown> = { name, version: '1.0.0', type: 'module' };
+	if (fixture.exports !== undefined) pkg.exports = fixture.exports;
+	if (fixture.main !== undefined) pkg.main = fixture.main;
+	fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify(pkg));
+
+	return name;
+}
+
+afterAll(() => {
+	for (const entry of fs.readdirSync(NODE_MODULES)) {
+		if (entry.startsWith(PREFIX)) {
+			fs.rmSync(path.join(NODE_MODULES, entry), { recursive: true, force: true });
+		}
+	}
+});
+
+const ADDON = `export default { id: 'fixture', shortDescription: 'x', homepage: '', options: {}, run: () => {} }`;
+
+describe('hasSvExport', () => {
+	it('detects a mapped ./sv entry', () => {
+		expect(hasSvExport({ '.': './a.mjs', './sv': './sv.mjs' })).toBe(true);
+	});
+	it('ignores packages without a ./sv entry', () => {
+		expect(hasSvExport({ '.': './a.mjs' })).toBe(false);
+	});
+	it('does not throw on the string form', () => {
+		expect(hasSvExport('./dist/index.mjs')).toBe(false);
+	});
+	it('does not throw on the array form', () => {
+		expect(hasSvExport(['./dist/index.mjs'])).toBe(false);
+	});
+	it('does not throw when exports is absent', () => {
+		expect(hasSvExport(undefined)).toBe(false);
+	});
+});
+
+describe('importAddonCode', () => {
+	it('loads a package whose exports field is a bare string', async () => {
+		const name = writeFixture({
+			exports: './dist/index.mjs',
+			files: { 'dist/index.mjs': ADDON }
+		});
+		await expect(importAddonCode(name, '1.0.0', './dist/index.mjs')).resolves.toMatchObject({
+			id: 'fixture'
+		});
+	});
+
+	it('loads a package that only declares main', async () => {
+		const name = writeFixture({ main: './dist/index.mjs', files: { 'dist/index.mjs': ADDON } });
+		await expect(importAddonCode(name, '1.0.0', undefined)).resolves.toMatchObject({
+			id: 'fixture'
+		});
+	});
+
+	it('loads a package whose exports only declares conditions', async () => {
+		const exports = { import: './dist/index.mjs', default: './dist/index.mjs' };
+		const name = writeFixture({ exports, files: { 'dist/index.mjs': ADDON } });
+		await expect(importAddonCode(name, '1.0.0', exports)).resolves.toMatchObject({
+			id: 'fixture'
+		});
+	});
+
+	it('prefers ./sv over the default entry', async () => {
+		const exports = { '.': './dist/main.mjs', './sv': './dist/sv.mjs' };
+		const name = writeFixture({
+			exports,
+			files: {
+				'dist/main.mjs': `export default { id: 'main' }`,
+				'dist/sv.mjs': `export default { id: 'sv' }`
+			}
+		});
+		await expect(importAddonCode(name, '1.0.0', exports)).resolves.toMatchObject({ id: 'sv' });
+	});
+
+	it('falls back to the default entry when ./sv is missing from the tarball', async () => {
+		const exports = { '.': './dist/main.mjs', './sv': './dist/gone.mjs' };
+		const name = writeFixture({ exports, files: { 'dist/main.mjs': ADDON } });
+		await expect(importAddonCode(name, '1.0.0', exports)).resolves.toMatchObject({
+			id: 'fixture'
+		});
+	});
+
+	it('reports unbundled dependencies for an ESM entry', async () => {
+		const exports = { '.': './dist/index.mjs' };
+		const name = writeFixture({
+			exports,
+			files: { 'dist/index.mjs': `import 'sv-fixture-absent-dep';\n${ADDON}` }
+		});
+		await expect(importAddonCode(name, '1.0.0', exports)).rejects.toThrow(
+			/dependencies that are not bundled/
+		);
+	});
+
+	it('reports unbundled dependencies for a CJS entry', async () => {
+		const exports = { '.': './dist/index.cjs' };
+		const name = writeFixture({
+			exports,
+			files: { 'dist/index.cjs': `require('sv-fixture-absent-dep');\nmodule.exports = {}` }
+		});
+		await expect(importAddonCode(name, '1.0.0', exports)).rejects.toThrow(
+			/dependencies that are not bundled/
+		);
+	});
+
+	it('names the missing module rather than swallowing it', async () => {
+		const exports = { '.': './dist/index.mjs' };
+		const name = writeFixture({
+			exports,
+			files: { 'dist/index.mjs': `import 'sv-fixture-absent-dep';\n${ADDON}` }
+		});
+		await expect(importAddonCode(name, '1.0.0', exports)).rejects.toThrow(/sv-fixture-absent-dep/);
+	});
+
+	it('says so when the entry has no default export', async () => {
+		const exports = { '.': './dist/index.mjs' };
+		const name = writeFixture({
+			exports,
+			files: { 'dist/index.mjs': `export const addon = {}` }
+		});
+		await expect(importAddonCode(name, '1.0.0', exports)).rejects.toThrow(/no default export/);
+	});
+
+	it('never renders an empty bullet', async () => {
+		const exports = { '.': './dist/index.mjs' };
+		const name = writeFixture({
+			exports,
+			files: { 'dist/index.mjs': `export const addon = {}` }
+		});
+		await expect(importAddonCode(name, '1.0.0', exports)).rejects.toThrow(
+			expect.objectContaining({ message: expect.not.stringMatching(/-\s*\n/) })
+		);
+	});
+});

packages/sv/src/create/shared/+addon/CONTRIBUTING.md

@@ -45,4 +45,4 @@ npm publish
 
 ## Things to be aware of
 
-Community add-ons must have `sv` as a `peerDependency` and should **not** have any `dependencies`. Everything else (including `@sveltejs/sv-utils`) is bundled at build time by tsdown.
+Community add-ons must have `sv` as a `peerDependency`. Any `dependencies` declared in `package.json` will not be available at runtime. Everything else (including `@sveltejs/sv-utils`) is bundled at build time by tsdown.