feat: change preprocessor ordering, allow attributes modification (#8618)

- change mapping order
- add support to modify attributes of script/style tags
- add source mapping tests to preprocessor tests

Author: dummdidumm

CHANGELOG.md

@@ -14,6 +14,8 @@
 * **breaking** Deprecate `SvelteComponentTyped`, use `SvelteComponent` instead ([#8512](https://github.com/sveltejs/svelte/pull/8512))
 * **breaking** Error on falsy values instead of stores passed to `derived` ([#7947](https://github.com/sveltejs/svelte/pull/7947))
 * **breaking** Custom store implementers now need to pass an `update` function additionally to the `set` function ([#6750](https://github.com/sveltejs/svelte/pull/6750))
+* **breaking** Change order in which preprocessors are applied ([#8618](https://github.com/sveltejs/svelte/pull/8618))
+* Add a way to modify attributes for script/style preprocessors ([#8618](https://github.com/sveltejs/svelte/pull/8618))
 * Improve hydration speed by adding `data-svelte-h` attribute to detect unchanged HTML elements ([#7426](https://github.com/sveltejs/svelte/pull/7426))
 * Add `a11y no-noninteractive-element-interactions` rule ([#8391](https://github.com/sveltejs/svelte/pull/8391))
 * Add `a11y-no-static-element-interactions`rule ([#8251](https://github.com/sveltejs/svelte/pull/8251))

site/content/docs/05-compile-time.md

@@ -186,7 +186,7 @@ const ast = svelte.parse(source, { filename: 'App.svelte' });
 
 ### `svelte.preprocess`
 
-A number of [community-maintained preprocessing plugins](https://sveltesociety.dev/tools#preprocessors) are available to allow you to use Svelte with tools like TypeScript, PostCSS, SCSS, and Less.
+A number of [official and community-maintained preprocessing plugins](https://sveltesociety.dev/tools#preprocessors) are available to allow you to use Svelte with tools like TypeScript, PostCSS, SCSS, and Less.
 
 You can write your own preprocessor using the `svelte.preprocess` API.
 
@@ -197,6 +197,7 @@ result: {
 } = await svelte.preprocess(
 	source: string,
 	preprocessors: Array<{
+		name: string,
 		markup?: (input: { content: string, filename: string }) => Promise<{
 			code: string,
 			dependencies?: Array<string>
@@ -220,48 +221,41 @@ result: {
 
 The `preprocess` function provides convenient hooks for arbitrarily transforming component source code. For example, it can be used to convert a `<style lang="sass">` block into vanilla CSS.
 
-The first argument is the component source code. The second is an array of *preprocessors* (or a single preprocessor, if you only have one), where a preprocessor is an object with `markup`, `script` and `style` functions, each of which is optional.
-
-Each `markup`, `script` or `style` function must return an object (or a Promise that resolves to an object) with a `code` property, representing the transformed source code, and an optional array of `dependencies`.
+The first argument is the component source code. The second is an array of *preprocessors* (or a single preprocessor, if you only have one), where a preprocessor is an object with a `name` which is required, and `markup`, `script` and `style` functions, each of which is optional.
 
 The `markup` function receives the entire component source text, along with the component's `filename` if it was specified in the third argument.
 
-> Preprocessor functions should additionally return a `map` object alongside `code` and `dependencies`, where `map` is a sourcemap representing the transformation.
+The `script` and `style` functions receive the contents of `<script>` and `<style>` elements respectively (`content`) as well as the entire component source text (`markup`). In addition to `filename`, they get an object of the element's attributes.
+
+Each `markup`, `script` or `style` function must return an object (or a Promise that resolves to an object) with a `code` property, representing the transformed source code. Optionally they can return an array of `dependencies` which represents files to watch for changes, and a `map` object which is a sourcemap mapping back the transformation to the original code. `script` and `style` preprocessors can optionally return a record of attributes which represent the updated attributes on the script/style tag.
+
+> Preprocessor functions should return a `map` object whenever possible or else debugging becomes harder as stack traces can't link to the original code correctly.
 
 ```js
-const svelte = require('svelte/compiler');
-const MagicString = require('magic-string');
+import { preprocess } from 'svelte/compiler';
+import MagicString from 'magic-string';
+import sass from 'sass';
+import { dirname } from 'path';
 
-const { code } = await svelte.preprocess(source, {
+const { code } = await preprocess(source, {
+	name: 'my-fancy-preprocessor',
 	markup: ({ content, filename }) => {
+		// Return code as is when no foo string present
 		const pos = content.indexOf('foo');
 		if(pos < 0) {
-			return { code: content }
+			return;
 		}
-		const s = new MagicString(content, { filename })
-		s.overwrite(pos, pos + 3, 'bar', { storeName: true })
+
+		// Replace foo with bar using MagicString which provides
+		// a source map along with the changed code
+		const s = new MagicString(content, { filename });
+		s.overwrite(pos, pos + 3, 'bar', { storeName: true });
+
 		return {
 			code: s.toString(),
-			map: s.generateMap()
+			map: s.generateMap({ hires: true, file: filename })
 		}
-	}
-}, {
-	filename: 'App.svelte'
-});
-```
-
----
-
-The `script` and `style` functions receive the contents of `<script>` and `<style>` elements respectively (`content`) as well as the entire component source text (`markup`). In addition to `filename`, they get an object of the element's attributes.
-
-If a `dependencies` array is returned, it will be included in the result object. This is used by packages like [rollup-plugin-svelte](https://github.com/sveltejs/rollup-plugin-svelte) to watch additional files for changes, in the case where your `<style>` tag has an `@import` (for example).
-
-```js
-const svelte = require('svelte/compiler');
-const sass = require('node-sass');
-const { dirname } = require('path');
-
-const { code, dependencies } = await svelte.preprocess(source, {
+	},
 	style: async ({ content, attributes, filename }) => {
 		// only process <style lang="sass">
 		if (attributes.lang !== 'sass') return;
@@ -277,9 +271,13 @@ const { code, dependencies } = await svelte.preprocess(source, {
 			else resolve(result);
 		}));
 
+		// remove lang attribute from style tag
+		delete attributes.lang;
+
 		return {
 			code: css.toString(),
-			dependencies: stats.includedFiles
+			dependencies: stats.includedFiles,
+			attributes
 		};
 	}
 }, {
@@ -289,29 +287,33 @@ const { code, dependencies } = await svelte.preprocess(source, {
 
 ---
 
-Multiple preprocessors can be used together. The output of the first becomes the input to the second. `markup` functions run first, then `script` and `style`.
+Multiple preprocessors can be used together. The output of the first becomes the input to the second. Within one preprocessor, `markup` runs first, then `script` and `style`.
+
+> In Svelte 3, all `markup` functions ran first, then all `script` and then all `style` preprocessors. This order was changed in Svelte 4.
 
 ```js
 const svelte = require('svelte/compiler');
 
 const { code } = await svelte.preprocess(source, [
 	{
+		name: 'first preprocessor',
 		markup: () => {
 			console.log('this runs first');
 		},
 		script: () => {
-			console.log('this runs third');
+			console.log('this runs second');
 		},
 		style: () => {
-			console.log('this runs fifth');
+			console.log('this runs third');
 		}
 	},
 	{
+		name: 'second preprocessor',
 		markup: () => {
-			console.log('this runs second');
+			console.log('this runs fourth');
 		},
 		script: () => {
-			console.log('this runs fourth');
+			console.log('this runs fifth');
 		},
 		style: () => {
 			console.log('this runs sixth');

src/compiler/preprocess/index.js

@@ -7,7 +7,6 @@ import {
 } from '../utils/mapped_code.js';
 import { decode_map } from './decode_sourcemap.js';
 import { replace_in_code, slice_source } from './replace_in_code.js';
-import { regex_whitespaces } from '../utils/patterns.js';
 
 const regex_filepath_separator = /[/\\]/;
 
@@ -132,11 +131,18 @@ function processed_content_to_code(processed, location, file_basename) {
  * representing the tag content replaced with `processed`.
  * @param {import('./public.js').Processed} processed
  * @param {'style' | 'script'} tag_name
- * @param {string} attributes
+ * @param {string} original_attributes
+ * @param {string} generated_attributes
  * @param {import('./private.js').Source} source
  * @returns {MappedCode}
  */
-function processed_tag_to_code(processed, tag_name, attributes, source) {
+function processed_tag_to_code(
+	processed,
+	tag_name,
+	original_attributes,
+	generated_attributes,
+	source
+) {
 	const { file_basename, get_location } = source;
 
 	/**
@@ -145,34 +151,105 @@ function processed_tag_to_code(processed, tag_name, attributes, source) {
 	 */
 	const build_mapped_code = (code, offset) =>
 		MappedCode.from_source(slice_source(code, offset, source));
-	const tag_open = `<${tag_name}${attributes || ''}>`;
+
+	// To map the open/close tag and content starts positions correctly, we need to
+	// differentiate between the original attributes and the generated attributes:
+	// `source` contains the original attributes and its get_location maps accordingly.
+	const original_tag_open = `<${tag_name}${original_attributes}>`;
+	const tag_open = `<${tag_name}${generated_attributes}>`;
+	/** @type {MappedCode} */
+	let tag_open_code;
+
+	if (original_tag_open.length !== tag_open.length) {
+		// Generate a source map for the open tag
+		/** @type {import('@ampproject/remapping').DecodedSourceMap['mappings']} */
+		const mappings = [
+			[
+				// start of tag
+				[0, 0, 0, 0],
+				// end of tag start
+				[`<${tag_name}`.length, 0, 0, `<${tag_name}`.length]
+			]
+		];
+
+		const line = tag_open.split('\n').length - 1;
+		const column = tag_open.length - (line === 0 ? 0 : tag_open.lastIndexOf('\n')) - 1;
+
+		while (mappings.length <= line) {
+			// end of tag start again, if this is a multi line mapping
+			mappings.push([[0, 0, 0, `<${tag_name}`.length]]);
+		}
+
+		// end of tag
+		mappings[line].push([
+			column,
+			0,
+			original_tag_open.split('\n').length - 1,
+			original_tag_open.length - original_tag_open.lastIndexOf('\n') - 1
+		]);
+
+		/** @type {import('@ampproject/remapping').DecodedSourceMap} */
+		const map = {
+			version: 3,
+			names: [],
+			sources: [file_basename],
+			mappings
+		};
+		sourcemap_add_offset(map, get_location(0), 0);
+		tag_open_code = MappedCode.from_processed(tag_open, map);
+	} else {
+		tag_open_code = build_mapped_code(tag_open, 0);
+	}
+
 	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.source.length);
+	const tag_close_code = build_mapped_code(
+		tag_close,
+		original_tag_open.length + source.source.length
+	);
+
 	parse_attached_sourcemap(processed, tag_name);
 	const content_code = processed_content_to_code(
 		processed,
-		get_location(tag_open.length),
+		get_location(original_tag_open.length),
 		file_basename
 	);
+
 	return tag_open_code.concat(content_code).concat(tag_close_code);
 }
-const regex_quoted_value = /^['"](.*)['"]$/;
+
+const attribute_pattern = /([\w-$]+\b)(?:=(?:"([^"]*)"|'([^']*)'|(\S+)))?/g;
 
 /**
  * @param {string} str
  */
 function parse_tag_attributes(str) {
-	// note: won't work with attribute values containing spaces.
-	return str
-		.split(regex_whitespaces)
-		.filter(Boolean)
-		.reduce((attrs, attr) => {
-			const i = attr.indexOf('=');
-			const [key, value] = i > 0 ? [attr.slice(0, i), attr.slice(i + 1)] : [attr];
-			const [, unquoted] = (value && value.match(regex_quoted_value)) || [];
-			return { ...attrs, [key]: unquoted ?? value ?? true };
-		}, {});
+	/** @type {Record<string, string | boolean>} */
+	const attrs = {};
+
+	/** @type {RegExpMatchArray} */
+	let match;
+	while ((match = attribute_pattern.exec(str)) !== null) {
+		const name = match[1];
+		const value = match[2] || match[3] || match[4];
+		attrs[name] = !value || value;
+	}
+
+	return attrs;
+}
+
+/**
+ * @param {Record<string, string | boolean> | undefined} attributes
+ */
+function stringify_tag_attributes(attributes) {
+	if (!attributes) return;
+
+	let value = Object.entries(attributes)
+		.map(([key, value]) => (value === true ? key : `${key}="${value}"`))
+		.join(' ');
+	if (value) {
+		value = ' ' + value;
+	}
+	return value;
 }
 
 const regex_style_tags = /<!--[^]*?-->|<style(\s[^]*?)?(?:>([^]*?)<\/style>|\/>)/gi;
@@ -216,6 +293,7 @@ async function process_tag(tag_name, preprocessor, source) {
 			processed,
 			tag_name,
 			attributes,
+			stringify_tag_attributes(processed.attributes) ?? attributes,
 			slice_source(content, tag_offset, source)
 		);
 	}
@@ -264,20 +342,21 @@ export default async function preprocess(source, preprocessor, options) {
 			? preprocessor
 			: [preprocessor]
 		: [];
-	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 result = new PreprocessResult(source, filename);
+
 	// TODO keep track: what preprocessor generated what sourcemap?
 	// to make debugging easier = detect low-resolution sourcemaps in fn combine_mappings
-	for (const process of markup) {
-		result.update_source(await process_markup(process, result));
-	}
-	for (const process of script) {
-		result.update_source(await process_tag('script', process, result));
-	}
-	for (const preprocess of style) {
-		result.update_source(await process_tag('style', preprocess, result));
+	for (const preprocessor of preprocessors) {
+		if (preprocessor.markup) {
+			result.update_source(await process_markup(preprocessor.markup, result));
+		}
+		if (preprocessor.script) {
+			result.update_source(await process_tag('script', preprocessor.script, result));
+		}
+		if (preprocessor.style) {
+			result.update_source(await process_tag('style', preprocessor.style, result));
+		}
 	}
+
 	return result.to_processed();
 }