fix(vitePreprocess): use relative paths in sourcemap sources and output css dependencies (#625)

dominikg · Bob Fanger · web-flow · commit be73db6860c9 · 2023-04-21T16:52:03.000+02:00
* fix: Sourcemap for "/path/to/Component.svelte" points to missing source files

* refactor: more generic implementation and use unit test instead of e2e

* fix: take sourceRoot into account and add unit tests for sourcemap utils

* fix: use unique suffix for vite-preprocess and simplify suffix stripping

* chore: fix testcase name

* fix: take care of absolute path syntax differences between linux and windows

* fix: make sure sourceRoot is taken into account and removed

* fix: use fileURLToPath and prepend sourceRoot with single slash

* fix: add dependencies to vite preprocess style output and more tests

* fix: update pnpm-lock

* fix: lint

* fix: account for possible empty sources value and revert fileURLToPath use as it turns out there can be file:///&lt;not-absolute-path&gt; inputs wtf

* fix: write tests in os agnostic way, add test for relative sourceroot

---------

Co-authored-by: Bob Fanger &lt;b.fanger@wearetriple.com&gt;
diff --git a/.changeset/big-candles-agree.md b/.changeset/big-candles-agree.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/vite-plugin-svelte': patch
+---
+
+fix(vitePreprocess): add dependencies to style preprocessor output
diff --git a/.changeset/real-jokes-help.md b/.changeset/real-jokes-help.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/vite-plugin-svelte': patch
+---
+
+fix(vitePreprocess): use relative paths without lang suffix in sourcemaps to avoid missing source file errors.
diff --git a/packages/e2e-tests/css-dev-sourcemap/__tests__/css-dev-sourcemap.spec.ts b/packages/e2e-tests/css-dev-sourcemap/__tests__/css-dev-sourcemap.spec.ts
@@ -0,0 +1,38 @@
+import { browserLogs, getColor, getText, isBuild } from '~utils';
+import { expect } from 'vitest';
+
+test('should not have failed requests', async () => {
+	browserLogs.forEach((msg) => {
+		expect(msg).not.toMatch('404');
+	});
+});
+
+test('should apply css compiled from scss', async () => {
+	expect(await getText('#test')).toBe('red');
+	expect(await getColor('#test')).toBe('red');
+	expect(await getText('.foo')).toBe('magenta');
+	expect(await getColor('.foo')).toBe('magenta');
+});
+
+if (!isBuild) {
+	test('should generate sourcemap', async () => {
+		const style = await getText('style');
+		const lines = style.split(`\n`).map((l) => l.trim());
+		const css = lines[0];
+		const mapComment = lines[1];
+		expect(css).toBe(
+			'.foo.s-XsEmFtvddWTw{color:magenta}#test.s-XsEmFtvddWTw{color:red}.s-XsEmFtvddWTw{}'
+		);
+		const b64start = '/*# sourceMappingURL=data:application/json;base64,';
+		const b64end = ' */';
+		expect(mapComment.startsWith(b64start));
+		expect(mapComment.endsWith(b64end));
+		const map = JSON.parse(
+			Buffer.from(mapComment.slice(b64start.length, -1 * b64end.length), 'base64').toString('utf-8')
+		);
+		expect(map.sources).toStrictEqual(['foo.scss', 'App.svelte']);
+		expect(map.file).toBe('App.svelte');
+		// we are not testing the quality of the mapping here, just that it exists.
+		expect(map.mappings).toBeDefined();
+	});
+}
diff --git a/packages/e2e-tests/css-dev-sourcemap/index.html b/packages/e2e-tests/css-dev-sourcemap/index.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html lang="en">
+	<head>
+		<meta charset="utf-8" />
+		<meta name="viewport" content="width=device-width,initial-scale=1" />
+
+		<title>Svelte app</title>
+
+		<script type="module" src="/src/main.js"></script>
+	</head>
+
+	<body></body>
+</html>
diff --git a/packages/e2e-tests/css-dev-sourcemap/package.json b/packages/e2e-tests/css-dev-sourcemap/package.json
@@ -0,0 +1,17 @@
+{
+  "name": "e2e-tests-css-dev-sourcemap",
+  "private": true,
+  "version": "0.0.0",
+  "type": "module",
+  "scripts": {
+    "dev": "vite",
+    "build": "vite build",
+    "preview": "vite preview"
+  },
+  "devDependencies": {
+    "@sveltejs/vite-plugin-svelte": "workspace:^",
+    "svelte": "^3.58.0",
+    "vite": "^4.2.1",
+    "sass": "^1.61.0"
+  }
+}
diff --git a/packages/e2e-tests/css-dev-sourcemap/src/App.svelte b/packages/e2e-tests/css-dev-sourcemap/src/App.svelte
@@ -0,0 +1,11 @@
+<div id="test">red</div>
+<div class="foo">magenta</div>
+
+<style lang="scss">
+	@import './foo';
+	#test {
+		& {
+			color: red;
+		}
+	}
+</style>
diff --git a/packages/e2e-tests/css-dev-sourcemap/src/foo.scss b/packages/e2e-tests/css-dev-sourcemap/src/foo.scss
@@ -0,0 +1,3 @@
+.foo {
+	color: magenta;
+}
diff --git a/packages/e2e-tests/css-dev-sourcemap/src/main.js b/packages/e2e-tests/css-dev-sourcemap/src/main.js
@@ -0,0 +1,7 @@
+import App from './App.svelte';
+
+const app = new App({
+	target: document.body
+});
+
+export default app;
diff --git a/packages/e2e-tests/css-dev-sourcemap/src/vite-env.d.ts b/packages/e2e-tests/css-dev-sourcemap/src/vite-env.d.ts
@@ -0,0 +1,2 @@
+/// <reference types="svelte" />
+/// <reference types="vite/client" />
diff --git a/packages/e2e-tests/css-dev-sourcemap/svelte.config.js b/packages/e2e-tests/css-dev-sourcemap/svelte.config.js
@@ -0,0 +1,5 @@
+import { vitePreprocess } from '@sveltejs/vite-plugin-svelte';
+
+export default {
+	preprocess: [vitePreprocess()]
+};
diff --git a/packages/e2e-tests/css-dev-sourcemap/vite.config.js b/packages/e2e-tests/css-dev-sourcemap/vite.config.js
@@ -0,0 +1,8 @@
+import { build, defineConfig } from 'vite';
+import { svelte } from '@sveltejs/vite-plugin-svelte';
+
+// https://vitejs.dev/config/
+export default defineConfig({
+	plugins: [svelte({})],
+	css: { devSourcemap: true }
+});
diff --git a/packages/vite-plugin-svelte/src/__tests__/fixtures/preprocess/foo.scss b/packages/vite-plugin-svelte/src/__tests__/fixtures/preprocess/foo.scss
@@ -0,0 +1,3 @@
+.foo {
+	color: green;
+}
diff --git a/packages/vite-plugin-svelte/src/__tests__/preprocess.spec.ts b/packages/vite-plugin-svelte/src/__tests__/preprocess.spec.ts
@@ -0,0 +1,51 @@
+import { describe, it, expect } from 'vitest';
+import { vitePreprocess } from '../preprocess';
+import path from 'path';
+import { normalizePath } from 'vite';
+import { fileURLToPath } from 'url';
+
+const fixtureDir = normalizePath(
+	path.join(path.dirname(fileURLToPath(import.meta.url)), 'fixtures', 'preprocess')
+);
+
+describe('vitePreprocess', () => {
+	it('returns function', () => {
+		const preprocessorGroup = vitePreprocess({ script: true, style: true });
+		expect(typeof preprocessorGroup).toBe('object');
+		expect(typeof preprocessorGroup.script).toBe('function');
+		expect(typeof preprocessorGroup.style).toBe('function');
+	});
+
+	describe('style', async () => {
+		it('produces sourcemap with relative filename', async () => {
+			const { style } = vitePreprocess({ style: { css: { devSourcemap: true } } });
+			const scss = `
+			  @import './foo';
+				.foo {
+				  &.bar {
+				    color: red;
+				  }
+				}`.replace(/\t/g, '');
+
+			const processed = await style({
+				content: scss,
+				attributes: {
+					lang: 'scss'
+				},
+				markup: '', // not read by vitePreprocess
+				filename: `${fixtureDir}/File.svelte`
+			});
+			expect(processed).toBeDefined();
+			// @ts-ignore
+			const { code, map, dependencies } = processed;
+			expect(code).toBe('.foo {\n  color: green;\n}\n\n.foo.bar {\n  color: red;\n}');
+			expect(map.file).toBe('File.svelte');
+			expect(map.sources.length).toBe(2);
+			expect(map.sources[0]).toBe('foo.scss');
+			expect(map.sources[1]).toBe('File.svelte');
+			expect(dependencies).toBeDefined();
+			expect(dependencies[0]).toBe(path.resolve(fixtureDir, 'foo.scss'));
+			expect(dependencies.length).toBe(1);
+		});
+	});
+});
diff --git a/packages/vite-plugin-svelte/src/preprocess.ts b/packages/vite-plugin-svelte/src/preprocess.ts
@@ -2,11 +2,13 @@ import { preprocessCSS, resolveConfig, transformWithEsbuild } from 'vite';
 import type { ESBuildOptions, InlineConfig, ResolvedConfig } from 'vite';
 // eslint-disable-next-line node/no-missing-import
 import type { Preprocessor, PreprocessorGroup } from 'svelte/types/compiler/preprocess';
-import { mapSourcesToRelative } from './utils/sourcemaps';
+import { mapToRelative, removeLangSuffix } from './utils/sourcemaps';
 
 const supportedStyleLangs = ['css', 'less', 'sass', 'scss', 'styl', 'stylus', 'postcss', 'sss'];
 const supportedScriptLangs = ['ts'];
 
+export const lang_sep = '.vite-preprocess.';
+
 export function vitePreprocess(opts?: {
 	script?: boolean;
 	style?: boolean | InlineConfig | ResolvedConfig;
@@ -39,7 +41,7 @@ function viteScript(): { script: Preprocessor } {
 				}
 			});
 
-			mapSourcesToRelative(map, filename);
+			mapToRelative(map, filename);
 
 			return {
 				code,
@@ -72,23 +74,29 @@ function viteStyle(config: InlineConfig | ResolvedConfig = {}): {
 			}
 			transform = getCssTransformFn(resolvedConfig);
 		}
-		const moduleId = `${filename}.${lang}`;
-		const { code, map } = await transform(content, moduleId);
-
-		mapSourcesToRelative(map, moduleId);
-
+		const suffix = `${lang_sep}${lang}`;
+		const moduleId = `${filename}${suffix}`;
+		const { code, map, deps } = await transform(content, moduleId);
+		removeLangSuffix(map, suffix);
+		mapToRelative(map, filename);
+		const dependencies = deps ? Array.from(deps).filter((d) => !d.endsWith(suffix)) : undefined;
 		return {
 			code,
-			map: map ?? undefined
+			map: map ?? undefined,
+			dependencies
 		};
 	};
 	// @ts-expect-error tag so can be found by v-p-s
 	style.__resolvedConfig = null;
 	return { style };
 }
 
-// eslint-disable-next-line no-unused-vars
-type CssTransform = (code: string, filename: string) => Promise<{ code: string; map?: any }>;
+type CssTransform = (
+	// eslint-disable-next-line no-unused-vars
+	code: string,
+	// eslint-disable-next-line no-unused-vars
+	filename: string
+) => Promise<{ code: string; map?: any; deps?: Set<string> }>;
 
 function getCssTransformFn(config: ResolvedConfig): CssTransform {
 	return async (code, filename) => {
diff --git a/packages/vite-plugin-svelte/src/utils/__tests__/sourcemaps.spec.ts b/packages/vite-plugin-svelte/src/utils/__tests__/sourcemaps.spec.ts
@@ -0,0 +1,79 @@
+import { describe, it, expect } from 'vitest';
+import { removeLangSuffix, mapToRelative } from '../sourcemaps';
+import { lang_sep } from '../../preprocess';
+import { normalizePath } from 'vite';
+import path from 'path';
+import { fileURLToPath, pathToFileURL } from 'url';
+
+const fixtureDir = normalizePath(
+	path.join(path.dirname(fileURLToPath(import.meta.url)), 'fixtures', 'preprocess')
+);
+const filename = 'File.svelte';
+
+describe('removeLangSuffix', () => {
+	it('removes suffix', () => {
+		const suffix = `${lang_sep}scss`;
+		const map = {
+			file: `${fixtureDir}/${filename}${suffix}`,
+			sources: ['foo.scss', `${fixtureDir}/${filename}${suffix}`],
+			sourceRoot: fixtureDir
+		};
+		removeLangSuffix(map, suffix);
+		expect(map.file).toBe(`${fixtureDir}/${filename}`);
+		expect(map.sourceRoot).toBe(fixtureDir);
+		expect(map.sources[0]).toBe('foo.scss');
+		expect(map.sources[1]).toBe(`${fixtureDir}/${filename}`);
+	});
+});
+
+describe('mapToRelative', () => {
+	it('converts absolute to relative', () => {
+		const file = `${fixtureDir}/File.svelte`;
+		const map = {
+			file,
+			sources: [`${fixtureDir}/foo.scss`, file]
+		};
+		mapToRelative(map, file);
+		expect(map.file).toBe('File.svelte');
+		expect(map.sources[0]).toBe('foo.scss');
+		expect(map.sources[1]).toBe('File.svelte');
+	});
+
+	it('accounts for sourceRoot', () => {
+		const file = `${fixtureDir}/File.svelte`;
+		const sourceRoot = normalizePath(path.resolve(fixtureDir, '..'));
+		const rootedBase = fixtureDir.replace(sourceRoot, '');
+		const map = {
+			file,
+			sourceRoot,
+			sources: [
+				`${rootedBase}/foo.scss`,
+				`${rootedBase}/File.svelte`,
+				`${pathToFileURL(`${fixtureDir}/bar.scss`)}`
+			]
+		};
+		mapToRelative(map, file);
+		expect(map.file).toBe('File.svelte');
+		expect(map.sources[0]).toBe('foo.scss');
+		expect(map.sources[1]).toBe('File.svelte');
+		expect(map.sources[2]).toBe('bar.scss');
+		expect(map.sources.length).toBe(3);
+		expect(map.sourceRoot).not.toBeDefined();
+	});
+
+	it('accounts for relative sourceRoot', () => {
+		const file = `${fixtureDir}/File.svelte`;
+		const map = {
+			file,
+			sourceRoot: './some-path/..',
+			sources: [`foo.scss`, `File.svelte`, `${pathToFileURL(`${fixtureDir}/bar.scss`)}`]
+		};
+		mapToRelative(map, file);
+		expect(map.file).toBe('File.svelte');
+		expect(map.sources[0]).toBe('./some-path/../foo.scss');
+		expect(map.sources[1]).toBe('./some-path/../File.svelte');
+		expect(map.sources[2]).toBe('bar.scss');
+		expect(map.sources.length).toBe(3);
+		expect(map.sourceRoot).not.toBeDefined();
+	});
+});
diff --git a/packages/vite-plugin-svelte/src/utils/compile.ts b/packages/vite-plugin-svelte/src/utils/compile.ts
@@ -9,7 +9,7 @@ import { StatCollection } from './vite-plugin-svelte-stats';
 //eslint-disable-next-line node/no-missing-import
 import type { Processed } from 'svelte/types/compiler/preprocess';
 import { createInjectScopeEverythingRulePreprocessorGroup } from './preprocess';
-import { mapSourcesToRelative } from './sourcemaps';
+import { mapToRelative } from './sourcemaps';
 
 const scriptLangRE = /<script [^>]*lang=["']?([^"' >]+)["']?[^>]*>/;
 
@@ -95,7 +95,7 @@ const _createCompileSvelte = (makeHot: Function) => {
 			if (preprocessed.map) compileOptions.sourcemap = preprocessed.map;
 		}
 		if (typeof preprocessed?.map === 'object') {
-			mapSourcesToRelative(preprocessed?.map, filename);
+			mapToRelative(preprocessed?.map, filename);
 		}
 		if (raw && svelteRequest.query.type === 'preprocessed') {
 			// shortcut
@@ -132,8 +132,8 @@ const _createCompileSvelte = (makeHot: Function) => {
 		if (endStat) {
 			endStat();
 		}
-		mapSourcesToRelative(compiled.js?.map, filename);
-		mapSourcesToRelative(compiled.css?.map, filename);
+		mapToRelative(compiled.js?.map, filename);
+		mapToRelative(compiled.css?.map, filename);
 		if (!raw) {
 			// wire css import and code for hmr
 			const hasCss = compiled.css?.code?.trim().length > 0;
diff --git a/packages/vite-plugin-svelte/src/utils/sourcemaps.ts b/packages/vite-plugin-svelte/src/utils/sourcemaps.ts
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +'@sveltejs/vite-plugin-svelte': patch
 +---
++
 +fix(vitePreprocess): add dependencies to style preprocessor output
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+/// <reference types="svelte" />`
	`2`	`+/// <reference types="vite/client" />`