preprocessor sourcemaps: prettify code 2

move string_with_map.ts to string_with_sourcemap.ts
rename GeneratedStringWithMap to StringWithSourcemap
rename offset_source_location to sourcemap_add_offset
rename source_maps to sourcemap_list
rename source_locator to get_location
rename as_sourcemap to get_sourcemap
rename encode to sourcemap_encode
move { brace to line before
add some comments

src/compiler/preprocess/index.ts

@@ -1,7 +1,7 @@
 import remapper from '@ampproject/remapping';
 import { decode } from 'sourcemap-codec';
 import { getLocator } from 'locate-character';
-import { GeneratedStringWithMap, offset_source_location } from '../utils/string_with_map';
+import { StringWithSourcemap, sourcemap_add_offset } from '../utils/string_with_sourcemap';
 
 
 export interface Processed {
@@ -43,7 +43,7 @@ function parse_attributes(str: string) {
 interface Replacement {
 	offset: number;
 	length: number;
-	replacement: GeneratedStringWithMap;
+	replacement: StringWithSourcemap;
 }
 
 export default async function preprocess(
@@ -60,28 +60,41 @@ export default async function preprocess(
 	const markup = preprocessors.map(p => p.markup).filter(Boolean);
 	const script = preprocessors.map(p => p.script).filter(Boolean);
 	const style = preprocessors.map(p => p.style).filter(Boolean);
-	const source_maps: Array<Processed['map']> = [];
 
-	let source_locator: ReturnType<typeof getLocator>;
+	const sourcemap_list: Array<Processed['map']> = [];
 
-	function get_replacement(offset: number, original: string, processed: Processed, prefix: string, suffix: string): GeneratedStringWithMap {
-		const generated_prefix = GeneratedStringWithMap.from_source(filename, prefix, source_locator(offset));
-		const generated_suffix = GeneratedStringWithMap.from_source(filename, suffix, source_locator(offset + prefix.length + original.length));
+	let get_location: ReturnType<typeof getLocator>;
+
+	function get_replacement(
+		offset: number,
+		original: string,
+		processed: Processed,
+		prefix: string,
+		suffix: string
+	): StringWithSourcemap {
+		const generated_prefix = StringWithSourcemap.from_source(
+			filename, prefix, get_location(offset));
+		const generated_suffix = StringWithSourcemap.from_source(
+			filename, suffix, get_location(offset + prefix.length + original.length));
 
 		let generated;
 		if (processed.map) {
 			const full_map = typeof processed.map === "string" ? JSON.parse(processed.map) : processed.map;
 			const decoded_map = { ...full_map, mappings: decode(full_map.mappings) };
-			const processed_offset = source_locator(offset + prefix.length);
-			generated = GeneratedStringWithMap.from_generated(processed.code, offset_source_location(processed_offset, decoded_map));
+			const processed_offset = get_location(offset + prefix.length);
+			generated = StringWithSourcemap.from_generated(processed.code, sourcemap_add_offset(processed_offset, decoded_map));
 		} else {
-			generated = GeneratedStringWithMap.from_generated(processed.code);
+			generated = StringWithSourcemap.from_generated(processed.code);
 		}
 		const map = generated_prefix.concat(generated).concat(generated_suffix);
 		return map;
 	}
 
-	async function replace_async(str: string, re: RegExp, func: (...any) => Promise<GeneratedStringWithMap>): Promise<GeneratedStringWithMap> {
+	async function replace_async(
+		str: string,
+		re: RegExp,
+		func: (...any) => Promise<StringWithSourcemap>
+	): Promise<StringWithSourcemap> {
 		const replacement_promises: Array<Promise<Replacement>> = [];
 		str.replace(re, (...args) => {
 			replacement_promises.push(
@@ -98,86 +111,117 @@ export default async function preprocess(
 		});
 		const replacements = await Promise.all(replacement_promises);
 
-		let out: GeneratedStringWithMap;
+		let out: StringWithSourcemap;
 		let last_end = 0;
-		for (const { offset, length, replacement } of replacements)
-		{
-			const content = GeneratedStringWithMap.from_source(filename, str.slice(last_end, offset), source_locator(last_end));
+		for (const { offset, length, replacement } of replacements) {
+			// content = source before replacement
+			const content = StringWithSourcemap.from_source(
+				filename, str.slice(last_end, offset), get_location(last_end));
 			out = out ? out.concat(content) : content;
 			out = out.concat(replacement);
 			last_end = offset + length;
 		}
-		const final_content = GeneratedStringWithMap.from_source(filename, str.slice(last_end), source_locator(last_end));
+		// final_content = source after last replacement
+		const final_content = StringWithSourcemap.from_source(
+			filename, str.slice(last_end), get_location(last_end));
 		out = out.concat(final_content);
 		return out;
 	}
-	
+
 	for (const fn of markup) {
+
+		// run markup preprocessor
 		const processed = await fn({
 			content: source,
 			filename
 		});
-		if (processed && processed.dependencies) dependencies.push(...processed.dependencies);
+
+		if (processed && processed.dependencies) {
+			dependencies.push(...processed.dependencies);
+		}
 		source = processed ? processed.code : source;
-		if (processed && processed.map) source_maps.unshift(processed.map);
+		if (processed && processed.map) {
+			sourcemap_list.unshift(processed.map);
+		}
 	}
 
 	for (const fn of script) {
-		source_locator = getLocator(source);
+		get_location = getLocator(source);
 		const res = await replace_async(
 			source,
 			/<!--[^]*?-->|<script(\s[^]*?)?(?:>([^]*?)<\/script>|\/>)/gi,
 			async (match, attributes = '', content, offset) => {
-				const no_change = () => GeneratedStringWithMap.from_source(filename, match, source_locator(offset));
-
+				const no_change = () => StringWithSourcemap.from_source(
+					filename, match, get_location(offset));
 				if (!attributes && !content) {
 					return no_change();
 				}
-
 				attributes = attributes || '';
+
+				// run script preprocessor
 				const processed = await fn({
 					content,
 					attributes: parse_attributes(attributes),
 					filename
 				});
 
-				if (!processed) return no_change();
-				if (processed.dependencies) dependencies.push(...processed.dependencies);
-				return get_replacement(offset, content, processed, `<script${attributes}>`, `</script>`);
+				if (!processed) {
+					return no_change();
+				}
+				if (processed.dependencies) {
+					dependencies.push(...processed.dependencies);
+				}
+				return get_replacement(
+					offset, content, processed,
+					`<script${attributes}>`, `</script>`
+				);
 			}
 		);
 		source = res.generated;
-		source_maps.unshift(res.as_sourcemap());
+		sourcemap_list.unshift(res.get_sourcemap());
 	}
 
 	for (const fn of style) {
-		source_locator = getLocator(source);
+		get_location = getLocator(source);
 		const res = await replace_async(
 			source,
 			/<!--[^]*?-->|<style(\s[^]*?)?>([^]*?)<\/style>/gi,
 			async (match, attributes = '', content, offset) => {
-				const no_change = () => GeneratedStringWithMap.from_source(filename, match, source_locator(offset));
+				const no_change = () => StringWithSourcemap.from_source(
+					filename, match, get_location(offset));
 				if (!attributes && !content) {
 					return no_change();
 				}
 
+				// run style preprocessor
 				const processed: Processed = await fn({
 					content,
 					attributes: parse_attributes(attributes),
 					filename
 				});
 
-				if (!processed) return no_change();
-				if (processed.dependencies) dependencies.push(...processed.dependencies);
-				return get_replacement(offset, content, processed, `<style${attributes}>`, `</style>`);
+				if (!processed) {
+					return no_change();
+				}
+				if (processed.dependencies) {
+					dependencies.push(...processed.dependencies);
+				}
+				return get_replacement(
+					offset, content, processed,
+					`<style${attributes}>`, `</style>`
+				);
 			}
 		);
 
 		source = res.generated;
-		source_maps.unshift(res.as_sourcemap());
+		sourcemap_list.unshift(res.get_sourcemap());
 	}
 
-	const map: ReturnType<typeof remapper> = source_maps.length == 0 ? null : remapper(source_maps as any, () => null);
+	const map: ReturnType<typeof remapper> =
+		sourcemap_list.length == 0
+			? null
+			: remapper(sourcemap_list as any, () => null);
+
 	return {
 		// TODO return separated output, in future version where svelte.compile supports it:
 		// style: { code: styleCode, map: styleMap },

src/compiler/utils/string_with_sourcemap.ts

@@ -1,4 +1,4 @@
-import { encode } from "sourcemap-codec";
+import { encode as sourcemap_encode } from "sourcemap-codec";
 
 type MappingSegment =
 	| [number]
@@ -24,7 +24,7 @@ function get_end_location(s: string): SourceLocation {
 	};
 }
 
-export function offset_source_location(
+export function sourcemap_add_offset(
 	offset: SourceLocation,
 	map: SourceMappings
 ): SourceMappings {
@@ -72,7 +72,7 @@ function merge_tables<T>(
 	return { table, new_idx };
 }
 
-export class GeneratedStringWithMap {
+export class StringWithSourcemap {
 	readonly generated: string;
 	readonly map: SourceMappings;
 
@@ -81,16 +81,16 @@ export class GeneratedStringWithMap {
 		this.map = map;
 	}
 
-	as_sourcemap() {
+	get_sourcemap() {
 		return {
 			version: 3,
 			sources: this.map.sources,
 			names: [],
-			mappings: encode(this.map.mappings as any),
+			mappings: sourcemap_encode(this.map.mappings as any),
 		};
 	}
 
-	concat(other: GeneratedStringWithMap): GeneratedStringWithMap {
+	concat(other: StringWithSourcemap): StringWithSourcemap {
 		// if one is empty, return the other
 		if (this.generated.length == 0) return other;
 		if (other.generated.length == 0) return this;
@@ -103,6 +103,8 @@ export class GeneratedStringWithMap {
 			this.map.sources,
 			other.map.sources
 		);
+
+		// combine names
 		const {
 			table: new_names,
 			new_idx: other_name_idx
@@ -135,10 +137,14 @@ export class GeneratedStringWithMap {
 		);
 
 		// combine the mappings
+
+		// this.map is readonly, so we copy
 		let new_mappings = this.map.mappings.slice();
 
-		// shift the first line of the second mapping
-		// by the number of columns in the last line of the first mapping
+		// combine:
+		// 1. last line of first map
+		// 2. first line of second map
+		// columns of 2 must be shifted
 		const end = get_end_location(this.generated);
 		const col_offset = end.column + 1;
 		const first_line =
@@ -148,17 +154,18 @@ export class GeneratedStringWithMap {
 						const new_seg = seg.slice() as MappingSegment;
 						new_seg[0] = seg[0] + col_offset;
 						return new_seg;
-				  });
-		new_mappings[new_mappings.length - 1] = new_mappings[
-			new_mappings.length - 1
-		].concat(first_line);
+					});
+
+		// append segments to last line of first map
+		new_mappings[new_mappings.length - 1] =
+		new_mappings[new_mappings.length - 1].concat(first_line);
 
-		// the rest don't need modification and can just be appended
+		// the other lines don't need modification and can just be appended
 		new_mappings = new_mappings.concat(
 			other_mappings.slice(1) as MappingSegment[][]
 		);
 
-		return new GeneratedStringWithMap(
+		return new StringWithSourcemap(
 			this.generated + other.generated, {
 			sources: new_sources,
 			names: new_names,
@@ -169,8 +176,8 @@ export class GeneratedStringWithMap {
 	static from_generated(
 		generated: string,
 		map?: SourceMappings
-	): GeneratedStringWithMap {
-		if (map) return new GeneratedStringWithMap(generated, map);
+	): StringWithSourcemap {
+		if (map) return new StringWithSourcemap(generated, map);
 
 		const replacement_map: SourceMappings = {
 			names: [],
@@ -179,7 +186,7 @@ export class GeneratedStringWithMap {
 		};
 
 		if (generated.length == 0)
-			return new GeneratedStringWithMap(generated, replacement_map);
+			return new StringWithSourcemap(generated, replacement_map);
 
 		// we generate a mapping
 		// where the source was overwritten by the generated
@@ -188,14 +195,14 @@ export class GeneratedStringWithMap {
 			replacement_map.mappings.push([]); // unmapped line
 		}
 
-		return new GeneratedStringWithMap(generated, replacement_map);
+		return new StringWithSourcemap(generated, replacement_map);
 	}
 
 	static from_source(
 		source_file: string,
 		source: string,
 		offset_in_source?: SourceLocation
-	): GeneratedStringWithMap {
+	): StringWithSourcemap {
 		const offset = offset_in_source || { line: 0, column: 0 };
 		const map: SourceMappings = {
 			names: [],
@@ -203,7 +210,7 @@ export class GeneratedStringWithMap {
 			mappings: [],
 		};
 
-		if (source.length == 0) return new GeneratedStringWithMap(source, map);
+		if (source.length == 0) return new StringWithSourcemap(source, map);
 
 		// we create a high resolution identity map here,
 		// we know that it will eventually be merged with svelte's map,
@@ -213,12 +220,13 @@ export class GeneratedStringWithMap {
 		const identity_map = lines.map((line, line_idx) => {
 			const segs = line
 				.split(/([^\d\w\s]|\s+)/g)
-				.filter((x) => x !== "")
+				.filter((s) => s !== "")
 				.map((s) => {
 					const seg: MappingSegment = [
 						pos,
 						0,
-						offset.line + line_idx,
+						line_idx + offset.line,
+						// shift first line
 						pos + (line_idx == 0 ? offset.column : 0),
 					];
 					pos = pos + s.length;
@@ -230,6 +238,6 @@ export class GeneratedStringWithMap {
 
 		map.mappings = identity_map;
 
-		return new GeneratedStringWithMap(source, map);
+		return new StringWithSourcemap(source, map);
 	}
 }