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 perserveComments on ssr (#4730) #4736

Merged
merged 3 commits into from
Jun 28, 2021

Conversation

confraria
Copy link
Contributor

@confraria confraria commented Apr 27, 2020

Fixes #4730

Should I create a test in server-side-rendering/samples or the existing one in js/samples/ssr-preserve-comments is enough?

Adding the test in server-side-rendering will require an extra config flag to not use assert.htmlEqual on that specific test, as it removes comments.

@bwbroersma
Copy link
Contributor

First of all thanks for making a PR for my issue 🙂

Adding the test in server-side-rendering will require an extra config flag to not use assert.htmlEqual on that specific test, as it removes comments.

I needed to patch this behavior in #4737 anyway, see my solution.

@confraria
Copy link
Contributor Author

😀 i had something similar but after discovering the existing test I removed it.
So since you already have a another PR should I close this one?

@bwbroersma
Copy link
Contributor

Please don't close this one, my other PR is just for whitespace! 🙂

@confraria
Copy link
Contributor Author

ah of course.. 🤦
Should I add another test to server-side-rendering/samples then?

@tanhauhau
Copy link
Member

hmm.. i see @Rich-Harris didn't follow up on his promise over there: #3539 (comment)

@confraria
Copy link
Contributor Author

confraria commented Jun 22, 2021

oh this is still open 🙃 is it still relevant? I can try to solve the conflict

@tanhauhau
Copy link
Member

no worries, i've rebased and fixed the conflicts

@dummdidumm dummdidumm merged commit 554d5dd into sveltejs:master Jun 28, 2021
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.

preserveComments doesn't work
6 participants