Skip to content

[fix] whitespace html entities lost in component slot #8464

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 11, 2023
Merged

[fix] whitespace html entities lost in component slot #8464

merged 1 commit into from
Apr 11, 2023

Conversation

@xxkl1
Copy link
Contributor

@xxkl1 xxkl1 commented Apr 8, 2023 •
edited
Loading

closes: #8359

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

The reason is due to the following code

svelte/src/compiler/compile/nodes/InlineComponent.ts

Lines 170 to 172 in d42ca04

function not_whitespace_text(node) {
return !(node.type === 'Text' && regex_only_whitespaces.test(node.data));
}

svelte/src/compiler/utils/patterns.ts

Line 7 in d42ca04

export const regex_only_whitespaces = /^\s+$/;

  is non-breaking space characte, /\s/ match it and judging that it should not be retained.

&nbsp; in <span>&nbsp;</span> use [ \t\r\n] so it will not be delete.

<Component>
	<span>&nbsp;</span>
</Component>

svelte/src/compiler/compile/render_dom/wrappers/Fragment.ts

Lines 102 to 104 in d42ca04

if (should_trim && !child.keep_space()) {
data = trim_end(data);
if (!data) continue;

svelte/src/compiler/utils/trim.ts

Lines 7 to 9 in d42ca04

export function trim_end(str: string) {
return str.replace(regex_ends_with_whitespaces, '');
}

svelte/src/compiler/utils/patterns.ts

Line 6 in d42ca04

export const regex_ends_with_whitespaces = /[ \t\r\n]*$/;

/\s/ is looks equal with /[ \u00A0\u2009\u200C\u200D\u3000\t\n\r\f]/, \u00A0\u2009\u200C\u200D\u3000 is unicode code of \&nbsp; \&thinsp \&zwnj; \&zwj; &emsp;.

I think we can use [ \t\n\r\f] equals with [\u0020\t\n\r\f] to solve this problem, \u0020 is standard space character we need skip.

The test error of html space entities will be hided by /\s/ in normalizeHtml. The reason is html space entities be replace to '', so /\s/ need to replaced by [ \t\n\r\f] and save html space entities in test too.

svelte/test/helpers.ts

Lines 143 to 155 in d42ca04

export function normalizeHtml(window, html, preserveComments = false) {
try {
const node = window.document.createElement('div');
node.innerHTML = html
.replace(/(<!--.*?-->)/g, preserveComments ? '$1' : '')
.replace(/>[\s\r\n]+</g, '><')
.trim();
cleanChildren(node);
return node.innerHTML.replace(/<\/?noscript\/?>/g, '');
} catch (err) {
throw new Error(`Failed to normalize HTML:\n${html}`);
}
}

@vercel
Copy link

vercel bot commented Apr 8, 2023

@xxkl1 is attempting to deploy a commit to the Svelte Team on Vercel.

A member of the Team first needs to authorize it.

@xxkl1 xxkl1 force-pushed the fix/html-entities-inside-component-slot branch from 8b14f49 to 71aa09f Compare April 9, 2023 13:32
@dummdidumm dummdidumm added this to the 4.x milestone Apr 11, 2023
@dummdidumm dummdidumm changed the base branch from master to version-4 April 11, 2023 13:20
@dummdidumm dummdidumm merged commit 765023d into sveltejs:version-4 Apr 11, 2023
@dummdidumm
Copy link
Member

dummdidumm commented Apr 11, 2023

Thank you!

dummdidumm pushed a commit that referenced this pull request Apr 18, 2023
@xxkl1 @dummdidumm
fix: html space entities lost in component slot (#8464)
b95ae0e 
fixes #8359
@gtm-nayan gtm-nayan mentioned this pull request Jun 17, 2023
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

If slot content consists only of &nbsp, it is not passed to component

2 participants

@xxkl1 @dummdidumm