From 9373450fd017482784cdcfd192e27bad7b3be377 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Charles?= Date: Mon, 17 Oct 2022 11:49:25 +0200 Subject: [PATCH 1/4] [feat] allow users to override IllegalModuleGuard default behaviour --- packages/kit/src/exports/vite/dev/index.js | 6 ++++-- .../src/exports/vite/graph_analysis/index.js | 20 ++++++++++++++----- .../exports/vite/graph_analysis/index.spec.js | 14 +++++++++++++ .../exports/vite/graph_analysis/types.d.ts | 4 ++++ packages/kit/src/exports/vite/index.js | 3 ++- packages/kit/types/index.d.ts | 1 + 6 files changed, 40 insertions(+), 8 deletions(-) diff --git a/packages/kit/src/exports/vite/dev/index.js b/packages/kit/src/exports/vite/dev/index.js index 489d78c9e4a6..8a2934010874 100644 --- a/packages/kit/src/exports/vite/dev/index.js +++ b/packages/kit/src/exports/vite/dev/index.js @@ -94,7 +94,8 @@ export async function dev(vite, vite_config, svelte_config) { prevent_illegal_vite_imports( module_node, normalizePath(svelte_config.kit.files.lib), - extensions + extensions, + { allow_server_import_from_client: svelte_config.kit.allowServerImportFromClient } ); return module.default; @@ -111,7 +112,8 @@ export async function dev(vite, vite_config, svelte_config) { prevent_illegal_vite_imports( module_node, normalizePath(svelte_config.kit.files.lib), - extensions + extensions, + { allow_server_import_from_client: svelte_config.kit.allowServerImportFromClient } ); } diff --git a/packages/kit/src/exports/vite/graph_analysis/index.js b/packages/kit/src/exports/vite/graph_analysis/index.js index 6c40503268c1..cd74d1805123 100644 --- a/packages/kit/src/exports/vite/graph_analysis/index.js +++ b/packages/kit/src/exports/vite/graph_analysis/index.js @@ -3,6 +3,7 @@ import { normalizePath } from 'vite'; import { remove_query_from_id, get_module_types } from './utils.js'; /** @typedef {import('./types').ImportGraph} ImportGraph */ +/** @typedef {import('./types').IllegalModuleGuardOptions} IllegalModuleGuardOptions */ const CWD_ID = normalizePath(process.cwd()); const NODE_MODULES_ID = normalizePath(path.resolve(process.cwd(), 'node_modules')); @@ -24,12 +25,18 @@ export class IllegalModuleGuard { /** @type {Array} */ #chain = []; + /** @type {(filepath: string) => boolean} */ + #allow_server_import_from_client; + /** * @param {string} lib_dir + * @param {IllegalModuleGuardOptions} [options] */ - constructor(lib_dir) { + constructor(lib_dir, options) { this.#lib_dir = normalizePath(lib_dir); this.#server_dir = normalizePath(path.resolve(lib_dir, 'server')); + this.#allow_server_import_from_client = + options?.allow_server_import_from_client ?? (() => false); } /** @@ -57,6 +64,7 @@ export class IllegalModuleGuard { * @returns {boolean} */ #is_illegal(module_id) { + if (this.#allow_server_import_from_client(module_id)) return false; if (this.#is_kit_illegal(module_id) || this.#is_user_illegal(module_id)) return true; return false; } @@ -255,11 +263,12 @@ export class ViteImportGraph { * @param {(id: string) => import('rollup').ModuleInfo | null} node_getter * @param {import('rollup').ModuleInfo} node * @param {string} lib_dir + * @param {IllegalModuleGuardOptions} [options] * @returns {void} */ -export function prevent_illegal_rollup_imports(node_getter, node, lib_dir) { +export function prevent_illegal_rollup_imports(node_getter, node, lib_dir, options) { const graph = new RollupImportGraph(node_getter, node); - const guard = new IllegalModuleGuard(lib_dir); + const guard = new IllegalModuleGuard(lib_dir, options); guard.assert_legal(graph); } @@ -268,10 +277,11 @@ export function prevent_illegal_rollup_imports(node_getter, node, lib_dir) { * @param {import('vite').ModuleNode} node * @param {string} lib_dir * @param {Iterable} module_types File extensions to analyze in addition to the defaults: `.ts`, `.js`, etc. + * @param {IllegalModuleGuardOptions} [options] * @returns {void} */ -export function prevent_illegal_vite_imports(node, lib_dir, module_types) { +export function prevent_illegal_vite_imports(node, lib_dir, module_types, options) { const graph = new ViteImportGraph(get_module_types(module_types), node); - const guard = new IllegalModuleGuard(lib_dir); + const guard = new IllegalModuleGuard(lib_dir, options); guard.assert_legal(graph); } diff --git a/packages/kit/src/exports/vite/graph_analysis/index.spec.js b/packages/kit/src/exports/vite/graph_analysis/index.spec.js index 1b00de5a804a..aaf4d37df9f9 100644 --- a/packages/kit/src/exports/vite/graph_analysis/index.spec.js +++ b/packages/kit/src/exports/vite/graph_analysis/index.spec.js @@ -181,6 +181,20 @@ describe('IllegalImportGuard', (test) => { assert.not.throws(() => guard.assert_legal(module_graph)); }); + + test('assert ignores illegal server-only modules when allowed by user config', () => { + const guard = new IllegalModuleGuard(FAKE_LIB_DIR, { + allow_server_import_from_client: (moduleId) => moduleId === USER_SERVER_FOLDER_ID + }); + + const module_graph = get_module_graph({ + id: USER_SERVER_FOLDER_ID, + dynamic: false, + children: generator_from_array([]) + }); + + assert.not.throws(() => guard.assert_legal(module_graph)); + }); }); /* diff --git a/packages/kit/src/exports/vite/graph_analysis/types.d.ts b/packages/kit/src/exports/vite/graph_analysis/types.d.ts index 1239fec437e1..fc8f37146c9c 100644 --- a/packages/kit/src/exports/vite/graph_analysis/types.d.ts +++ b/packages/kit/src/exports/vite/graph_analysis/types.d.ts @@ -3,3 +3,7 @@ export interface ImportGraph { readonly dynamic: boolean; readonly children: Generator; } + +export interface IllegalModuleGuardOptions { + readonly allow_server_import_from_client?: (filepath: string) => boolean; +} diff --git a/packages/kit/src/exports/vite/index.js b/packages/kit/src/exports/vite/index.js index dd9d4f0e5c45..f1fdc1e78a11 100644 --- a/packages/kit/src/exports/vite/index.js +++ b/packages/kit/src/exports/vite/index.js @@ -358,7 +358,8 @@ function kit() { prevent_illegal_rollup_imports( this.getModuleInfo.bind(this), module_node, - vite.normalizePath(svelte_config.kit.files.lib) + vite.normalizePath(svelte_config.kit.files.lib), + { allow_server_import_from_client: svelte_config.kit.allowServerImportFromClient } ); } }); diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index f36e2105bd0d..ed31dc2232b9 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -232,6 +232,7 @@ export interface KitConfig { name?: string; pollInterval?: number; }; + allowServerImportFromClient?: (filepath: string) => boolean; } export interface Handle { From 9cddd54ca130aa3a72c5f85160a8565326c74f4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Charles?= Date: Mon, 17 Oct 2022 11:55:42 +0200 Subject: [PATCH 2/4] changeset --- .changeset/fair-pumas-admire.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/fair-pumas-admire.md diff --git a/.changeset/fair-pumas-admire.md b/.changeset/fair-pumas-admire.md new file mode 100644 index 000000000000..39d138dbe91e --- /dev/null +++ b/.changeset/fair-pumas-admire.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +[feat] allow users to override IllegalModuleGuard default behaviour From 8efe498e1099282ece769e5696f9a7c2247c49fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Charles?= Date: Mon, 17 Oct 2022 12:20:09 +0200 Subject: [PATCH 3/4] should not verify nested files --- .../src/exports/vite/graph_analysis/index.js | 17 +++++++++----- .../exports/vite/graph_analysis/index.spec.js | 23 ++++++++++++++++--- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/packages/kit/src/exports/vite/graph_analysis/index.js b/packages/kit/src/exports/vite/graph_analysis/index.js index cd74d1805123..23fedb2f1a3d 100644 --- a/packages/kit/src/exports/vite/graph_analysis/index.js +++ b/packages/kit/src/exports/vite/graph_analysis/index.js @@ -47,13 +47,18 @@ export class IllegalModuleGuard { assert_legal(node) { this.#chain.push(node); for (const child of node.children) { - if (this.#is_illegal(child.id)) { - this.#chain.push(child); - const error = this.#format_illegal_import_chain(this.#chain); - this.#chain = []; // Reset the chain in case we want to reuse this guard - throw new Error(error); + if (this.#allow_server_import_from_client(child.id)) { + // Do not assert child nodes if module is allowed by user config + continue; + } else { + if (this.#is_illegal(child.id)) { + this.#chain.push(child); + const error = this.#format_illegal_import_chain(this.#chain); + this.#chain = []; // Reset the chain in case we want to reuse this guard + throw new Error(error); + } + this.assert_legal(child); } - this.assert_legal(child); } this.#chain.pop(); } diff --git a/packages/kit/src/exports/vite/graph_analysis/index.spec.js b/packages/kit/src/exports/vite/graph_analysis/index.spec.js index aaf4d37df9f9..f546edc961ab 100644 --- a/packages/kit/src/exports/vite/graph_analysis/index.spec.js +++ b/packages/kit/src/exports/vite/graph_analysis/index.spec.js @@ -184,13 +184,30 @@ describe('IllegalImportGuard', (test) => { test('assert ignores illegal server-only modules when allowed by user config', () => { const guard = new IllegalModuleGuard(FAKE_LIB_DIR, { - allow_server_import_from_client: (moduleId) => moduleId === USER_SERVER_FOLDER_ID + allow_server_import_from_client: (moduleId) => moduleId.includes('.allowed.') }); const module_graph = get_module_graph({ - id: USER_SERVER_FOLDER_ID, + id: 'entry.svelte', dynamic: false, - children: generator_from_array([]) + children: generator_from_array([ + { + id: 'something.allowed.js', + dynamic: false, + children: generator_from_array([ + { + id: PROD_VIRTUAL_STATIC_ID, + dynamic: false, + children: generator_from_array([]) + }, + { + id: USER_SERVER_FOLDER_ID, + dynamic: false, + children: generator_from_array([]) + } + ]) + } + ]) }); assert.not.throws(() => guard.assert_legal(module_graph)); From 688c69fe0417539d825354ff8312a60fefebe67f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20Charles?= Date: Mon, 17 Oct 2022 12:38:02 +0200 Subject: [PATCH 4/4] remove unnecessary check --- packages/kit/src/exports/vite/graph_analysis/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/kit/src/exports/vite/graph_analysis/index.js b/packages/kit/src/exports/vite/graph_analysis/index.js index 23fedb2f1a3d..7c896061c170 100644 --- a/packages/kit/src/exports/vite/graph_analysis/index.js +++ b/packages/kit/src/exports/vite/graph_analysis/index.js @@ -69,7 +69,6 @@ export class IllegalModuleGuard { * @returns {boolean} */ #is_illegal(module_id) { - if (this.#allow_server_import_from_client(module_id)) return false; if (this.#is_kit_illegal(module_id) || this.#is_user_illegal(module_id)) return true; return false; }