Skip to content

Commit

Permalink
feat: externalize svelte in ssr
Browse files Browse the repository at this point in the history
  • Loading branch information
bluwy committed Sep 23, 2021
1 parent 36a087b commit ffdbcf1
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 75 deletions.
22 changes: 0 additions & 22 deletions packages/vite-plugin-svelte/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { VitePluginSvelteCache } from './utils/vite-plugin-svelte-cache';
import { ensureWatchedFile, setupWatchers } from './utils/watch';
import { resolveViaPackageJsonSvelte } from './utils/resolve';
import { addExtraPreprocessors } from './utils/preprocess';
import { PartialResolvedId } from 'rollup';

export function svelte(inlineOptions?: Partial<Options>): Plugin {
if (process.env.DEBUG != null) {
Expand All @@ -37,8 +36,6 @@ export function svelte(inlineOptions?: Partial<Options>): Plugin {
) => Promise<CompileData>;
/* eslint-enable no-unused-vars */

let resolvedSvelteSSR: Promise<PartialResolvedId | null>;

return {
name: 'vite-plugin-svelte',
// make sure our resolver runs before vite internal resolver to resolve svelte field correctly
Expand Down Expand Up @@ -104,25 +101,6 @@ export function svelte(inlineOptions?: Partial<Options>): Plugin {
return importee; // query with svelte tag, an id we generated, no need for further analysis
}

if (ssr && importee === 'svelte') {
if (!resolvedSvelteSSR) {
resolvedSvelteSSR = this.resolve('svelte/ssr', undefined, { skipSelf: true }).then(
(svelteSSR) => {
log.debug('resolved svelte to svelte/ssr');
return svelteSSR;
},
(err) => {
log.debug(
'failed to resolve svelte to svelte/ssr. Update svelte to a version that exports it',
err
);
return null; // returning null here leads to svelte getting resolved regularly
}
);
}
return resolvedSvelteSSR;
}

try {
const resolved = resolveViaPackageJsonSvelte(importee, importer);
if (resolved) {
Expand Down
1 change: 0 additions & 1 deletion packages/vite-plugin-svelte/src/utils/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ export const SVELTE_IMPORTS = [
'svelte/easing',
'svelte/internal',
'svelte/motion',
'svelte/ssr',
'svelte/store',
'svelte/transition',
'svelte'
Expand Down
26 changes: 3 additions & 23 deletions packages/vite-plugin-svelte/src/utils/dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,10 @@ function getSvelteDependencies(
.map((dep) => resolveDependencyData(dep, localRequire))
.filter(Boolean) as DependencyData[];
for (const { pkg, dir } of resolvedDeps) {
const type = getSvelteDependencyType(pkg);
if (!type) continue;
result.push({ name: pkg.name, type, pkg, dir, path });
if (!isSvelteComponentLib(pkg)) continue;
result.push({ name: pkg.name, pkg, dir, path });
// continue crawling for component libraries so we can optimize them, js libraries are fine
if (type === 'component-library' && pkg.dependencies) {
if (pkg.dependencies) {
let dependencyNames = Object.keys(pkg.dependencies);
const circular = dependencyNames.filter((name) => path.includes(name));
if (circular.length > 0) {
Expand Down Expand Up @@ -104,24 +103,10 @@ function parsePkg(dir: string, silent = false): Pkg | void {
}
}

function getSvelteDependencyType(pkg: Pkg): SvelteDependencyType | undefined {
if (isSvelteComponentLib(pkg)) {
return 'component-library';
} else if (isSvelteLib(pkg)) {
return 'js-library';
} else {
return undefined;
}
}

function isSvelteComponentLib(pkg: Pkg) {
return !!pkg.svelte;
}

function isSvelteLib(pkg: Pkg) {
return !!pkg.dependencies?.svelte || !!pkg.peerDependencies?.svelte;
}

const COMMON_DEPENDENCIES_WITHOUT_SVELTE_FIELD = [
'@lukeed/uuid',
'@sveltejs/vite-plugin-svelte',
Expand Down Expand Up @@ -194,16 +179,11 @@ interface DependencyData {

export interface SvelteDependency {
name: string;
type: SvelteDependencyType;
dir: string;
pkg: Pkg;
path: string[];
}

// component-library => exports svelte components
// js-library => only uses svelte api, no components
export type SvelteDependencyType = 'component-library' | 'js-library';

export interface Pkg {
name: string;
svelte?: string;
Expand Down
36 changes: 7 additions & 29 deletions packages/vite-plugin-svelte/src/utils/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ export function buildExtraViteConfig(
}

// @ts-ignore
extraViteConfig.ssr = buildSSROptionsForSvelte(svelteDeps, options, config);
extraViteConfig.ssr = buildSSROptionsForSvelte(svelteDeps, config);

if (options.experimental?.useVitePreprocess) {
// needed to transform svelte files with component imports
Expand All @@ -227,8 +227,6 @@ function buildOptimizeDepsForSvelte(
options: ResolvedOptions,
optimizeDeps?: DepOptimizationOptions
): DepOptimizationOptions {
// only svelte component libraries needs to be processed for optimizeDeps, js libraries work fine
svelteDeps = svelteDeps.filter((dep) => dep.type === 'component-library');
// include svelte imports for optimization unless explicitly excluded
const include: string[] = [];
const exclude: string[] = ['svelte-hmr'];
Expand All @@ -242,11 +240,8 @@ function buildOptimizeDepsForSvelte(
);
};
if (!isExcluded('svelte')) {
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.filter((x) => !isIncluded(x)));
log.debug(`adding bare svelte packages to optimizeDeps.include: ${SVELTE_IMPORTS.join(', ')} `);
include.push(...SVELTE_IMPORTS.filter((x) => !isIncluded(x)));
} else {
log.debug('"svelte" is excluded in optimizeDeps.exclude, skipped adding it to include.');
}
Expand Down Expand Up @@ -280,28 +275,8 @@ function buildOptimizeDepsForSvelte(
return { include, exclude };
}

function buildSSROptionsForSvelte(
svelteDeps: SvelteDependency[],
options: ResolvedOptions,
config: UserConfig
): any {
function buildSSROptionsForSvelte(svelteDeps: SvelteDependency[], config: UserConfig): any {
const noExternal: string[] = [];

// add svelte to ssr.noExternal unless it is present in ssr.external
// so we can resolve it with svelte/ssr
if (options.isBuild && config.build?.ssr) {
// @ts-ignore
if (!config.ssr?.external?.includes('svelte')) {
noExternal.push('svelte');
}
} else {
// for non-ssr build, we exclude svelte js library deps to make development faster
// and also because vite doesn't handle them properly.
// see https://github.com/sveltejs/vite-plugin-svelte/issues/168
// see https://github.com/vitejs/vite/issues/2579
svelteDeps = svelteDeps.filter((dep) => dep.type === 'component-library');
}

// add svelte dependencies to ssr.noExternal unless present in ssr.external or optimizeDeps.include
noExternal.push(
...Array.from(new Set(svelteDeps.map((s) => s.name))).filter((x) => {
Expand All @@ -310,6 +285,9 @@ function buildSSROptionsForSvelte(
})
);
return {
// never bundle svelte so node can resolve the `node` conditional exports
// NOTE: this also externalizes `svelte/*`
external: ['svelte'],

This comment has been minimized.

Copy link
@benmccann

benmccann Sep 23, 2021

Member

I probably wouldn't do this. It'll get bundled with esbuild and I'm not sure esbuild will tree shake it. Probably better to just leave it as is and then we can eventually fix the Vite issue (vitejs/vite#3953 (comment))

This comment has been minimized.

Copy link
@bluwy

bluwy Sep 24, 2021

Author Member

Yeah that would be the state for now, or once esbuild is able to treeshake noops.

This comment has been minimized.

Copy link
@dominikg

dominikg Sep 24, 2021

Member

esbuild 0.13 improved treeshaking, maybe it'll work? https://github.com/evanw/esbuild/blob/master/CHANGELOG.md

This comment has been minimized.

Copy link
@bluwy

bluwy Sep 24, 2021

Author Member

Tried the new treeShaking option but with the same result unfortunately. We might need to wait for esbuild to address "Dead-code elimination within function bodies".

noExternal
};
}
Expand Down

0 comments on commit ffdbcf1

Please sign in to comment.