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 4aa233141..53c7996ba 100644 --- a/documentation/docs/30-add-ons/99-community.md +++ b/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 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 9af512ae0..514da62e9 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 013f4cd5f..80a2b2749 100644 --- a/packages/sv/src/core/fetch-packages.ts +++ b/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, 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,17 +94,18 @@ export async function downloadPackage(options: DownloadOptions): Promise { +export async function importAddonCode( + pkgName: string, + pkgVersion: string, + exports?: PackageExports +): Promise { 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; + 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/) }) + ); + }); +}); 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.