diff --git a/.changeset/calm-envs-explain.md b/.changeset/calm-envs-explain.md new file mode 100644 index 000000000000..d313b76790c3 --- /dev/null +++ b/.changeset/calm-envs-explain.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: clarify circular imports from `src/env` diff --git a/packages/kit/src/core/env.js b/packages/kit/src/core/env.js index e76c1d5476d2..c76c36582fcf 100644 --- a/packages/kit/src/core/env.js +++ b/packages/kit/src/core/env.js @@ -5,6 +5,7 @@ import path from 'node:path'; import * as devalue from 'devalue'; import { dedent } from './sync/utils.js'; import { get_global_name, runtime_directory } from './utils.js'; +import { stackless } from '../utils/error.js'; import { resolve_entry } from '../utils/filesystem.js'; import { handle_issues, validate } from '../exports/internal/env.js'; import { get_config_aliases } from '../exports/vite/utils.js'; @@ -37,6 +38,8 @@ export function resolve_env_entry(config, root) { export async function load_explicit_env(kit, file, root, mode) { /** @type {Set} */ const deps = new Set(); + /** @type {Map} */ + const env_importers = new Map(); if (!file) { return { variables: null, deps }; @@ -64,6 +67,16 @@ export async function load_explicit_env(kit, file, root, mode) { plugins: [ { name: 'dependency-scanner', + enforce: 'pre', + resolveId(id, importer) { + const prefixes = ['$app/env/', `${runtime_directory}/app/env/`]; + const prefix = prefixes.find((prefix) => id.startsWith(prefix)); + const type = prefix && id.slice(prefix.length); + + if (importer && (type === 'private' || type === 'public')) { + env_importers.set(type, importer); + } + }, load(id) { deps.add(id); } @@ -96,14 +109,27 @@ export async function load_explicit_env(kit, file, root, mode) { } catch (e) { const error = /** @type {any} */ (e || {}); - if ( - error.code === 'ERR_MODULE_NOT_FOUND' && - error.message?.includes(`Cannot find module '$app`) - ) { - throw new Error( - `Cannot import \`$app/*\` modules other than \`$app/env\` inside \`src/env\``, - { cause: e } + if (error.code === 'ERR_MODULE_NOT_FOUND') { + const match = error.message?.match( + /\/env\/(private|public)\/server\.js/ ); + + if (match) { + const type = /** @type {EnvType} */ (match[1]); + const importer = env_importers.get(type); + const message = importer + ? `Module \`${posixify(path.relative(root, importer))}\` imports \`$app/env/${type}\`, which creates a circular dependency with \`src/env\`` + : `Cannot import \`$app/env/${type}\` inside \`src/env\` or its dependencies because it creates a circular dependency`; + + throw stackless(message); + } + + if (error.message?.includes(`Cannot find module '$app`)) { + throw new Error( + `Cannot import \`$app/*\` modules other than \`$app/env\` inside \`src/env\``, + { cause: e } + ); + } } throw error; diff --git a/packages/kit/src/core/sync/sync.spec.js b/packages/kit/src/core/sync/sync.spec.js index 74d70fb26583..c03468a63cb0 100644 --- a/packages/kit/src/core/sync/sync.spec.js +++ b/packages/kit/src/core/sync/sync.spec.js @@ -3,7 +3,9 @@ import os from 'node:os'; import path from 'node:path'; import { expect, test } from 'vitest'; import { process_config, validate_config } from '../config/index.js'; +import { load_explicit_env } from '../env.js'; import { relative_path } from '../../utils/filesystem.js'; +import { posixify } from '../../utils/os.js'; import { create, update } from './sync.js'; import create_manifest_data from './create_manifest_data/index.js'; @@ -36,6 +38,33 @@ test('generates client manifest imports relative to the project root', () => { } }); +test('explains circular imports through $app/env/private', async () => { + const root = path.resolve(import.meta.dirname, '../../../test/apps/basics'); + const dir = fs.mkdtempSync(path.join(root, 'node_modules/.svelte-kit-env-')); + const entry = path.join(dir, 'env.ts'); + + fs.writeFileSync( + entry, + `import { defineEnvVars } from '@sveltejs/kit/env'; +import './helper.js'; +export const variables = defineEnvVars({ FOO: {} }); +` + ); + const helper = path.join(dir, 'helper.ts'); + fs.writeFileSync(helper, `import '$app/env/private';\n`); + + try { + const config = process_config(validate_config({}), root); + + await expect(load_explicit_env(config, entry, root, 'development')).rejects.toMatchObject({ + message: `Module \`${posixify(path.relative(root, helper))}\` imports \`$app/env/private\`, which creates a circular dependency with \`src/env\``, + stack: '' + }); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + test('requests a manifest rebuild if static analysis encounters a missing route file', () => { const manifest_data = /** @type {import('types').ManifestData} */ ({ assets: [],