Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add logic to select and claim raw html tags in head #6977

Closed
wants to merge 2 commits into from

Conversation

myisaak
Copy link

@myisaak myisaak commented Dec 2, 2021

This PR fixes #6463 with duplicated tags in head (and possibly #6832). Thanks to the helpful investigations from @hbirler #6463 (comment) and from @rmunn #6832 (comment), the problem had to do with the selector logic for claimable elements inside head, that only selected tags with svelte-data prop. I came up with a solution that extended it further to include raw html tags that are between HTML_TAG_START and HTML_TAG_END comments.

Note that for this logic I've added an internal api method called get_hydratable_raw_elements:

export function get_hydratable_raw_elements(parent: HTMLElement = document.body) {
	const elements = Array.from(parent.childNodes);
	let cursor = 0;
	while (cursor < elements.length) {
		const start_index = find_comment(elements, 'HTML_TAG_START', cursor);
		const end_index = find_comment(elements, 'HTML_TAG_END', start_index);
		if (start_index === end_index) {
			elements.splice(cursor, elements.length - cursor);
			break;
		}

		detach(elements[start_index]);
		detach(elements[end_index]);

		elements.splice(cursor, 1 + start_index - cursor);
		cursor += end_index - start_index - 1;
	}
	return elements;
}

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with [feat], [fix], [chore], or [docs].
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with npm test and lint the project with npm run lint

@myisaak myisaak force-pushed the fix-html-tag-hydration branch from fea212e to 8dea38c Compare December 2, 2021 09:40
@myisaak
Copy link
Author

myisaak commented Dec 15, 2021

Ok, I'm assuming the PR needs more validation, since 2 weeks have passed and it's still unmerged. Therefore I'll write a test and include it in this PR.

@myisaak myisaak force-pushed the fix-html-tag-hydration branch from 5029de6 to c221c59 Compare February 16, 2022 21:08
if (start_index === end_index) {
elements.splice(cursor, elements.length - cursor);
break;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happends if you have multiple {@html 'xxx'} in the head?

how woudl you ensure you are not claiming the html tag of other components?

Copy link
Member

@tanhauhau tanhauhau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and yes, please include a test case for this.

@myisaak
Copy link
Author

myisaak commented Apr 16, 2022

I had some trouble writing tests that pass when checking for tags in head. Could you perhaps point me to some documentation? I couldn't find any do far.

@benmccann benmccann changed the title [fix] add logic to select and claim raw html tags in head (#6463) fix: add logic to select and claim raw html tags in head Mar 14, 2023
@benmccann
Copy link
Member

#6463 was fixed by #7745. Is this PR still needed?

@dummdidumm
Copy link
Member

You're right, this should no longer be needed - closing, thank you.

@dummdidumm dummdidumm closed this Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

{@html} hydration in head duplicates elements
4 participants