From 63ea6a309a95e25b3fa5c08d6cda0e69d8d3d3a2 Mon Sep 17 00:00:00 2001 From: dominikg Date: Wed, 18 Aug 2021 16:25:54 +0200 Subject: [PATCH 01/10] wip: deep search for svelte dependencies --- .../optimizedeps-include/vite.config.js | 3 - .../src/utils/dependencies.ts | 124 ++++++++++++++++++ .../vite-plugin-svelte/src/utils/options.ts | 9 ++ 3 files changed, 133 insertions(+), 3 deletions(-) create mode 100644 packages/vite-plugin-svelte/src/utils/dependencies.ts diff --git a/packages/playground/optimizedeps-include/vite.config.js b/packages/playground/optimizedeps-include/vite.config.js index c26e633a5..75a5d5281 100644 --- a/packages/playground/optimizedeps-include/vite.config.js +++ b/packages/playground/optimizedeps-include/vite.config.js @@ -12,9 +12,6 @@ const SVELTE_IMPORTS = [ export default defineConfig(({ command, mode }) => { const isProduction = mode === 'production'; return { - optimizeDeps: { - include: [...SVELTE_IMPORTS] - }, plugins: [svelte()], build: { minify: isProduction diff --git a/packages/vite-plugin-svelte/src/utils/dependencies.ts b/packages/vite-plugin-svelte/src/utils/dependencies.ts new file mode 100644 index 000000000..424d8b7c6 --- /dev/null +++ b/packages/vite-plugin-svelte/src/utils/dependencies.ts @@ -0,0 +1,124 @@ +import { log } from './log'; +import path from 'path'; +import fs from 'fs'; +import { createRequire } from 'module'; + +export function findSvelteDependencies( + root: string, + cwdFallback = true +): Record { + log.debug(`findSvelteDependencies: searching svelte dependencies in ${root}`); + + const pkgFile = path.join(root, 'package.json'); + if (!fs.existsSync(pkgFile)) { + if (cwdFallback) { + const cwd = process.cwd(); + if (root !== cwd) { + log.debug(`no package.json found in vite root ${root}`); + return findSvelteDependencies(cwd, false); + } + } + log.debug(`no package.json found, search failed`); + return {}; + } + + const stack = [{ dir: root, depPath: [] as string[] }]; + // name->SvelteDependency[] + const result: Record = {}; + const doNotScan = new Set([ + 'svelte', + 'vite', + '@sveltejs/kit', + '@sveltejs/vite-plugin-svelte' + ]); + + while (stack.length > 0) { + const { dir, depPath } = stack.shift()!; + const pkg = parsePkg(dir); + if (!pkg) { + continue; + } + if (dir !== root) { + if (!isSvelte(pkg)) { + doNotScan.add(pkg.name); + continue; + } + if (!result[pkg.name]) { + result[pkg.name] = []; + } + result[pkg.name].push({ name: pkg.name, pkg, dir, paths: [depPath] }); + } + + const pkgRequire = createRequire(dir); + const deps = [ + ...Object.keys(pkg.dependencies || {}), + ...Object.keys(pkg.devDependencies || {}) + ]; + const resolvedDeps = deps.map((dep) => ({ name: dep, dir: getDependencyDir(pkgRequire, dep) })); + const nestedDepPath = [...depPath, pkg.name]; + for (const dep of resolvedDeps) { + if (doNotScan.has(dep.name)) { + continue; + } + const existingResult = result[dep.name]?.find((x) => x.dir === dep.dir); + if (existingResult) { + // we already have this, just add an additional path to it + existingResult.paths.push(nestedDepPath); + } else { + stack.push({ dir: dep.dir, depPath: nestedDepPath }); + } + } + } + return result; +} + +// TODO better implementation +function getDependencyDir(pkgRequire: NodeRequire, dep: string) { + try { + return path.dirname(pkgRequire.resolve(path.join(dep, 'package.json'))); + } catch (e) { + // does not export package.json, walk up parent directories of default export until we find the one named like the package + let dir = path.dirname(pkgRequire.resolve(path.join(dep))); + while (dir && path.basename(dir) !== dep) { + const parent = path.dirname(dir); + if (parent !== dir) { + dir = parent; + } + } + return dir; + } +} + +function parsePkg(dir: string) { + const pkgFile = path.join(dir, 'package.json'); + try { + return JSON.parse(fs.readFileSync(pkgFile, 'utf-8')); + } catch (e) { + log.warn(`failed to parse ${pkgFile}`, e); + return null; + } +} + +function isSvelte(pkg: Pkg) { + return pkg.svelte || pkg.peerDependencies?.svelte; +} + +export interface SvelteDependency { + name: string; + dir: string; + pkg: Pkg; + paths: string[][]; +} + +export interface Pkg { + name: string; + svelte?: string; + dependencies?: DependencyList; + peerDependencies?: DependencyList; + devDependencies?: DependencyList; + [key: string]: any; +} + +export interface DependencyList { + [key: string]: string; +} diff --git a/packages/vite-plugin-svelte/src/utils/options.ts b/packages/vite-plugin-svelte/src/utils/options.ts index d747cc7fc..7161aeb62 100644 --- a/packages/vite-plugin-svelte/src/utils/options.ts +++ b/packages/vite-plugin-svelte/src/utils/options.ts @@ -13,6 +13,7 @@ import { // eslint-disable-next-line node/no-missing-import } from 'svelte/types/compiler/preprocess'; import path from 'path'; +import { findSvelteDependencies } from './dependencies'; const knownOptions = new Set([ 'configFile', @@ -192,6 +193,14 @@ export function buildExtraViteConfig( } else { log.debug('"svelte" is excluded in optimizeDeps.exclude, skipped adding it to include.'); } + + const svelteDeps = findSvelteDependencies(options.root); + console.log('svelteDeps', svelteDeps); + const svelteDepsToExclude = Object.keys(svelteDeps).filter( + (dep) => !config?.optimizeDeps?.include?.includes(dep) + ); + log.debug(`automatically excluding found svelte dependencies: ${svelteDepsToExclude.join(', ')}`); + exclude.push(...svelteDepsToExclude); const extraViteConfig: Partial = { optimizeDeps: { include, From e5d20f8161014d3e584d4e3044fc5111c038bf74 Mon Sep 17 00:00:00 2001 From: dominikg Date: Thu, 19 Aug 2021 12:14:23 +0200 Subject: [PATCH 02/10] wip: refactor to use recursion, return flat list and prepare for adding optimizeDeps.include --- packages/vite-plugin-svelte/package.json | 2 +- .../src/utils/dependencies.ts | 158 +++++++++++------- .../vite-plugin-svelte/src/utils/options.ts | 71 +++++--- 3 files changed, 142 insertions(+), 89 deletions(-) diff --git a/packages/vite-plugin-svelte/package.json b/packages/vite-plugin-svelte/package.json index f188a1cde..f5147440d 100644 --- a/packages/vite-plugin-svelte/package.json +++ b/packages/vite-plugin-svelte/package.json @@ -22,7 +22,7 @@ "./package.json": "./package.json" }, "scripts": { - "dev": "pnpm run build:ci -- --watch src", + "dev": "pnpm run build:ci -- --sourcemap --watch src", "build:ci": "rimraf dist && tsup-node src/index.ts --format esm,cjs --no-splitting", "build": "pnpm run build:ci -- --dts --sourcemap" }, diff --git a/packages/vite-plugin-svelte/src/utils/dependencies.ts b/packages/vite-plugin-svelte/src/utils/dependencies.ts index 424d8b7c6..d072878b7 100644 --- a/packages/vite-plugin-svelte/src/utils/dependencies.ts +++ b/packages/vite-plugin-svelte/src/utils/dependencies.ts @@ -3,118 +3,150 @@ import path from 'path'; import fs from 'fs'; import { createRequire } from 'module'; -export function findSvelteDependencies( - root: string, - cwdFallback = true -): Record { +export function findSvelteDependencies(root: string, cwdFallback = true): SvelteDependency[] { log.debug(`findSvelteDependencies: searching svelte dependencies in ${root}`); - const pkgFile = path.join(root, 'package.json'); if (!fs.existsSync(pkgFile)) { if (cwdFallback) { const cwd = process.cwd(); if (root !== cwd) { - log.debug(`no package.json found in vite root ${root}`); + log.debug(`no package.json found in vite root ${root}`); return findSvelteDependencies(cwd, false); } } log.debug(`no package.json found, search failed`); - return {}; + return []; } - const stack = [{ dir: root, depPath: [] as string[] }]; - // name->SvelteDependency[] - const result: Record = {}; - const doNotScan = new Set([ - 'svelte', - 'vite', - '@sveltejs/kit', - '@sveltejs/vite-plugin-svelte' - ]); + const pkg = parsePkg(root); + if (!pkg) { + return []; + } - while (stack.length > 0) { - const { dir, depPath } = stack.shift()!; - const pkg = parsePkg(dir); - if (!pkg) { - continue; - } - if (dir !== root) { - if (!isSvelte(pkg)) { - doNotScan.add(pkg.name); - continue; - } - if (!result[pkg.name]) { - result[pkg.name] = []; - } - result[pkg.name].push({ name: pkg.name, pkg, dir, paths: [depPath] }); - } + const deps = [ + ...Object.keys(pkg.dependencies || {}), + ...Object.keys(pkg.devDependencies || {}) + ].filter((dep) => !excludeFromScan(dep)); - const pkgRequire = createRequire(dir); - const deps = [ - ...Object.keys(pkg.dependencies || {}), - ...Object.keys(pkg.devDependencies || {}) - ]; - const resolvedDeps = deps.map((dep) => ({ name: dep, dir: getDependencyDir(pkgRequire, dep) })); - const nestedDepPath = [...depPath, pkg.name]; - for (const dep of resolvedDeps) { - if (doNotScan.has(dep.name)) { - continue; + return getSvelteDependencies(deps, root); +} + +function getSvelteDependencies( + deps: string[], + pkgDir: string, + path: string[] = [] +): SvelteDependency[] { + const result = []; + const require = createRequire(pkgDir); + const resolvedDeps = deps.map((dep) => resolveSvelteDependency(dep, require)).filter(Boolean); + // @ts-ignore + for (const { pkg, dir } of resolvedDeps) { + result.push({ name: pkg.name, pkg, dir, path }); + if (pkg.dependencies) { + let dependencyNames = Object.keys(pkg.dependencies); + const circular = dependencyNames.filter((name) => path.includes(name)); + if (circular.length > 0) { + log.warn.enabled && + log.warn( + `skipping circular svelte dependencies in automated vite optimizeDeps handling`, + circular.map((x) => path.concat(x).join('>')) + ); + dependencyNames = dependencyNames.filter((name) => !path.includes(name)); } - const existingResult = result[dep.name]?.find((x) => x.dir === dep.dir); - if (existingResult) { - // we already have this, just add an additional path to it - existingResult.paths.push(nestedDepPath); - } else { - stack.push({ dir: dep.dir, depPath: nestedDepPath }); + if (path.length === 3) { + log.warn.once(`encountered deep svelte dependency tree ${path.join('>')}`); } + result.push(...getSvelteDependencies(dependencyNames, dir, path.concat(pkg.name))); } } return result; } -// TODO better implementation -function getDependencyDir(pkgRequire: NodeRequire, dep: string) { +function resolveSvelteDependency( + dep: string, + require: NodeRequire +): { dir: string; pkg: Pkg } | void { try { - return path.dirname(pkgRequire.resolve(path.join(dep, 'package.json'))); + const pkgJson = `${dep}/package.json`; + const pkg = require(pkgJson); + if (!isSvelte(pkg)) { + return; + } + const dir = path.dirname(require.resolve(pkgJson)); + return { dir, pkg }; } catch (e) { - // does not export package.json, walk up parent directories of default export until we find the one named like the package - let dir = path.dirname(pkgRequire.resolve(path.join(dep))); - while (dir && path.basename(dir) !== dep) { + log.debug.once(`dependency ${dep} does not export package.json`, e); + // walk up from default export until we find package.json with name=dep + let dir = path.dirname(require.resolve(dep)); + while (dir) { + const pkg = parsePkg(dir, true); + if (pkg && pkg.name === dep) { + if (!isSvelte(pkg)) { + return; + } + log.warn(`package ${dep} has a "svelte" field but does not export it's package.json`); + return { dir, pkg }; + } const parent = path.dirname(dir); - if (parent !== dir) { - dir = parent; + if (parent === dir) { + break; } + dir = parent; } - return dir; } + log.debug.once(`failed to resolve ${dep}`); } -function parsePkg(dir: string) { +function parsePkg(dir: string, silent = false): Pkg | void { const pkgFile = path.join(dir, 'package.json'); try { return JSON.parse(fs.readFileSync(pkgFile, 'utf-8')); } catch (e) { - log.warn(`failed to parse ${pkgFile}`, e); - return null; + !silent && log.warn.enabled && log.warn(`failed to parse ${pkgFile}`, e); } } function isSvelte(pkg: Pkg) { - return pkg.svelte || pkg.peerDependencies?.svelte; + return !!pkg.svelte; +} + +const EXCLUDE = [ + '@sveltejs/vite-plugin-svelte', + '@sveltejs/kit', + 'svelte', + 'svelte-hmr', + 'svelte-preprocess', + 'eslint', + 'prettier', + 'vite', + 'postcss' +]; +const EXCLUDE_PREFIX = [ + '@types/', + '@rollup/', + '@sveltejs/adapter-', + 'eslint-plugin-', + 'prettier-plugin-', + 'postcss-plugin-', + '@postcss-plugins/', + '@rollup/' +]; + +function excludeFromScan(dep: string): boolean { + return EXCLUDE.some((ex) => dep === ex) || EXCLUDE_PREFIX.some((ex) => dep.startsWith(ex)); } export interface SvelteDependency { name: string; dir: string; pkg: Pkg; - paths: string[][]; + path: string[]; } export interface Pkg { name: string; svelte?: string; dependencies?: DependencyList; - peerDependencies?: DependencyList; devDependencies?: DependencyList; [key: string]: any; } diff --git a/packages/vite-plugin-svelte/src/utils/options.ts b/packages/vite-plugin-svelte/src/utils/options.ts index 7161aeb62..641f77378 100644 --- a/packages/vite-plugin-svelte/src/utils/options.ts +++ b/packages/vite-plugin-svelte/src/utils/options.ts @@ -14,6 +14,7 @@ import { } from 'svelte/types/compiler/preprocess'; import path from 'path'; import { findSvelteDependencies } from './dependencies'; +import { DepOptimizationOptions } from 'vite/src/node/optimizer/index'; const knownOptions = new Set([ 'configFile', @@ -180,32 +181,8 @@ export function buildExtraViteConfig( options: ResolvedOptions, config: UserConfig ): Partial { - // include svelte imports for optimization unless explicitly excluded - const include: string[] = []; - const exclude: string[] = ['svelte-hmr']; - const isSvelteExcluded = config.optimizeDeps?.exclude?.includes('svelte'); - if (!isSvelteExcluded) { - const svelteImportsToInclude = SVELTE_IMPORTS.filter((x) => x !== 'svelte/ssr'); // not used on clientside - log.debug( - `adding bare svelte packages to optimizeDeps.include: ${svelteImportsToInclude.join(', ')} ` - ); - include.push(...svelteImportsToInclude); - } else { - log.debug('"svelte" is excluded in optimizeDeps.exclude, skipped adding it to include.'); - } - - const svelteDeps = findSvelteDependencies(options.root); - console.log('svelteDeps', svelteDeps); - const svelteDepsToExclude = Object.keys(svelteDeps).filter( - (dep) => !config?.optimizeDeps?.include?.includes(dep) - ); - log.debug(`automatically excluding found svelte dependencies: ${svelteDepsToExclude.join(', ')}`); - exclude.push(...svelteDepsToExclude); const extraViteConfig: Partial = { - optimizeDeps: { - include, - exclude - }, + optimizeDeps: buildOptimizeDepsForSvelte(options.root, config.optimizeDeps), resolve: { mainFields: [...SVELTE_RESOLVE_MAIN_FIELDS], dedupe: [...SVELTE_IMPORTS, ...SVELTE_HMR_IMPORTS] @@ -242,6 +219,50 @@ export function buildExtraViteConfig( return extraViteConfig; } +function buildOptimizeDepsForSvelte( + root: string, + optimizeDeps?: DepOptimizationOptions +): DepOptimizationOptions { + // include svelte imports for optimization unless explicitly excluded + const include: string[] = []; + const exclude: string[] = ['svelte-hmr']; + const isSvelteExcluded = optimizeDeps?.exclude?.includes('svelte'); + if (!isSvelteExcluded) { + const svelteImportsToInclude = SVELTE_IMPORTS.filter((x) => x !== 'svelte/ssr'); // not used on clientside + log.debug( + `adding bare svelte packages to optimizeDeps.include: ${svelteImportsToInclude.join(', ')} ` + ); + include.push(...svelteImportsToInclude); + } else { + log.debug('"svelte" is excluded in optimizeDeps.exclude, skipped adding it to include.'); + } + + // extra handling for svelte dependencies in the project + const svelteDeps = findSvelteDependencies(root); + console.log('svelteDeps', svelteDeps); + const svelteDepsToExclude = Array.from(new Set(svelteDeps.map((dep) => dep.name))).filter( + (dep) => !optimizeDeps?.include?.includes(dep) + ); + log.debug(`automatically excluding found svelte dependencies: ${svelteDepsToExclude.join(', ')}`); + exclude.push(...svelteDepsToExclude); + + /* // TODO enable once https://github.com/vitejs/vite/pull/4634 lands + const transitiveDepsToInclude = svelteDeps + .filter((dep) => svelteDepsToExclude.includes(dep.name)) + .flatMap((dep) => + Object.keys(dep.pkg.dependencies || {}) + .filter((depOfDep) => !svelteDepsToExclude.includes(depOfDep)) + .map((depOfDep) => dep.path.concat(depOfDep).join('>')) + ); + log.debug( + `reincluding transitive dependencies of excluded svelte dependencies`, + transitiveDepsToInclude + ); + include.push(...transitiveDepsToInclude); +*/ + return { include, exclude }; +} + export interface Options { // eslint-disable no-unused-vars /** path to svelte config file, either absolute or relative to vite root*/ From 5594a78fd4737d78fe680e381798190010270860 Mon Sep 17 00:00:00 2001 From: dominikg Date: Thu, 19 Aug 2021 16:19:30 +0200 Subject: [PATCH 03/10] wip: add tests, fix path for createRequire and improve exclusions --- .../hmr-test-dependency/package.json | 3 +- .../test-dependency-svelte-field/package.json | 3 ++ .../src/utils/__tests__/dependencies.spec.ts | 22 +++++++++ .../src/utils/dependencies.ts | 49 +++++++++++++------ pnpm-lock.yaml | 5 +- 5 files changed, 64 insertions(+), 18 deletions(-) create mode 100644 packages/vite-plugin-svelte/src/utils/__tests__/dependencies.spec.ts diff --git a/packages/e2e-tests/hmr-test-dependency/package.json b/packages/e2e-tests/hmr-test-dependency/package.json index eb7c9b010..249376295 100644 --- a/packages/e2e-tests/hmr-test-dependency/package.json +++ b/packages/e2e-tests/hmr-test-dependency/package.json @@ -2,5 +2,6 @@ "version": "1.0.0", "private": true, "name": "e2e-tests-hmr-test-dependency", - "main": "index.js" + "main": "index.js", + "svelte": "index.js" } diff --git a/packages/e2e-tests/test-dependency-svelte-field/package.json b/packages/e2e-tests/test-dependency-svelte-field/package.json index fd24f6a86..148cb1d67 100644 --- a/packages/e2e-tests/test-dependency-svelte-field/package.json +++ b/packages/e2e-tests/test-dependency-svelte-field/package.json @@ -9,5 +9,8 @@ ], "exports": { "./package.json": "./package.json" + }, + "dependencies": { + "e2e-tests-hmr-test-dependency": "workspace:*" } } diff --git a/packages/vite-plugin-svelte/src/utils/__tests__/dependencies.spec.ts b/packages/vite-plugin-svelte/src/utils/__tests__/dependencies.spec.ts new file mode 100644 index 000000000..94d5fbf30 --- /dev/null +++ b/packages/vite-plugin-svelte/src/utils/__tests__/dependencies.spec.ts @@ -0,0 +1,22 @@ +import { findSvelteDependencies } from '../dependencies'; +import * as path from 'path'; + +describe('dependencies', () => { + describe('findSvelteDependencies', () => { + it('should find svelte dependencies in packages/e2e-test/hmr', async () => { + const deps = findSvelteDependencies(path.resolve('packages/e2e-tests/hmr')); + expect(deps).toHaveLength(1); + expect(deps[0].name).toBe('e2e-tests-hmr-test-dependency'); + expect(deps[0].path).toEqual([]); + }); + it('should find nested svelte dependencies in packages/e2e-test/package-json-svelte-field', async () => { + const deps = findSvelteDependencies( + path.resolve('packages/e2e-tests/package-json-svelte-field') + ); + expect(deps).toHaveLength(2); + expect(deps[0].name).toBe('e2e-tests-test-dependency-svelte-field'); + expect(deps[1].name).toBe('e2e-tests-hmr-test-dependency'); + expect(deps[1].path).toEqual(['e2e-tests-test-dependency-svelte-field']); + }); + }); +}); diff --git a/packages/vite-plugin-svelte/src/utils/dependencies.ts b/packages/vite-plugin-svelte/src/utils/dependencies.ts index d072878b7..0aec03f87 100644 --- a/packages/vite-plugin-svelte/src/utils/dependencies.ts +++ b/packages/vite-plugin-svelte/src/utils/dependencies.ts @@ -37,8 +37,10 @@ function getSvelteDependencies( path: string[] = [] ): SvelteDependency[] { const result = []; - const require = createRequire(pkgDir); - const resolvedDeps = deps.map((dep) => resolveSvelteDependency(dep, require)).filter(Boolean); + const localRequire = createRequire(`${pkgDir}/package.json`); + const resolvedDeps = deps + .map((dep) => resolveSvelteDependency(dep, localRequire)) + .filter(Boolean); // @ts-ignore for (const { pkg, dir } of resolvedDeps) { result.push({ name: pkg.name, pkg, dir, path }); @@ -64,20 +66,20 @@ function getSvelteDependencies( function resolveSvelteDependency( dep: string, - require: NodeRequire + localRequire: NodeRequire ): { dir: string; pkg: Pkg } | void { try { const pkgJson = `${dep}/package.json`; - const pkg = require(pkgJson); + const pkg = localRequire(pkgJson); if (!isSvelte(pkg)) { return; } - const dir = path.dirname(require.resolve(pkgJson)); + const dir = path.dirname(localRequire.resolve(pkgJson)); return { dir, pkg }; } catch (e) { log.debug.once(`dependency ${dep} does not export package.json`, e); // walk up from default export until we find package.json with name=dep - let dir = path.dirname(require.resolve(dep)); + let dir = path.dirname(localRequire.resolve(dep)); while (dir) { const pkg = parsePkg(dir, true); if (pkg && pkg.name === dep) { @@ -113,27 +115,42 @@ function isSvelte(pkg: Pkg) { const EXCLUDE = [ '@sveltejs/vite-plugin-svelte', '@sveltejs/kit', + 'autoprefixer', + 'dotenv', + 'esbuild', + 'eslint', + 'jest', + 'postcss', + 'prettier', 'svelte', 'svelte-hmr', 'svelte-preprocess', - 'eslint', - 'prettier', - 'vite', - 'postcss' + 'typescript', + 'vite' ]; const EXCLUDE_PREFIX = [ - '@types/', + '@postcss-plugins/', '@rollup/', '@sveltejs/adapter-', - 'eslint-plugin-', - 'prettier-plugin-', + '@types/', + '@typescript-eslint/', + 'eslint-', + 'jest-', 'postcss-plugin-', - '@postcss-plugins/', - '@rollup/' + 'prettier-plugin-', + 'rollup-plugin-', + 'vite-plugin-' ]; function excludeFromScan(dep: string): boolean { - return EXCLUDE.some((ex) => dep === ex) || EXCLUDE_PREFIX.some((ex) => dep.startsWith(ex)); + return ( + EXCLUDE.some((ex) => dep === ex) || + EXCLUDE_PREFIX.some((ex) => + ex.startsWith('@') + ? dep.startsWith(ex) + : dep.substring(dep.lastIndexOf('/') + 1).startsWith(ex) + ) + ); } export interface SvelteDependency { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index dd9ed7de7..879ebea29 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -219,7 +219,10 @@ importers: vite: 2.5.0 packages/e2e-tests/test-dependency-svelte-field: - specifiers: {} + specifiers: + e2e-tests-hmr-test-dependency: workspace:* + dependencies: + e2e-tests-hmr-test-dependency: link:../hmr-test-dependency packages/e2e-tests/ts-type-import: specifiers: From 48cd3771041806ae507c6ab668dca1d40cd8e1da Mon Sep 17 00:00:00 2001 From: dominikg Date: Thu, 19 Aug 2021 17:05:54 +0200 Subject: [PATCH 04/10] wip: improve naming and documentation --- .../src/utils/__tests__/dependencies.spec.ts | 8 ++-- .../src/utils/dependencies.ts | 40 ++++++++++++------- .../vite-plugin-svelte/src/utils/options.ts | 4 +- 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/packages/vite-plugin-svelte/src/utils/__tests__/dependencies.spec.ts b/packages/vite-plugin-svelte/src/utils/__tests__/dependencies.spec.ts index 94d5fbf30..d028186b4 100644 --- a/packages/vite-plugin-svelte/src/utils/__tests__/dependencies.spec.ts +++ b/packages/vite-plugin-svelte/src/utils/__tests__/dependencies.spec.ts @@ -1,16 +1,16 @@ -import { findSvelteDependencies } from '../dependencies'; +import { findRootSvelteDependencies } from '../dependencies'; import * as path from 'path'; describe('dependencies', () => { - describe('findSvelteDependencies', () => { + describe('findRootSvelteDependencies', () => { it('should find svelte dependencies in packages/e2e-test/hmr', async () => { - const deps = findSvelteDependencies(path.resolve('packages/e2e-tests/hmr')); + const deps = findRootSvelteDependencies(path.resolve('packages/e2e-tests/hmr')); expect(deps).toHaveLength(1); expect(deps[0].name).toBe('e2e-tests-hmr-test-dependency'); expect(deps[0].path).toEqual([]); }); it('should find nested svelte dependencies in packages/e2e-test/package-json-svelte-field', async () => { - const deps = findSvelteDependencies( + const deps = findRootSvelteDependencies( path.resolve('packages/e2e-tests/package-json-svelte-field') ); expect(deps).toHaveLength(2); diff --git a/packages/vite-plugin-svelte/src/utils/dependencies.ts b/packages/vite-plugin-svelte/src/utils/dependencies.ts index 0aec03f87..cf479c1d5 100644 --- a/packages/vite-plugin-svelte/src/utils/dependencies.ts +++ b/packages/vite-plugin-svelte/src/utils/dependencies.ts @@ -3,7 +3,7 @@ import path from 'path'; import fs from 'fs'; import { createRequire } from 'module'; -export function findSvelteDependencies(root: string, cwdFallback = true): SvelteDependency[] { +export function findRootSvelteDependencies(root: string, cwdFallback = true): SvelteDependency[] { log.debug(`findSvelteDependencies: searching svelte dependencies in ${root}`); const pkgFile = path.join(root, 'package.json'); if (!fs.existsSync(pkgFile)) { @@ -11,10 +11,10 @@ export function findSvelteDependencies(root: string, cwdFallback = true): Svelte const cwd = process.cwd(); if (root !== cwd) { log.debug(`no package.json found in vite root ${root}`); - return findSvelteDependencies(cwd, false); + return findRootSvelteDependencies(cwd, false); } } - log.debug(`no package.json found, search failed`); + log.warn(`no package.json found, findRootSvelteDependencies failed`); return []; } @@ -26,7 +26,7 @@ export function findSvelteDependencies(root: string, cwdFallback = true): Svelte const deps = [ ...Object.keys(pkg.dependencies || {}), ...Object.keys(pkg.devDependencies || {}) - ].filter((dep) => !excludeFromScan(dep)); + ].filter((dep) => !is_common_without_svelte_field(dep)); return getSvelteDependencies(deps, root); } @@ -56,7 +56,7 @@ function getSvelteDependencies( dependencyNames = dependencyNames.filter((name) => !path.includes(name)); } if (path.length === 3) { - log.warn.once(`encountered deep svelte dependency tree ${path.join('>')}`); + log.warn.once(`encountered deep svelte dependency tree: ${path.join('>')}`); } result.push(...getSvelteDependencies(dependencyNames, dir, path.concat(pkg.name))); } @@ -86,7 +86,9 @@ function resolveSvelteDependency( if (!isSvelte(pkg)) { return; } - log.warn(`package ${dep} has a "svelte" field but does not export it's package.json`); + log.warn.once( + `package.json of ${dep} has a "svelte" field but does not include itself in exports field. Please ask package maintainer to update` + ); return { dir, pkg }; } const parent = path.dirname(dir); @@ -112,7 +114,7 @@ function isSvelte(pkg: Pkg) { return !!pkg.svelte; } -const EXCLUDE = [ +const COMMON_DEPENDENCIES_WITHOUT_SVELTE_FIELD = [ '@sveltejs/vite-plugin-svelte', '@sveltejs/kit', 'autoprefixer', @@ -120,6 +122,7 @@ const EXCLUDE = [ 'esbuild', 'eslint', 'jest', + 'mdsvex', 'postcss', 'prettier', 'svelte', @@ -128,7 +131,7 @@ const EXCLUDE = [ 'typescript', 'vite' ]; -const EXCLUDE_PREFIX = [ +const COMMON_PREFIXES_WITHOUT_SVELTE_FIELD = [ '@postcss-plugins/', '@rollup/', '@sveltejs/adapter-', @@ -142,13 +145,22 @@ const EXCLUDE_PREFIX = [ 'vite-plugin-' ]; -function excludeFromScan(dep: string): boolean { +/** + * Test for common dependency names that tell us it is not a package including a svelte field, eg. eslint + plugins. + * + * This speeds up the find process as we don't have to try and require the package.json for all of them + * + * @param dependency {string} + * @returns {boolean} true if it is a dependency without a svelte field + */ +function is_common_without_svelte_field(dependency: string): boolean { return ( - EXCLUDE.some((ex) => dep === ex) || - EXCLUDE_PREFIX.some((ex) => - ex.startsWith('@') - ? dep.startsWith(ex) - : dep.substring(dep.lastIndexOf('/') + 1).startsWith(ex) + COMMON_DEPENDENCIES_WITHOUT_SVELTE_FIELD.includes(dependency) || + COMMON_PREFIXES_WITHOUT_SVELTE_FIELD.some( + (prefix) => + prefix.startsWith('@') + ? dependency.startsWith(prefix) + : dependency.substring(dependency.lastIndexOf('/') + 1).startsWith(prefix) // check prefix omitting @scope/ ) ); } diff --git a/packages/vite-plugin-svelte/src/utils/options.ts b/packages/vite-plugin-svelte/src/utils/options.ts index 641f77378..c85bcf653 100644 --- a/packages/vite-plugin-svelte/src/utils/options.ts +++ b/packages/vite-plugin-svelte/src/utils/options.ts @@ -13,7 +13,7 @@ import { // eslint-disable-next-line node/no-missing-import } from 'svelte/types/compiler/preprocess'; import path from 'path'; -import { findSvelteDependencies } from './dependencies'; +import { findRootSvelteDependencies } from './dependencies'; import { DepOptimizationOptions } from 'vite/src/node/optimizer/index'; const knownOptions = new Set([ @@ -238,7 +238,7 @@ function buildOptimizeDepsForSvelte( } // extra handling for svelte dependencies in the project - const svelteDeps = findSvelteDependencies(root); + const svelteDeps = findRootSvelteDependencies(root); console.log('svelteDeps', svelteDeps); const svelteDepsToExclude = Array.from(new Set(svelteDeps.map((dep) => dep.name))).filter( (dep) => !optimizeDeps?.include?.includes(dep) From a1837b600bf5d8b0cfee8eb37b6688e116017dcf Mon Sep 17 00:00:00 2001 From: dominikg Date: Thu, 19 Aug 2021 19:17:33 +0200 Subject: [PATCH 05/10] cover all dependencies of a default create-svelte project in common dependency test --- packages/vite-plugin-svelte/src/utils/dependencies.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/vite-plugin-svelte/src/utils/dependencies.ts b/packages/vite-plugin-svelte/src/utils/dependencies.ts index cf479c1d5..012dfe6f2 100644 --- a/packages/vite-plugin-svelte/src/utils/dependencies.ts +++ b/packages/vite-plugin-svelte/src/utils/dependencies.ts @@ -115,9 +115,11 @@ function isSvelte(pkg: Pkg) { } const COMMON_DEPENDENCIES_WITHOUT_SVELTE_FIELD = [ + '@lukeed/uuid', '@sveltejs/vite-plugin-svelte', '@sveltejs/kit', 'autoprefixer', + 'cookie', 'dotenv', 'esbuild', 'eslint', @@ -126,12 +128,15 @@ const COMMON_DEPENDENCIES_WITHOUT_SVELTE_FIELD = [ 'postcss', 'prettier', 'svelte', + 'svelte-check', 'svelte-hmr', 'svelte-preprocess', + 'tslib', 'typescript', 'vite' ]; const COMMON_PREFIXES_WITHOUT_SVELTE_FIELD = [ + '@fontsource/', '@postcss-plugins/', '@rollup/', '@sveltejs/adapter-', From 56859671e45ed9a396bf720b397d9f843357fef5 Mon Sep 17 00:00:00 2001 From: dominikg Date: Thu, 19 Aug 2021 19:24:39 +0200 Subject: [PATCH 06/10] fix: remove console.log and enable eslint no-console --- .eslintrc.js | 1 + packages/vite-plugin-svelte/src/utils/options.ts | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/.eslintrc.js b/.eslintrc.js index 37dbb2831..9eb440f3c 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -22,6 +22,7 @@ module.exports = { ecmaVersion: 2020 }, rules: { + 'no-console': ['error'], 'no-debugger': ['error'], 'node/no-missing-import': [ 'error', diff --git a/packages/vite-plugin-svelte/src/utils/options.ts b/packages/vite-plugin-svelte/src/utils/options.ts index c85bcf653..4c8ac80f6 100644 --- a/packages/vite-plugin-svelte/src/utils/options.ts +++ b/packages/vite-plugin-svelte/src/utils/options.ts @@ -239,7 +239,6 @@ function buildOptimizeDepsForSvelte( // extra handling for svelte dependencies in the project const svelteDeps = findRootSvelteDependencies(root); - console.log('svelteDeps', svelteDeps); const svelteDepsToExclude = Array.from(new Set(svelteDeps.map((dep) => dep.name))).filter( (dep) => !optimizeDeps?.include?.includes(dep) ); From 605095f269428ca2c1496a8981b45417195ae223 Mon Sep 17 00:00:00 2001 From: dominikg Date: Thu, 19 Aug 2021 19:26:53 +0200 Subject: [PATCH 07/10] add changeset --- .changeset/four-chairs-rhyme.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/four-chairs-rhyme.md diff --git a/.changeset/four-chairs-rhyme.md b/.changeset/four-chairs-rhyme.md new file mode 100644 index 000000000..e354562df --- /dev/null +++ b/.changeset/four-chairs-rhyme.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/vite-plugin-svelte': minor +--- + +automatically exclude svelte dependencies in vite.optimizeDeps From 832336c15c8a4b2323b29a5f2b78f7da5ed794a4 Mon Sep 17 00:00:00 2001 From: dominikg Date: Thu, 19 Aug 2021 19:47:02 +0200 Subject: [PATCH 08/10] fix: limit eslint no-console to vite-plugin-svelte/src --- .eslintrc.js | 10 ++++++++-- packages/vite-plugin-svelte/src/utils/log.ts | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 9eb440f3c..e2a91e37a 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -22,8 +22,8 @@ module.exports = { ecmaVersion: 2020 }, rules: { - 'no-console': ['error'], - 'no-debugger': ['error'], + 'no-console': 'off', + 'no-debugger': 'error', 'node/no-missing-import': [ 'error', { @@ -59,6 +59,12 @@ module.exports = { 'no-process-exit': 'off' }, overrides: [ + { + files: ['packages/vite-plugin-svelte/src/**'], + rules: { + 'no-console': 'error' + } + }, { files: ['packages/e2e-tests/**', 'packages/playground/**'], rules: { diff --git a/packages/vite-plugin-svelte/src/utils/log.ts b/packages/vite-plugin-svelte/src/utils/log.ts index a3676cc44..348a90d64 100644 --- a/packages/vite-plugin-svelte/src/utils/log.ts +++ b/packages/vite-plugin-svelte/src/utils/log.ts @@ -1,4 +1,4 @@ -/* eslint-disable no-unused-vars */ +/* eslint-disable no-unused-vars,no-console */ import { cyan, yellow, red } from 'kleur/colors'; import debug from 'debug'; import { ResolvedOptions, Warning } from './options'; From d7ae91c8f59dab86b2ccb9b030541d0ec03e343c Mon Sep 17 00:00:00 2001 From: dominikg Date: Fri, 20 Aug 2021 12:14:40 +0200 Subject: [PATCH 09/10] fix: remove warning log that might confuse user, add extra try/catch to ensure find doesn't stop early --- .../src/utils/dependencies.ts | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/packages/vite-plugin-svelte/src/utils/dependencies.ts b/packages/vite-plugin-svelte/src/utils/dependencies.ts index 012dfe6f2..abe2d5adf 100644 --- a/packages/vite-plugin-svelte/src/utils/dependencies.ts +++ b/packages/vite-plugin-svelte/src/utils/dependencies.ts @@ -56,7 +56,7 @@ function getSvelteDependencies( dependencyNames = dependencyNames.filter((name) => !path.includes(name)); } if (path.length === 3) { - log.warn.once(`encountered deep svelte dependency tree: ${path.join('>')}`); + log.debug.once(`encountered deep svelte dependency tree: ${path.join('>')}`); } result.push(...getSvelteDependencies(dependencyNames, dir, path.concat(pkg.name))); } @@ -79,23 +79,27 @@ function resolveSvelteDependency( } catch (e) { log.debug.once(`dependency ${dep} does not export package.json`, e); // walk up from default export until we find package.json with name=dep - let dir = path.dirname(localRequire.resolve(dep)); - while (dir) { - const pkg = parsePkg(dir, true); - if (pkg && pkg.name === dep) { - if (!isSvelte(pkg)) { - return; + try { + let dir = path.dirname(localRequire.resolve(dep)); + while (dir) { + const pkg = parsePkg(dir, true); + if (pkg && pkg.name === dep) { + if (!isSvelte(pkg)) { + return; + } + log.warn.once( + `package.json of ${dep} has a "svelte" field but does not include itself in exports field. Please ask package maintainer to update` + ); + return { dir, pkg }; } - log.warn.once( - `package.json of ${dep} has a "svelte" field but does not include itself in exports field. Please ask package maintainer to update` - ); - return { dir, pkg }; - } - const parent = path.dirname(dir); - if (parent === dir) { - break; + const parent = path.dirname(dir); + if (parent === dir) { + break; + } + dir = parent; } - dir = parent; + } catch (e) { + log.debug.once(`error while trying to find package.json of ${dep}`, e); } } log.debug.once(`failed to resolve ${dep}`); From eeb78b3ac57a6072dda438530aaec13fc20db76a Mon Sep 17 00:00:00 2001 From: dominikg Date: Fri, 20 Aug 2021 12:28:38 +0200 Subject: [PATCH 10/10] fix: prevent duplicate entries in optimizeDeps.include/exclude --- packages/vite-plugin-svelte/src/utils/options.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/vite-plugin-svelte/src/utils/options.ts b/packages/vite-plugin-svelte/src/utils/options.ts index 4c8ac80f6..91d1bee7d 100644 --- a/packages/vite-plugin-svelte/src/utils/options.ts +++ b/packages/vite-plugin-svelte/src/utils/options.ts @@ -232,7 +232,7 @@ function buildOptimizeDepsForSvelte( log.debug( `adding bare svelte packages to optimizeDeps.include: ${svelteImportsToInclude.join(', ')} ` ); - include.push(...svelteImportsToInclude); + include.push(...svelteImportsToInclude.filter((x) => !optimizeDeps?.include?.includes(x))); } else { log.debug('"svelte" is excluded in optimizeDeps.exclude, skipped adding it to include.'); } @@ -243,7 +243,7 @@ function buildOptimizeDepsForSvelte( (dep) => !optimizeDeps?.include?.includes(dep) ); log.debug(`automatically excluding found svelte dependencies: ${svelteDepsToExclude.join(', ')}`); - exclude.push(...svelteDepsToExclude); + exclude.push(...svelteDepsToExclude.filter((x) => !optimizeDeps?.exclude?.includes(x))); /* // TODO enable once https://github.com/vitejs/vite/pull/4634 lands const transitiveDepsToInclude = svelteDeps