From 7733fe2d0a02a2aefc68548b182817b2545b6240 Mon Sep 17 00:00:00 2001 From: AdrianGonz97 <31664583+AdrianGonz97@users.noreply.github.com> Date: Fri, 7 Aug 2026 20:14:15 +0000 Subject: [PATCH 1/2] branch off of `version-1` instead --- .changeset/tired-comics-strive.md | 5 ++ documentation/docs/30-add-ons/99-community.md | 4 +- .../snapshots/@my-org/sv/CONTRIBUTING.md | 2 +- packages/sv/src/core/common.ts | 9 ++- packages/sv/src/core/fetch-packages.ts | 75 ++++++++++++------- .../src/create/shared/+addon/CONTRIBUTING.md | 2 +- 6 files changed, 65 insertions(+), 32 deletions(-) create mode 100644 .changeset/tired-comics-strive.md diff --git a/.changeset/tired-comics-strive.md b/.changeset/tired-comics-strive.md new file mode 100644 index 000000000..c1d65a32c --- /dev/null +++ b/.changeset/tired-comics-strive.md @@ -0,0 +1,5 @@ +--- +'sv': patch +--- + +fix(addon): relax dependency fields restriction on community add-ons diff --git a/documentation/docs/30-add-ons/99-community.md b/documentation/docs/30-add-ons/99-community.md index 9af218bd5..35bef18b6 100644 --- a/documentation/docs/30-add-ons/99-community.md +++ b/documentation/docs/30-add-ons/99-community.md @@ -150,7 +150,7 @@ Community add-ons are bundled with [tsdown](https://tsdown.dev/) into a single f ### `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 +164,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 diff --git a/packages/sv/src/cli/tests/snapshots/@my-org/sv/CONTRIBUTING.md b/packages/sv/src/cli/tests/snapshots/@my-org/sv/CONTRIBUTING.md index d5da26c73..dd520ac7c 100644 --- a/packages/sv/src/cli/tests/snapshots/@my-org/sv/CONTRIBUTING.md +++ b/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. diff --git a/packages/sv/src/core/common.ts b/packages/sv/src/core/common.ts index 37bbbb41f..d6c414ebb 100644 --- a/packages/sv/src/core/common.ts +++ b/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); diff --git a/packages/sv/src/core/fetch-packages.ts b/packages/sv/src/core/fetch-packages.ts index e00db1d52..1681e1112 100644 --- a/packages/sv/src/core/fetch-packages.ts +++ b/packages/sv/src/core/fetch-packages.ts @@ -15,7 +15,6 @@ const NODE_MODULES = fileURLToPath(new URL('../../node_modules', import.meta.url function verifyPackage(addonPkg: Record, 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, 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,7 +94,8 @@ export async function downloadPackage(options: DownloadOptions): Promise { +async function importAddonCode( + pkgName: string, + pkgVersion: string, + exports?: Record +): Promise { const issues: string[] = []; - let details: AddonDefinition | undefined; - try { - ({ default: details } = await import(`${pkgName}/sv`)); - } catch { - issues.push(`'/sv' export not found`); + const error = () => { + return new Error( + `Failed to load add-on '${pkgName}@${pkgVersion}':\n- ${issues.join('\n- ')}\n\n` + + `Please report this to the add-on author.` + ); + }; + + if (!exports) { + issues.push(`'exports' field not found in package.json`); + throw error(); } - if (!details) { + const svImport = exports['./sv'] ? `${pkgName}/sv` : undefined; + const defaultImport = exports['.'] ? pkgName : undefined; + if (!svImport && !defaultImport) { + issues.push(`export conditions './sv' or '.' are not present in package.json`); + throw error(); + } + + let details: AddonDefinition | undefined; + + for (const importPath of [svImport, defaultImport]) { + if (!importPath) continue; try { - ({ default: details } = await import(pkgName)); - } catch { - issues.push(`default export not found`); + details ??= await import(importPath).then((m) => m.default); + } catch (e) { + if (isNodeError(e)) { + if (e.code === 'ERR_MODULE_NOT_FOUND') { + issues.push('the add-on contains dependencies that are not bundled'); + throw error(); + } + issues.push(`Failed to import add-on '${importPath}': ${e.message}`); + } else { + issues.push(`An unknown error has occurred: ${e}`); + } } } - if (!details && issues.length > 0) { - throw new Error( - `Failed to load add-on '${pkgName}@${pkgVersion}':\n- ${issues.join('\n- ')}\n\n` + - `Please report this to the add-on author.` - ); + if (!details) { + throw error(); } - return details!; + return details; +} + +function isNodeError(err: unknown): err is Error & NodeJS.ErrnoException { + return err instanceof Error; } type PackageJSON = { diff --git a/packages/sv/src/create/shared/+addon/CONTRIBUTING.md b/packages/sv/src/create/shared/+addon/CONTRIBUTING.md index d5da26c73..dd520ac7c 100644 --- a/packages/sv/src/create/shared/+addon/CONTRIBUTING.md +++ b/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. From 66464e16d0f430ac74841d3b43b7eb24c470521d Mon Sep 17 00:00:00 2001 From: "jyc.dev" Date: Fri, 28 Aug 2026 20:40:34 +0200 Subject: [PATCH 2/2] fix(addon): harden add-on entry point resolution (#1278) * fix(addon): harden add-on entry point resolution Try `./sv` then `.` as documented, instead of bailing on the first failure. Stop rejecting valid `exports` shapes (bare string, conditions only, absent) before attempting an import, handle the CJS `MODULE_NOT_FOUND` code, and keep Node's message so the missing module is named. Document that `sv` provides `@sveltejs/sv-utils` and that leaving it unbundled couples the add-on to whatever version `sv` ships. * Update packages/sv/src/core/common.ts Co-authored-by: CokaKoala <31664583+AdrianGonz97@users.noreply.github.com> --------- Co-authored-by: CokaKoala <31664583+AdrianGonz97@users.noreply.github.com> --- documentation/docs/30-add-ons/99-community.md | 2 + packages/sv/src/core/fetch-packages.ts | 77 ++++----- packages/sv/src/core/tests/fetch-packages.ts | 156 ++++++++++++++++++ 3 files changed, 194 insertions(+), 41 deletions(-) create mode 100644 packages/sv/src/core/tests/fetch-packages.ts diff --git a/documentation/docs/30-add-ons/99-community.md b/documentation/docs/30-add-ons/99-community.md index b6c8cff63..53c7996ba 100644 --- a/documentation/docs/30-add-ons/99-community.md +++ b/documentation/docs/30-add-ons/99-community.md @@ -148,6 +148,8 @@ 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. Any `dependencies` declared will **not** be available at runtime, everything must be bundled: diff --git a/packages/sv/src/core/fetch-packages.ts b/packages/sv/src/core/fetch-packages.ts index 3027274c5..80a2b2749 100644 --- a/packages/sv/src/core/fetch-packages.ts +++ b/packages/sv/src/core/fetch-packages.ts @@ -95,10 +95,10 @@ export async function downloadPackage(options: DownloadOptions): Promise + exports?: PackageExports ): Promise { const issues: string[] = []; + let unresolvedModule = false; - const error = () => { - return new Error( - `Failed to load add-on '${pkgName}@${pkgVersion}':\n- ${issues.join('\n- ')}\n\n` + - `Please report this to the add-on author.` - ); - }; - - if (!exports) { - issues.push(`'exports' field not found in package.json`); - throw error(); - } - - const svImport = exports['./sv'] ? `${pkgName}/sv` : undefined; - const defaultImport = exports['.'] ? pkgName : undefined; - if (!svImport && !defaultImport) { - issues.push(`export conditions './sv' or '.' are not present in package.json`); - throw error(); - } - - let details: AddonDefinition | undefined; + // 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]; - for (const importPath of [svImport, defaultImport]) { - if (!importPath) continue; + for (const specifier of candidates) { try { - details ??= await import(importPath).then((m) => m.default); + const details: AddonDefinition | undefined = (await import(specifier)).default; + if (details) return details; + + issues.push(`'${specifier}' resolved but has no default export`); } catch (e) { - if (isNodeError(e)) { - if (e.code === 'ERR_MODULE_NOT_FOUND') { - issues.push('the add-on contains dependencies that are not bundled'); - throw error(); - } - issues.push(`Failed to import add-on '${importPath}': ${e.message}`); - } else { - issues.push(`An unknown error has occurred: ${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) { - throw error(); - } + const hint = unresolvedModule + ? `\nThis usually means the add-on has dependencies that are not bundled.\n` + : ''; - return details; + 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.` + ); } -function isNodeError(err: unknown): err is Error & NodeJS.ErrnoException { - return err instanceof Error; +/** `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; + type PackageJSON = { name: string; version: string; diff --git a/packages/sv/src/core/tests/fetch-packages.ts b/packages/sv/src/core/tests/fetch-packages.ts new file mode 100644 index 000000000..80ee49e21 --- /dev/null +++ b/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 }; + +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 = { 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/) }) + ); + }); +});