Skip to content

Commit ee2f55b

Browse files
svelte-triage-bot[bot]svelte-triage-bot
andauthored
fix: identify circular imports from src/env (#17014)
Fixes #17013. When `src/env` imports a dependency that imports `$app/env/private` or `$app/env/public`, loading the environment configuration depends on modules that have not yet been generated. Recognize this resolution failure and report the circular dependency directly instead of exposing the internal generated-module path. When available, the diagnostic now identifies the project-relative module that imported `$app/env/private` or `$app/env/public`. A regression test covers the transitive helper-import pattern and verifies that the responsible helper module is reported. ## Testing - `pnpm vitest run -c vitest.kit.config.js src/core/sync/sync.spec.js --reporter=verbose` - `pnpm -F @sveltejs/kit test:unit` - `pnpm format` - `pnpm -F @sveltejs/kit lint` - `pnpm -F @sveltejs/kit prepublishOnly` --------- Co-authored-by: svelte-triage-bot <team@svelte.com>
1 parent c024802 commit ee2f55b

3 files changed

Lines changed: 67 additions & 7 deletions

File tree

.changeset/calm-envs-explain.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@sveltejs/kit': patch
3+
---
4+
5+
fix: clarify circular imports from `src/env`

packages/kit/src/core/env.js

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import path from 'node:path';
55
import * as devalue from 'devalue';
66
import { dedent } from './sync/utils.js';
77
import { get_global_name, runtime_directory } from './utils.js';
8+
import { stackless } from '../utils/error.js';
89
import { resolve_entry } from '../utils/filesystem.js';
910
import { handle_issues, validate } from '../exports/internal/env.js';
1011
import { get_config_aliases } from '../exports/vite/utils.js';
@@ -37,6 +38,8 @@ export function resolve_env_entry(config, root) {
3738
export async function load_explicit_env(kit, file, root, mode) {
3839
/** @type {Set<string>} */
3940
const deps = new Set();
41+
/** @type {Map<EnvType, string>} */
42+
const env_importers = new Map();
4043

4144
if (!file) {
4245
return { variables: null, deps };
@@ -64,6 +67,16 @@ export async function load_explicit_env(kit, file, root, mode) {
6467
plugins: [
6568
{
6669
name: 'dependency-scanner',
70+
enforce: 'pre',
71+
resolveId(id, importer) {
72+
const prefixes = ['$app/env/', `${runtime_directory}/app/env/`];
73+
const prefix = prefixes.find((prefix) => id.startsWith(prefix));
74+
const type = prefix && id.slice(prefix.length);
75+
76+
if (importer && (type === 'private' || type === 'public')) {
77+
env_importers.set(type, importer);
78+
}
79+
},
6780
load(id) {
6881
deps.add(id);
6982
}
@@ -96,14 +109,27 @@ export async function load_explicit_env(kit, file, root, mode) {
96109
} catch (e) {
97110
const error = /** @type {any} */ (e || {});
98111

99-
if (
100-
error.code === 'ERR_MODULE_NOT_FOUND' &&
101-
error.message?.includes(`Cannot find module '$app`)
102-
) {
103-
throw new Error(
104-
`Cannot import \`$app/*\` modules other than \`$app/env\` inside \`src/env\``,
105-
{ cause: e }
112+
if (error.code === 'ERR_MODULE_NOT_FOUND') {
113+
const match = error.message?.match(
114+
/<sveltekit:generated>\/env\/(private|public)\/server\.js/
106115
);
116+
117+
if (match) {
118+
const type = /** @type {EnvType} */ (match[1]);
119+
const importer = env_importers.get(type);
120+
const message = importer
121+
? `Module \`${posixify(path.relative(root, importer))}\` imports \`$app/env/${type}\`, which creates a circular dependency with \`src/env\``
122+
: `Cannot import \`$app/env/${type}\` inside \`src/env\` or its dependencies because it creates a circular dependency`;
123+
124+
throw stackless(message);
125+
}
126+
127+
if (error.message?.includes(`Cannot find module '$app`)) {
128+
throw new Error(
129+
`Cannot import \`$app/*\` modules other than \`$app/env\` inside \`src/env\``,
130+
{ cause: e }
131+
);
132+
}
107133
}
108134

109135
throw error;

packages/kit/src/core/sync/sync.spec.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ import os from 'node:os';
33
import path from 'node:path';
44
import { expect, test } from 'vitest';
55
import { process_config, validate_config } from '../config/index.js';
6+
import { load_explicit_env } from '../env.js';
67
import { relative_path } from '../../utils/filesystem.js';
8+
import { posixify } from '../../utils/os.js';
79
import { create, update } from './sync.js';
810
import create_manifest_data from './create_manifest_data/index.js';
911

@@ -36,6 +38,33 @@ test('generates client manifest imports relative to the project root', () => {
3638
}
3739
});
3840

41+
test('explains circular imports through $app/env/private', async () => {
42+
const root = path.resolve(import.meta.dirname, '../../../test/apps/basics');
43+
const dir = fs.mkdtempSync(path.join(root, 'node_modules/.svelte-kit-env-'));
44+
const entry = path.join(dir, 'env.ts');
45+
46+
fs.writeFileSync(
47+
entry,
48+
`import { defineEnvVars } from '@sveltejs/kit/env';
49+
import './helper.js';
50+
export const variables = defineEnvVars({ FOO: {} });
51+
`
52+
);
53+
const helper = path.join(dir, 'helper.ts');
54+
fs.writeFileSync(helper, `import '$app/env/private';\n`);
55+
56+
try {
57+
const config = process_config(validate_config({}), root);
58+
59+
await expect(load_explicit_env(config, entry, root, 'development')).rejects.toMatchObject({
60+
message: `Module \`${posixify(path.relative(root, helper))}\` imports \`$app/env/private\`, which creates a circular dependency with \`src/env\``,
61+
stack: ''
62+
});
63+
} finally {
64+
fs.rmSync(dir, { recursive: true, force: true });
65+
}
66+
});
67+
3968
test('requests a manifest rebuild if static analysis encounters a missing route file', () => {
4069
const manifest_data = /** @type {import('types').ManifestData} */ ({
4170
assets: [],

0 commit comments

Comments
 (0)