ronantakizawa/codereview-bench huggingface dataset:
Repository: sveltejs/svelte
File: packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js (JavaScript)

Code before:
/** @import { AssignmentExpression, Expression, ExpressionStatement, Identifier, MemberExpression, SequenceExpression, Literal, Super, UpdateExpression } from 'estree' */
/** @import { AST, ExpressionMetadata } from '#compiler' */
/** @import { ComponentClientTransformState, Context } from '../../types' */
import { walk } from 'zimmerframe';
import { object } from '../../../../../utils/ast.js';
import * as b from '../../../../../utils/builders.js';
import { sanitize_template_string } from '../../../../../utils/sanitize_template_string.js';
import { regex_is_valid_identifier } from '../../../../patterns.js';
import is_reference from 'is-reference';
import { dev, is_ignored, locator } from '../../../../../state.js';
import { create_derived } from '../../utils.js';

/**
 * @param {ComponentClientTransformState} state
 * @param {Expression} value
 */
export function memoize_expression(state, value) {
	const id = b.id(state.scope.generate('expression'));
	state.init.push(b.const(id, create_derived(state, b.thunk(value))));
	return b.call('$.get', id);
}

/**
 *
 * @param {ComponentClientTransformState} state
 * @param {Expression} value
 */
export function get_expression_id(state, value) {
	return b.id(`$${state.expressions.push(value) - 1}`);
}


Review comment (suggestion):
```suggestion
import * as b from '#compiler/builders';
```

Code after:
/** @import { AssignmentExpression, Expression, ExpressionStatement, Identifier, MemberExpression, SequenceExpression, Literal, Super, UpdateExpression } from 'estree' */
/** @import { AST, ExpressionMetadata } from '#compiler' */
/** @import { ComponentClientTransformState, Context } from '../../types' */
import { walk } from 'zimmerframe';
import { object } from '../../../../../utils/ast.js';
import * as b from '#compiler/builders';
import { sanitize_template_string } from '../../../../../utils/sanitize_template_string.js';
import { regex_is_valid_identifier } from '../../../../patterns.js';
import is_reference from 'is-reference';
import { dev, is_ignored, locator } from '../../../../../state.js';
import { create_derived } from '../../utils.js';

/**
 * @param {ComponentClientTransformState} state
 * @param {Expression} value
 */
export function memoize_expression(state, value) {
	const id = b.id(state.scope.generate('expression'));
	state.init.push(b.const(id, create_derived(state, b.thunk(value))));
	return b.call('$.get', id);
}

/**
 *
 * @param {ComponentClientTransformState} state
 * @param {Expression} value
 */
export function get_expression_id(state, value) {
	return b.id(`$${state.expressions.push(value) - 1}`);
}
Repository: sveltejs/svelte
File: packages/svelte/src/internal/client/dom/elements/events.js (JavaScript)

Code before:
export function on(element, type, handler, options = {}) {
	var target_handler = create_event(type, element, handler, options);

	return () => {
		element.removeEventListener(type, target_handler, options);
	};
}

/**
 * @param {string} event_name
 * @param {Element} dom
 * @param {EventListener} [handler]
 * @param {boolean} [capture]
 * @param {boolean} [passive]
 * @returns {void}
 */
export function event(event_name, dom, handler, capture, passive) {
	var options = { capture, passive };
	var target_handler = create_event(event_name, dom, handler, options);

	if (
		dom === document.body ||
		// @ts-ignore
		dom === window ||
		// @ts-ignore
		dom === document ||
		dom instanceof HTMLMediaElement
	) {
		teardown(() => {
			dom.removeEventListener(event_name, target_handler, options);
		});
	}
}

/**
 * @param {Array<string>} events
 * @returns {void}
 */
export function delegate(events) {
	for (var i = 0; i < events.length; i++) {
		all_registered_events.add(events[i]);
	}

	for (var fn of root_event_handles) {
		fn(events);
	}
}

/**
 * @this {EventTarget}
 * @param {Event} event

Review comment (suggestion):
```suggestion
		dom === document ||
		// Firefox has quirky behavior, it can happen that we still get "canplay" events when the element is already removed
```

Code after:
export function on(element, type, handler, options = {}) {
	var target_handler = create_event(type, element, handler, options);

	return () => {
		element.removeEventListener(type, target_handler, options);
	};
}

/**
 * @param {string} event_name
 * @param {Element} dom
 * @param {EventListener} [handler]
 * @param {boolean} [capture]
 * @param {boolean} [passive]
 * @returns {void}
 */
export function event(event_name, dom, handler, capture, passive) {
	var options = { capture, passive };
	var target_handler = create_event(event_name, dom, handler, options);

	if (
		dom === document.body ||
		// @ts-ignore
		dom === window ||
		// @ts-ignore
		dom === document ||
		// Firefox has quirky behavior, it can happen that we still get "canplay" events when the element is already removed
		dom instanceof HTMLMediaElement
	) {
		teardown(() => {
			dom.removeEventListener(event_name, target_handler, options);
		});
	}
}

/**
 * @param {Array<string>} events
 * @returns {void}
 */
export function delegate(events) {
	for (var i = 0; i < events.length; i++) {
		all_registered_events.add(events[i]);
	}

	for (var fn of root_event_handles) {
		fn(events);
	}
}

/**
 * @this {EventTarget}
Repository: sveltejs/svelte
File: packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js (JavaScript)

Code before:
										attribute.type === 'SpreadAttribute'
								),
								context
							),
							'value',
							false,
							true
						)
					)
				)
			);
		} else if (value) {
			select_with_value = true;
			const left = b.id('$$payload.select_value');
			if (value.type === 'Attribute') {
				state.template.push(
					b.stmt(b.assignment('=', left, build_attribute_value(value.value, context)))
				);
			} else if (value.type === 'BindDirective') {
				state.template.push(
					b.stmt(
						b.assignment(
							'=',
							left,
							value.expression.type === 'SequenceExpression'
								? b.call(value.expression.expressions[0])
								: /** @type {Expression} */ (context.visit(value.expression))
						)
					)
				);
			}
		}
	}

	if (
		node.name === 'option' &&
		!node.attributes.some(
			(attribute) =>
				attribute.type === 'SpreadAttribute' ||
				((attribute.type === 'Attribute' || attribute.type === 'BindDirective') &&
					attribute.name === 'value')
		)
	) {
		const inner_state = { ...state, template: [], init: [] };
		process_children(trimmed, { ...context, state: inner_state });

		state.template.push(
			b.stmt(
				b.call(
					'$.valueless_option',
					b.id('$$payload'),

Review comment (suggestion):
same needs to happen for function bindings

```suggestion
								? /** @type {Expression} */ (context.visit(b.call(value.expression.expressions[0])))
```

Code after:
										attribute.type === 'SpreadAttribute'
								),
								context
							),
							'value',
							false,
							true
						)
					)
				)
			);
		} else if (value) {
			select_with_value = true;
			const left = b.id('$$payload.select_value');
			if (value.type === 'Attribute') {
				state.template.push(
					b.stmt(b.assignment('=', left, build_attribute_value(value.value, context)))
				);
			} else if (value.type === 'BindDirective') {
				state.template.push(
					b.stmt(
						b.assignment(
							'=',
							left,
							value.expression.type === 'SequenceExpression'
								? /** @type {Expression} */ (context.visit(b.call(value.expression.expressions[0])))
								: /** @type {Expression} */ (context.visit(value.expression))
						)
					)
				);
			}
		}
	}

	if (
		node.name === 'option' &&
		!node.attributes.some(
			(attribute) =>
				attribute.type === 'SpreadAttribute' ||
				((attribute.type === 'Attribute' || attribute.type === 'BindDirective') &&
					attribute.name === 'value')
		)
	) {
		const inner_state = { ...state, template: [], init: [] };
		process_children(trimmed, { ...context, state: inner_state });

		state.template.push(
			b.stmt(
				b.call(
					'$.valueless_option',
					b.id('$$payload'),
Repository: sveltejs/svelte
File: packages/svelte/tests/html_equal.js (JavaScript)

Code before:
 * @param {{ preserveComments: boolean }} opts
 */
function clean_children(node, opts) {
	let previous = null;
	let has_element_children = false;
	let template =
		node.nodeName === 'TEMPLATE' ? /** @type {HTMLTemplateElement} */ (node) : undefined;

	if (template) {
		const div = document.createElement('div');
		div.append(template.content);
		node = div;
	}

	// sort attributes
	const attributes = Array.from(node.attributes).sort((a, b) => {
		return a.name < b.name ? -1 : 1;
	});

	attributes.forEach((attr) => {
		node.removeAttribute(attr.name);
	});

	attributes.forEach((attr) => {
		if ((attr.name === 'onload' || attr.name === 'onerror') && attr.value === 'this.__e=event') {
			return;
		}

		node.setAttribute(attr.name, attr.value);
	});

	for (let child of [...node.childNodes]) {
		if (child.nodeType === 3) {
			let text = /** @type {Text} */ (child);

			if (
				node.namespaceURI === 'http://www.w3.org/2000/svg' &&
				node.tagName !== 'text' &&
				node.tagName !== 'tspan'
			) {
				node.removeChild(child);
				continue;
			}

			text.data = text.data.replace(/[^\S]+/g, ' ');

			if (previous && previous.nodeType === 3) {
				const prev = /** @type {Text} */ (previous);

				prev.data += text.data;
				node.removeChild(text);

Review comment (suggestion):
```suggestion
		// Strip out the special onload/onerror hydration events from the test output
		if ((attr.name === 'onload' || attr.name === 'onerror') && attr.value === 'this.__e=event') {
```

Code after:
 * @param {{ preserveComments: boolean }} opts
 */
function clean_children(node, opts) {
	let previous = null;
	let has_element_children = false;
	let template =
		node.nodeName === 'TEMPLATE' ? /** @type {HTMLTemplateElement} */ (node) : undefined;

	if (template) {
		const div = document.createElement('div');
		div.append(template.content);
		node = div;
	}

	// sort attributes
	const attributes = Array.from(node.attributes).sort((a, b) => {
		return a.name < b.name ? -1 : 1;
	});

	attributes.forEach((attr) => {
		node.removeAttribute(attr.name);
	});

	attributes.forEach((attr) => {
		// Strip out the special onload/onerror hydration events from the test output
		if ((attr.name === 'onload' || attr.name === 'onerror') && attr.value === 'this.__e=event') {
			return;
		}

		node.setAttribute(attr.name, attr.value);
	});

	for (let child of [...node.childNodes]) {
		if (child.nodeType === 3) {
			let text = /** @type {Text} */ (child);

			if (
				node.namespaceURI === 'http://www.w3.org/2000/svg' &&
				node.tagName !== 'text' &&
				node.tagName !== 'tspan'
			) {
				node.removeChild(child);
				continue;
			}

			text.data = text.data.replace(/[^\S]+/g, ' ');

			if (previous && previous.nodeType === 3) {
				const prev = /** @type {Text} */ (previous);

				prev.data += text.data;
Repository: sveltejs/svelte
File: packages/svelte/tests/html_equal.js (JavaScript)

Code before:
			}

			if (child.nodeType === 1) {
				has_element_children = true;
				clean_children(/** @type {Element} */ (child), opts);
			}
		}

		previous = child;
	}

	// collapse whitespace
	if (node.firstChild && node.firstChild.nodeType === 3) {
		const text = /** @type {Text} */ (node.firstChild);
		text.data = text.data.trimStart();
	}

	if (node.lastChild && node.lastChild.nodeType === 3) {
		const text = /** @type {Text} */ (node.lastChild);
		text.data = text.data.trimEnd();
	}

	if (has_element_children && node.parentNode) {
		node.innerHTML = `\n\  ${node.innerHTML.replace(/\n/g, '\n  ')}\n`;
	}

	if (template) {
		template.innerHTML = node.innerHTML;
	}
}

/**
 * @param {Window} window
 * @param {string} html
 * @param {{ preserveComments?: boolean }} opts
 */
export function normalize_html(window, html, { preserveComments = false } = {}) {
	try {
		const node = window.document.createElement('div');

		node.innerHTML = html.trim();
		clean_children(node, { preserveComments });

		return node.innerHTML;
	} catch (err) {
		throw new Error(`Failed to normalize HTML:\n${html}\nCause: ${err}`);
	}
}

/**
 * @param {string} html

Review comment (suggestion):
```suggestion
	// indent code for better readability
	if (has_element_children && node.parentNode) {
```

Code after:
			} else if (previous?.nodeType === 1 || previous?.nodeType === 8) {
				node.insertBefore(document.createTextNode('\n'), child);
			}

			if (child.nodeType === 1) {
				has_element_children = true;
				clean_children(/** @type {Element} */ (child), opts);
			}
		}

		previous = child;
	}

	// collapse whitespace
	if (node.firstChild && node.firstChild.nodeType === 3) {
		const text = /** @type {Text} */ (node.firstChild);
		text.data = text.data.trimStart();
	}

	if (node.lastChild && node.lastChild.nodeType === 3) {
		const text = /** @type {Text} */ (node.lastChild);
		text.data = text.data.trimEnd();
	}

	// indent code for better readability
	if (has_element_children && node.parentNode) {
		node.innerHTML = `\n\  ${node.innerHTML.replace(/\n/g, '\n  ')}\n`;
	}

	if (template) {
		template.innerHTML = node.innerHTML;
	}
}

/**
 * @param {Window} window
 * @param {string} html
 * @param {{ preserveComments?: boolean }} opts
 */
export function normalize_html(window, html, { preserveComments = false } = {}) {
	try {
		const node = window.document.createElement('div');

		node.innerHTML = html.trim();
		clean_children(node, { preserveComments });

		return node.innerHTML;
	} catch (err) {
		throw new Error(`Failed to normalize HTML:\n${html}\nCause: ${err}`);
	}
}
Repository: sveltejs/svelte
File: packages/svelte/tests/html_equal.js (JavaScript)

Code before:
			}

			text.data = text.data.replace(/[^\S]+/g, ' ');

			if (previous && previous.nodeType === 3) {
				const prev = /** @type {Text} */ (previous);

				prev.data += text.data;
				node.removeChild(text);

				text = prev;
				text.data = text.data.replace(/[^\S]+/g, ' ');

				continue;
			}
		}

		if (child.nodeType === 8 && !opts.preserveComments) {
			// comment
			child.remove();
			continue;
		}

		if (child.nodeType === 1 || child.nodeType === 8) {
			if (previous?.nodeType === 3) {
				const prev = /** @type {Text} */ (previous);
				prev.data = prev.data.replace(/^[^\S]+$/, '\n');
			} else if (previous?.nodeType === 1 || previous?.nodeType === 8) {
				node.insertBefore(document.createTextNode('\n'), child);
			}

			if (child.nodeType === 1) {
				has_element_children = true;
				clean_children(/** @type {Element} */ (child), opts);
			}
		}

		previous = child;
	}

	// collapse whitespace
	if (node.firstChild && node.firstChild.nodeType === 3) {
		const text = /** @type {Text} */ (node.firstChild);
		text.data = text.data.trimStart();
	}

	if (node.lastChild && node.lastChild.nodeType === 3) {
		const text = /** @type {Text} */ (node.lastChild);
		text.data = text.data.trimEnd();
	}


Review comment (suggestion):
```suggestion
		// add newlines for better readability and potentially recurse into children
		if (child.nodeType === 1 || child.nodeType === 8) {
```

Code after:
				continue;
			}

			text.data = text.data.replace(/[^\S]+/g, ' ');

			if (previous && previous.nodeType === 3) {
				const prev = /** @type {Text} */ (previous);

				prev.data += text.data;
				node.removeChild(text);

				text = prev;
				text.data = text.data.replace(/[^\S]+/g, ' ');

				continue;
			}
		}

		if (child.nodeType === 8 && !opts.preserveComments) {
			// comment
			child.remove();
			continue;
		}

		// add newlines for better readability and potentially recurse into children
		if (child.nodeType === 1 || child.nodeType === 8) {
			if (previous?.nodeType === 3) {
				const prev = /** @type {Text} */ (previous);
				prev.data = prev.data.replace(/^[^\S]+$/, '\n');
			} else if (previous?.nodeType === 1 || previous?.nodeType === 8) {
				node.insertBefore(document.createTextNode('\n'), child);
			}

			if (child.nodeType === 1) {
				has_element_children = true;
				clean_children(/** @type {Element} */ (child), opts);
			}
		}

		previous = child;
	}

	// collapse whitespace
	if (node.firstChild && node.firstChild.nodeType === 3) {
		const text = /** @type {Text} */ (node.firstChild);
		text.data = text.data.trimStart();
	}

	if (node.lastChild && node.lastChild.nodeType === 3) {
		const text = /** @type {Text} */ (node.lastChild);
		text.data = text.data.trimEnd();
Repository: sveltejs/svelte
File: packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js (JavaScript)

Code before:
		b.array(path),
		expression,
		loc && b.literal(loc.line),
		loc && b.literal(loc.column)
	);
}

/**
 *
 * @param {ComponentContext} context
 * @param {Expression} expression
 * @param {ExpressionMetadata} metadata
 */
export function build_expression(context, expression, metadata, state = context.state) {
	const value = /** @type {Expression} */ (context.visit(expression, state));

	if (context.state.analysis.runes) {
		return value;
	}

	if (!metadata.has_call && !metadata.has_member_expression && !metadata.has_assignment) {
		return value;
	}

	const sequence = b.sequence([]);

	for (const binding of metadata.references) {
		if (binding.kind === 'normal' && binding.declaration_kind !== 'import') {
			continue;
		}

		var getter = build_getter({ ...binding.node }, state);

		if (
			binding.kind === 'bindable_prop' ||
			binding.kind === 'template' ||
			binding.declaration_kind === 'import' ||
			binding.node.name === '$$props' ||
			binding.node.name === '$$restProps'
		) {
			getter = b.call('$.deep_read_state', getter);
		}

		sequence.expressions.push(getter);
	}

	sequence.expressions.push(b.call('$.untrack', b.thunk(value)));

	return sequence;
}


Review comment (suggestion):
```suggestion
	// Legacy reactivity is coarse-grained, looking at the statically visible dependencies. Replicate that here
	const sequence = b.sequence([]);
```

Code after:
		b.array(path),
		expression,
		loc && b.literal(loc.line),
		loc && b.literal(loc.column)
	);
}

/**
 *
 * @param {ComponentContext} context
 * @param {Expression} expression
 * @param {ExpressionMetadata} metadata
 */
export function build_expression(context, expression, metadata, state = context.state) {
	const value = /** @type {Expression} */ (context.visit(expression, state));

	if (context.state.analysis.runes) {
		return value;
	}

	if (!metadata.has_call && !metadata.has_member_expression && !metadata.has_assignment) {
		return value;
	}

	// Legacy reactivity is coarse-grained, looking at the statically visible dependencies. Replicate that here
	const sequence = b.sequence([]);

	for (const binding of metadata.references) {
		if (binding.kind === 'normal' && binding.declaration_kind !== 'import') {
			continue;
		}

		var getter = build_getter({ ...binding.node }, state);

		if (
			binding.kind === 'bindable_prop' ||
			binding.kind === 'template' ||
			binding.declaration_kind === 'import' ||
			binding.node.name === '$$props' ||
			binding.node.name === '$$restProps'
		) {
			getter = b.call('$.deep_read_state', getter);
		}

		sequence.expressions.push(getter);
	}

	sequence.expressions.push(b.call('$.untrack', b.thunk(value)));

	return sequence;
}
Repository: sveltejs/svelte
File: packages/svelte/src/internal/client/runtime.js (JavaScript)

Code before:
}

/**
 * Synchronously flush any pending updates.
 * Returns void if no callback is provided, otherwise returns the result of calling the callback.
 * @template [T=void]
 * @param {(() => T) | undefined} [fn]
 * @returns {T}
 */
export function flushSync(fn) {
	var result;

	if (fn) {
		is_flushing = true;
		flush_queued_root_effects();

		is_flushing = true;
		result = fn();
	}

	while (true) {
		flush_tasks();

		if (queued_root_effects.length === 0) {
			// this would be reset in `flush_queued_root_effects` but since we are early returning here,
			// we need to reset it here as well
			is_flushing = false;
			last_scheduled_effect = null;
			if (DEV) {
				dev_effect_stack = [];
			}
			return /** @type {T} */ (result);
		}

		is_flushing = true;
		flush_queued_root_effects();
	}
}

/**
 * Returns a promise that resolves once any pending state changes have been applied.
 * @returns {Promise<void>}
 */
export async function tick() {
	await Promise.resolve();
	// By calling flushSync we guarantee that any pending state changes are applied after one tick.
	// TODO look into whether we can make flushing subsequent updates synchronously in the future.
	flushSync();
}

/**

Review comment (suggestion):
```suggestion
			// we need to reset it here as well in case the first time there's 0 queued root effects
```

Code after:
}

/**
 * Synchronously flush any pending updates.
 * Returns void if no callback is provided, otherwise returns the result of calling the callback.
 * @template [T=void]
 * @param {(() => T) | undefined} [fn]
 * @returns {T}
 */
export function flushSync(fn) {
	var result;

	if (fn) {
		is_flushing = true;
		flush_queued_root_effects();

		is_flushing = true;
		result = fn();
	}

	while (true) {
		flush_tasks();

		if (queued_root_effects.length === 0) {
			// this would be reset in `flush_queued_root_effects` but since we are early returning here,
			// we need to reset it here as well in case the first time there's 0 queued root effects
			is_flushing = false;
			last_scheduled_effect = null;
			if (DEV) {
				dev_effect_stack = [];
			}
			return /** @type {T} */ (result);
		}

		is_flushing = true;
		flush_queued_root_effects();
	}
}

/**
 * Returns a promise that resolves once any pending state changes have been applied.
 * @returns {Promise<void>}
 */
export async function tick() {
	await Promise.resolve();
	// By calling flushSync we guarantee that any pending state changes are applied after one tick.
	// TODO look into whether we can make flushing subsequent updates synchronously in the future.
	flushSync();
}

/**