-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
fix: use valid HTML comment syntax #10978
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
Conversation
|
I'm not sure how useful it would be, but one improvement may be to collapse nested fragments. So, It is something that can be done regardless of whether or not this PR is merged, though. |
Some discussion was happening about this on Discord, and here are a few reasons I thought of for why not to merge this:
|
The new comments also break svgs. I'm using svelte in a weird way where a svelte component is used to render svgs on the server. The output from the 'svelte/server' render function includes the <![> comments. Which make it so the browser won't render the svg. |
So there are two things here:
We don't need to change the first one, maybe only the second. We can likely make it work differently in SVG contexts too, but maybe that's too complicated. I put up a draft PR to revert the changes #10980. |
I'm going to close this in favour of #10980, which changes the SSR output without using long comments in the template strings that are used in the browser to generate document fragments.
There are definitely opportunities for improvement that we plan to implement soon:
Collapsing nested fragments is a possibility but it would require a lot of bookkeeping during SSR and again during hydration — it's probably too much complexity for what it would bring. |
Fixes #10976. While the shorter comment syntax saves a few bytes, it's not standard-compliant. This isn't generally that important, but it doesn't seem like a big enough gain to justify.
Before submitting the PR, please make sure you do the following
feat:
,fix:
,chore:
, ordocs:
.Tests and linting
pnpm test
and lint the project withpnpm lint