Allow self-closing /> on HTML via a flag or feature

[phoenix-ru]

Syntax like that is often used in Vue templates:

<template>
  <slot />
  <div class="bad" />
</template>

Since swc_html_parser is spec-compliant, it acknowledges the <slot /> and <div />, but does not actually pop the DOM Tree stack.
It makes parsing such templates impossible: phoenix-ru/fervid#6

Can we add a flag to ParserConfig or a crate feature to treat such starting tags as self-closing? I can open a ticket and work on a PR.

[kdy1]

@alexander-akait Thoughts?

[phoenix-ru]

Also invalid as per Nu HTML checker

[phoenix-ru]

<unknown>text</unknown> is VALID (regular html element)
<unknown /> is VALID (in Vue; proposed change to SWC)
<br> is VALID (it's a void tag)
<br>text</br> is INVALID (void tag, cannot close it)

[phoenix-ru]

I initially noticed that SWC has a is_self_closing attribute
https://rustdoc.swc.rs/swc_html_ast/struct.Element.html

Which is set to true for <div />.

[alexander-akait]

Okay, sounds good for me, do you want to send a PR? I will review to avoid regression in other places (but yeah, we have a lot of tests)

[phoenix-ru]

@alexander-akait Great then, I will try to solve it and open a PR.

[phoenix-ru]

Relevant place in Vue docs: https://vuejs.org/guide/essentials/component-basics.html#self-closing-tags.

I managed to get it working for "other" start tags (e.g. <slot />) by adding logic here:

swc/crates/swc_html_parser/src/parser/mod.rs

Lines 4115 to 4118 in c39e75c

	Token::StartTag { .. } => {
	self.reconstruct_active_formatting_elements()?;
	self.insert_html_element(token_and_info)?;
	}

Regarding normal HTML tags like <p>, <b>, <div> and others, I am not really sure how to proceed, since they all have additional spec baggage attached and simply popping the stack would not be correct.

Which is why I think it's better to do this, since it can be neatly organized as a macro and accurately added where necessary:

if self.config.allow_self_closing && *is_self_closing {
    let mut end_token_and_info = TokenAndInfo {
        span: token_and_info.span,
        acknowledged: false,
        token: Token::EndTag {
            tag_name: tag_name.to_owned(),
            raw_tag_name: None,
            is_self_closing: false,
            attributes: vec![],
        },
    };
    self.process_token(&mut end_token_and_info, None)?;
}

To make it possible, I changed the signatures of token_and_info to &TokenAndInfo in these two places, because there is no need for it to be mutable:

swc/crates/swc_html_parser/src/parser/mod.rs

Lines 8337 to 8376 in c39e75c

	fn insert_html_element(&mut self, token_and_info: &mut TokenAndInfo) -> PResult<RcNode> {
	self.insert_foreign_element(token_and_info, Namespace::HTML, None)
	}
	fn insert_foreign_element(
	&mut self,
	token_and_info: &mut TokenAndInfo,
	namespace: Namespace,
	adjust_attributes: Option<AdjustAttributes>,
	) -> PResult<RcNode> {
	// Let the adjusted insertion location be the appropriate place for
	// inserting a node.
	let adjusted_insertion_location = self.get_appropriate_place_for_inserting_node(None)?;
	// Create an element for the token in the given namespace, with the
	// intended parent being the element in which the adjusted insertion
	// location finds itself.
	let node = self.create_element_for_token(
	token_and_info.token.clone(),
	token_and_info.span,
	Some(namespace),
	adjust_attributes,
	);
	// If it is possible to insert an element at the adjusted insertion
	// location, then insert the newly created element at the adjusted
	// insertion location.
	// NOTE: If the adjusted insertion location cannot accept more
	// elements, e.g. because it's a Document that already has an
	// element child, then the newly created element is dropped on the
	// floor.
	self.insert_at_position(adjusted_insertion_location, node.clone());
	// Push the element onto the stack of open elements so that it is the
	// new current node.
	self.open_elements_stack.push(node.clone());
	// Return the newly created element.
	Ok(node)
	}

@alexander-akait Could you please comment on that?

[alexander-akait]

Let's try this solution, I don't remember code deeply, because it was a long time ago, but we will add many tests to ensure we have a right solution

[phoenix-ru]

I discovered a code smell in parser, which always returns Result<T, Error> regardless if Err variant is ever constructed.
The example is this function, which always returns Ok or uses unreachable!. This leads to Results being used fanatically without any reason.

swc/crates/swc_html_parser/src/parser/mod.rs

Lines 8011 to 8134 in c39e75c

	fn get_appropriate_place_for_inserting_node(
	&mut self,
	override_target: Option<RcNode>,
	) -> PResult<InsertionPosition> {
	// 1.
	let target = override_target.unwrap_or_else(|| {
	if let Some(last) = self.open_elements_stack.items.last() {
	last.clone()
	} else {
	// Unreachable, because we always have `html` element on top
	unreachable!();
	}
	});
	// 2.
	let mut adjusted_insertion_location = if self.foster_parenting_enabled
	&& is_html_element!(target, "table" | "tbody" | "tfoot" | "thead" | "tr")
	{
	// 2.1
	let mut last_template = None;
	let mut last_template_index = 0;
	// 2.2
	let mut last_table = None;
	let mut last_table_index = 0;
	for (i, node) in self.open_elements_stack.items.iter().enumerate().rev() {
	if is_html_element!(node, "template") && last_template.is_none() {
	last_template = Some(node);
	last_template_index = i;
	if last_table.is_some() {
	break;
	}
	} else if is_html_element!(node, "table") && last_table.is_none() {
	last_table = Some(node);
	last_table_index = i;
	if last_template.is_some() {
	break;
	}
	}
	}
	// 2.3
	if (last_table.is_none()
	|| (last_table.is_some() && last_template_index > last_table_index))
	&& last_template.is_some()
	{
	let last_template = if let Some(last_template) = last_template {
	last_template.clone()
	} else {
	unreachable!();
	};
	InsertionPosition::LastChild(last_template)
	}
	// 2.4
	// Fragment case
	else if last_table.is_none() && self.open_elements_stack.items.first().is_some() {
	let first = if let Some(first) = self.open_elements_stack.items.first() {
	first.clone()
	} else {
	unreachable!();
	};
	InsertionPosition::LastChild(first)
	}
	// 2.5
	else if match last_table {
	Some(last_table) => {
	let parent = last_table.parent.take();
	let has_parent = parent.is_some();
	last_table.parent.set(parent);
	has_parent
	}
	_ => false,
	} {
	let sibling =
	if let Some(sibling) = self.open_elements_stack.items.get(last_table_index) {
	sibling.clone()
	} else {
	unreachable!()
	};
	InsertionPosition::BeforeSibling(sibling)
	} else {
	// 2.6
	let previous_element = if let Some(previous_element) =
	self.open_elements_stack.items.get(last_table_index - 1)
	{
	previous_element.clone()
	} else {
	unreachable!()
	};
	// 2.7
	InsertionPosition::LastChild(previous_element)
	}
	} else {
	InsertionPosition::LastChild(target)
	};
	// 3.
	adjusted_insertion_location = match &adjusted_insertion_location {
	InsertionPosition::LastChild(node) | InsertionPosition::BeforeSibling(node) => {
	match &node.data {
	Data::Element {
	namespace,
	tag_name,
	..
	} if tag_name == "template" && *namespace == Namespace::HTML => {
	adjusted_insertion_location
	}
	_ => adjusted_insertion_location,
	}
	}
	};
	// 4.
	Ok(adjusted_insertion_location)
	}

I am not touching it now, but I can create an issue (and perhaps tackle it later). Should I?