Skip to content

Commit

Permalink
fix: strip virtual module prefix from error messages (#10776)
Browse files Browse the repository at this point in the history
  • Loading branch information
benmccann authored Sep 26, 2023
1 parent b4c76c8 commit 34f1ec5
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 34 deletions.
5 changes: 5 additions & 0 deletions .changeset/gorgeous-shrimps-bow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: strip virtual module prefix from error messages
16 changes: 12 additions & 4 deletions packages/kit/src/exports/vite/graph_analysis/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import path from 'node:path';
import { posixify } from '../../../utils/filesystem.js';
import { strip_virtual_prefix } from '../utils.js';

const ILLEGAL_IMPORTS = new Set(['\0$env/dynamic/private', '\0$env/static/private']);
const ILLEGAL_IMPORTS = new Set([
'\0virtual:$env/dynamic/private',
'\0virtual:$env/static/private'
]);
const ILLEGAL_MODULE_NAME_PATTERN = /.*\.server\..+/;

/**
Expand Down Expand Up @@ -51,10 +55,14 @@ export function module_guard(context, { cwd, lib }) {
chain.map(({ id, dynamic }, i) => {
id = normalize_id(id, lib, cwd);

return `${' '.repeat(i * 2)}- ${id} ${dynamic ? 'dynamically imports' : 'imports'}\n`;
}) + `${' '.repeat(chain.length)}- ${id}`;
return `${' '.repeat(i * 2)}- ${strip_virtual_prefix(id)} ${
dynamic ? 'dynamically imports' : 'imports'
}\n`;
}) + `${' '.repeat(chain.length)}- ${strip_virtual_prefix(id)}`;

const message = `Cannot import ${id} into client-side code:\n${pyramid}`;
const message = `Cannot import ${strip_virtual_prefix(
id
)} into client-side code:\n${pyramid}`;

throw new Error(message);
}
Expand Down
24 changes: 12 additions & 12 deletions packages/kit/src/exports/vite/graph_analysis/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@ test('throws an error when importing $env/static/private', () => {
importedIds: ['~/src/routes/+page.svelte']
},
'~/src/routes/+page.svelte': {
importedIds: ['\0$env/static/private']
importedIds: ['\0virtual:$env/static/private']
}
},
`Cannot import \0$env/static/private into client-side code:
`Cannot import $env/static/private into client-side code:
- src/routes/+page.svelte imports
- \0$env/static/private`
- $env/static/private`
);
});

Expand All @@ -60,12 +60,12 @@ test('throws an error when dynamically importing $env/static/private', () => {
importedIds: ['~/src/routes/+page.svelte']
},
'~/src/routes/+page.svelte': {
dynamicallyImportedIds: ['\0$env/static/private']
dynamicallyImportedIds: ['\0virtual:$env/static/private']
}
},
`Cannot import \0$env/static/private into client-side code:
`Cannot import $env/static/private into client-side code:
- src/routes/+page.svelte dynamically imports
- \0$env/static/private`
- $env/static/private`
);
});

Expand All @@ -76,12 +76,12 @@ test('throws an error when importing $env/dynamic/private', () => {
importedIds: ['~/src/routes/+page.svelte']
},
'~/src/routes/+page.svelte': {
importedIds: ['\0$env/dynamic/private']
importedIds: ['\0virtual:$env/dynamic/private']
}
},
`Cannot import \0$env/dynamic/private into client-side code:
`Cannot import $env/dynamic/private into client-side code:
- src/routes/+page.svelte imports
- \0$env/dynamic/private`
- $env/dynamic/private`
);
});

Expand All @@ -92,12 +92,12 @@ test('throws an error when dynamically importing $env/dynamic/private', () => {
importedIds: ['~/src/routes/+page.svelte']
},
'~/src/routes/+page.svelte': {
dynamicallyImportedIds: ['\0$env/dynamic/private']
dynamicallyImportedIds: ['\0virtual:$env/dynamic/private']
}
},
`Cannot import \0$env/dynamic/private into client-side code:
`Cannot import $env/dynamic/private into client-side code:
- src/routes/+page.svelte dynamically imports
- \0$env/dynamic/private`
- $env/dynamic/private`
);
});

Expand Down
20 changes: 10 additions & 10 deletions packages/kit/src/exports/vite/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { assets_base, find_deps } from './build/utils.js';
import { dev } from './dev/index.js';
import { is_illegal, module_guard, normalize_id } from './graph_analysis/index.js';
import { preview } from './preview/index.js';
import { get_config_aliases, get_env } from './utils.js';
import { get_config_aliases, get_env, strip_virtual_prefix } from './utils.js';
import { write_client_manifest } from '../../core/sync/write_client_manifest.js';
import prerender from '../../core/postbuild/prerender.js';
import analyse from '../../core/postbuild/analyse.js';
Expand Down Expand Up @@ -336,7 +336,7 @@ function kit({ svelte_config }) {
async resolveId(id) {
// treat $env/static/[public|private] as virtual
if (id.startsWith('$env/') || id.startsWith('__sveltekit/') || id === '$service-worker') {
return `\0${id}`;
return `\0virtual:${id}`;
}
},

Expand All @@ -358,24 +358,24 @@ function kit({ svelte_config }) {
})
) {
const relative = normalize_id(id, normalized_lib, normalized_cwd);
throw new Error(`Cannot import ${relative} into client-side code`);
throw new Error(`Cannot import ${strip_virtual_prefix(relative)} into client-side code`);
}
}

switch (id) {
case '\0$env/static/private':
case '\0virtual:$env/static/private':
return create_static_module('$env/static/private', env.private);

case '\0$env/static/public':
case '\0virtual:$env/static/public':
return create_static_module('$env/static/public', env.public);

case '\0$env/dynamic/private':
case '\0virtual:$env/dynamic/private':
return create_dynamic_module(
'private',
vite_config_env.command === 'serve' ? env.private : undefined
);

case '\0$env/dynamic/public':
case '\0virtual:$env/dynamic/public':
// populate `$env/dynamic/public` from `window`
if (browser) {
return `export const env = ${global}.env;`;
Expand All @@ -386,12 +386,12 @@ function kit({ svelte_config }) {
vite_config_env.command === 'serve' ? env.public : undefined
);

case '\0$service-worker':
case '\0virtual:$service-worker':
return create_service_worker_module(svelte_config);

// for internal use only. it's published as $app/paths externally
// we use this alias so that we won't collide with user aliases
case '\0__sveltekit/paths': {
case '\0virtual:__sveltekit/paths': {
const { assets, base } = svelte_config.kit.paths;

// use the values defined in `global`, but fall back to hard-coded values
Expand Down Expand Up @@ -429,7 +429,7 @@ function kit({ svelte_config }) {
`;
}

case '\0__sveltekit/environment': {
case '\0virtual:__sveltekit/environment': {
const { version } = svelte_config.kit;

return dedent`
Expand Down
2 changes: 2 additions & 0 deletions packages/kit/src/exports/vite/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,5 @@ export function not_found(req, res, base) {
);
}
}

export const strip_virtual_prefix = /** @param {string} id */ (id) => id.replace('\0virtual:', '');
8 changes: 4 additions & 4 deletions packages/kit/test/apps/dev-only/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ test.describe.serial('Illegal imports', () => {
wait_for_started: false
});
expect(await page.textContent('.message-body')).toBe(
'Cannot import \0$env/dynamic/private into client-side code'
'Cannot import $env/dynamic/private into client-side code'
);
});

Expand All @@ -20,7 +20,7 @@ test.describe.serial('Illegal imports', () => {
wait_for_started: false
});
expect(await page.textContent('.message-body')).toBe(
'Cannot import \0$env/dynamic/private into client-side code'
'Cannot import $env/dynamic/private into client-side code'
);
});

Expand All @@ -29,7 +29,7 @@ test.describe.serial('Illegal imports', () => {
wait_for_started: false
});
expect(await page.textContent('.message-body')).toBe(
'Cannot import \0$env/static/private into client-side code'
'Cannot import $env/static/private into client-side code'
);
});

Expand All @@ -38,7 +38,7 @@ test.describe.serial('Illegal imports', () => {
wait_for_started: false
});
expect(await page.textContent('.message-body')).toBe(
'Cannot import \0$env/static/private into client-side code'
'Cannot import $env/static/private into client-side code'
);
});

Expand Down
8 changes: 4 additions & 4 deletions packages/kit/test/build-errors/env.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ test('$env/dynamic/private is not statically importable from the client', () =>
stdio: 'pipe',
timeout: 60000
}),
/.*Cannot import \0\$env\/dynamic\/private into client-side code:.*/gs
/.*Cannot import \$env\/dynamic\/private into client-side code:.*/gs
);
});

Expand All @@ -22,7 +22,7 @@ test('$env/dynamic/private is not dynamically importable from the client', () =>
stdio: 'pipe',
timeout: 60000
}),
/.*Cannot import \0\$env\/dynamic\/private into client-side code:.*/gs
/.*Cannot import \$env\/dynamic\/private into client-side code:.*/gs
);
});

Expand All @@ -34,7 +34,7 @@ test('$env/static/private is not statically importable from the client', () => {
stdio: 'pipe',
timeout: 60000
}),
/.*Cannot import \0\$env\/static\/private into client-side code:.*/gs
/.*Cannot import \$env\/static\/private into client-side code:.*/gs
);
});

Expand All @@ -46,6 +46,6 @@ test('$env/static/private is not dynamically importable from the client', () =>
stdio: 'pipe',
timeout: 60000
}),
/.*Cannot import \0\$env\/static\/private into client-side code:.*/gs
/.*Cannot import \$env\/static\/private into client-side code:.*/gs
);
});

0 comments on commit 34f1ec5

Please sign in to comment.