review feedback

Author: ehrencrona

src/compiler/preprocess/index.ts

@@ -2,8 +2,8 @@ import { RawSourceMap, DecodedSourceMap } from '@ampproject/remapping/dist/types
 import { getLocator } from 'locate-character';
 import { MappedCode, SourceLocation, sourcemap_add_offset, combine_sourcemaps } from '../utils/mapped_code';
 import { decode_map } from './decode_sourcemap';
-import { replace_in_code, Source } from './replace_in_code';
-import { MarkupPreprocessor, Preprocessor, PreprocessorGroup, Processed } from './types';
+import { replace_in_code, slice_source } from './replace_in_code';
+import { MarkupPreprocessor, Source, Preprocessor, PreprocessorGroup, Processed } from './types';
 
 interface SourceUpdate {
 	string?: string;
@@ -36,7 +36,7 @@ class PreprocessResult implements Source {
 	}
 
 	update_source({ string: source, map, dependencies }: SourceUpdate) {
-		if (source) {
+		if (source != null) {
 			this.source = source;
 			this.get_location = getLocator(source);
 		}
@@ -95,16 +95,18 @@ function processed_tag_to_code(
 	processed: Processed,
 	tag_name: 'style' | 'script',
 	attributes: string,
-	{ source, file_basename, get_location }: Source
+	source: Source
 ): MappedCode {
-	const build_mapped_code = (source: string, offset: number) =>
-		MappedCode.from_source(file_basename, source, get_location(offset));
+	const { file_basename, get_location } = source;
+
+	const build_mapped_code = (code: string, offset: number) =>
+		MappedCode.from_source(slice_source(code, offset, source));
 
 	const tag_open = `<${tag_name}${attributes || ''}>`;
 	const tag_close = `</${tag_name}>`;
 
 	const tag_open_code = build_mapped_code(tag_open, 0);
-	const tag_close_code = build_mapped_code(tag_close, tag_open.length + source.length);
+	const tag_close_code = build_mapped_code(tag_close, tag_open.length + source.source.length);
 
 	const content_code = processed_content_to_code(processed, get_location(tag_open.length), file_basename);
 
@@ -117,7 +119,8 @@ function parse_tag_attributes(str: string) {
 		.split(/\s+/)
 		.filter(Boolean)
 		.reduce((attrs, attr) => {
-			const [key, value] = attr.split('=');
+			const i = attr.indexOf('=');
+			const [key, value] = i > 0 ? [attr.slice(0, i), attr.slice(i+1)] : [attr];
 			const [, unquoted] = (value && value.match(/^['"](.*)['"]$/)) || [];
 
 			return { ...attrs, [key]: unquoted ?? value ?? true };
@@ -132,7 +135,7 @@ async function process_tag(
 	preprocessor: Preprocessor,
 	source: Source
 ): Promise<SourceUpdate> {
-	const { file_basename, filename, get_location } = source;
+	const { filename } = source;
 	const tag_regex =
 		tag_name === 'style'
 			? /<!--[^]*?-->|<style(\s[^]*?)?(?:>([^]*?)<\/style>|\/>)/gi
@@ -146,7 +149,7 @@ async function process_tag(
 		content = '',
 		tag_offset: number
 	): Promise<MappedCode> {
-		const no_change = () => MappedCode.from_source(file_basename, tag_with_content, get_location(tag_offset));
+		const no_change = () => MappedCode.from_source(slice_source(tag_with_content, tag_offset, source));
 
 		if (!attributes && !content) return no_change();
 
@@ -160,12 +163,7 @@ async function process_tag(
 		if (processed.dependencies) dependencies.push(...processed.dependencies);
 		if (!processed.map && processed.code === content) return no_change();
 
-		return processed_tag_to_code(processed, tag_name, attributes, {
-			source: content,
-			get_location: offset => source.get_location(offset + tag_offset),
-			filename,
-			file_basename
-		});
+		return processed_tag_to_code(processed, tag_name, attributes, slice_source(content, tag_offset, source));
 	}
 
 	const { string, map } = await replace_in_code(tag_regex, process_single_tag, source);

src/compiler/preprocess/replace_in_code.ts

@@ -1,19 +1,25 @@
-import { getLocator } from 'locate-character';
 import { MappedCode } from '../utils/mapped_code';
-
-export interface Source {
-	source: string;
-	get_location: ReturnType<typeof getLocator>;
-	file_basename: string;
-	filename: string;
-}
+import { Source } from './types';
 
 interface Replacement {
 	offset: number;
 	length: number;
 	replacement: MappedCode;
 }
 
+export function slice_source(
+	code_slice: string,
+	offset: number,
+	{ file_basename, filename, get_location }: Source
+): Source {
+	return {
+		source: code_slice,
+		get_location: (index: number) => get_location(index + offset),
+		file_basename,
+		filename
+	};
+}
+
 function calculate_replacements(
 	re: RegExp,
 	get_replacement: (...match: any[]) => Promise<MappedCode>,
@@ -40,20 +46,20 @@ function calculate_replacements(
 
 function perform_replacements(
 	replacements: Replacement[],
-	{ file_basename, source, get_location }: Source
+	source: Source
 ): MappedCode {
 	const out = new MappedCode();
 	let last_end = 0;
 
 	for (const { offset, length, replacement } of replacements) {
 		const unchanged_prefix = MappedCode.from_source(
-			file_basename, source.slice(last_end, offset), get_location(last_end));
+			slice_source(source.source.slice(last_end, offset), last_end, source)
+		);
 		out.concat(unchanged_prefix).concat(replacement);
 		last_end = offset + length;
 	}
 
-	const unchanged_suffix = MappedCode.from_source(
-		file_basename, source.slice(last_end), get_location(last_end));
+	const unchanged_suffix = MappedCode.from_source(slice_source(source.source.slice(last_end), last_end, source));
 
 	return out.concat(unchanged_suffix);
 }

src/compiler/preprocess/types.ts

@@ -1,3 +1,12 @@
+import { Location } from 'locate-character';
+
+export interface Source {
+	source: string;
+	get_location: (search: number) => Location;
+	file_basename: string;
+	filename: string;
+}
+
 export interface Processed {
 	code: string;
 	map?: string | object; // we are opaque with the type here to avoid dependency on the remapping module for our public types.

src/compiler/utils/mapped_code.ts

@@ -1,6 +1,7 @@
 import { DecodedSourceMap, RawSourceMap, SourceMapLoader } from '@ampproject/remapping/dist/types/types';
 import remapping from '@ampproject/remapping';
 import { SourceMap } from 'magic-string';
+import { Source } from '../preprocess/types';
 
 export type SourceLocation = {
 	line: number;
@@ -170,7 +171,7 @@ export class MappedCode {
 		const line_count = string.split('\n').length;
 
 		if (map) {
-			// ensure that count of source map mappings lines 
+			// ensure that count of source map mappings lines
 			// is equal to count of generated code lines
 			// (some tools may produce less)
 			const missing_lines = line_count - map.mappings.length;
@@ -188,11 +189,11 @@ export class MappedCode {
 		return new MappedCode(string, map);
 	}
 
-	static from_source(
-		source_file: string, source: string, offset?: SourceLocation
-	): MappedCode {
+	static from_source({ source, file_basename, get_location }: Source): MappedCode {
+		let offset: SourceLocation = get_location(0);
+
 		if (!offset) offset = { line: 0, column: 0 };
-		const map: DecodedSourceMap = { version: 3, names: [], sources: [source_file], mappings: [] };
+		const map: DecodedSourceMap = { version: 3, names: [], sources: [file_basename], mappings: [] };
 		if (source == '') return new MappedCode(source, map);
 
 		// we create a high resolution identity map here,