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

Spaces between html tags (like \n\t\t\t) #2745

Closed
frederikhors opened this issue May 13, 2019 · 10 comments · Fixed by #3030
Closed

Spaces between html tags (like \n\t\t\t) #2745

frederikhors opened this issue May 13, 2019 · 10 comments · Fixed by #3030
Assignees
Labels

Comments

@frederikhors
Copy link

First of all thanks a lot for your work: Svelte is amazing!

Now I have a problem with spaces.

If you just use the below code in a REPL:

<li>
  <span>Test</span>
  <span>username</span>
</li>

The compiler on my machine create a js with this code:

.innerHTML="<span>Test</span>\n\t\t\t  <span>username</span>"

Maybe I'm wrong, but I would like to understand how to get everything compiled without \n\t\t\t .

So from this:

.innerHTML="<span>Test</span>\n\t\t\t  <span>username</span>"

to this:

.innerHTML="<span>Test</span><span>username</span>"

Am I wrong?

@Conduitry
Copy link
Member

It looks like the collapsing of whitespace in #2258 (which defaults to on) isn't happening when the content is completely static and we're able to use innerHTML. It probably should.

With that fix though, this would be getting compiled to

.innerHTML = `<span>Test</span> <span>username</span>`;

to be consistent (and to be safer). We don't in generally want to remove all of the whitespace.

@Conduitry Conduitry added the bug label May 13, 2019
@frederikhors
Copy link
Author

@Conduitry there is something I can do now until fix?

@frederikhors
Copy link
Author

@Conduitry I think the space is not useful there. In React and Vue it doesn't exists.

Many components with the same DOM will loose style with the space.

Many browsers add a space between elements if there is a space in HTML. I think the space is not good there.

E.g: Bootstrap navbars. With spaces all becomes a problem.

@frederikhors
Copy link
Author

I think if I need space I can add myself a {' '} like in JSX world, nope?

@Conduitry
Copy link
Member

If you need to have no whitespace between two elements, you should place them right next to each other. Svelte's syntax isn't JSX, and its whitespace handling rules more closely follow those of HTML. This is intentional.

Yes, there are situations where it is safe to remove the spaces but we currently aren't. Those can be enhancements for later though, and aren't really bugs that need to be fixed. Not behaving like JSX is certainly not a bug. More whitespace optimizations can happen as part of #189.

What is a bug is that the whitespace is not being automatically collapsed to one space when an element has no dynamic contents and we can set it with .innerHTML = whatever instead of creating individual elements in it.

@mrkishi
Copy link
Member

mrkishi commented May 13, 2019

Until that happens, you are free to use an html minifier on the markup preprocessor, eg:

import { minify } from 'html-minifier';

...

preprocess: !dev && {
	markup({ content }) {
		return {
			code: html_minify(content, {
				caseSensitive: true,
				collapseWhitespace: true,
				decodeEntities: true,
				keepClosingSlash: true,
				removeAttributeQuotes: true,
				removeComments: true,
				removeRedundantAttributes: true,
				removeScriptTypeAttributes: true,
				removeStyleLinkTypeAttributes: true,
				sortAttributes: true,
				sortClassName: true,

				ignoreCustomFragments: [/<script>[\s\S]*?<\/script>/, /\{[\s\S]*?\}/],
			})
		}
	}
},

@maxmilton
Copy link
Contributor

For what it's worth, I've made a Svelte markup preprocessor which removes excessive whitespace: https://github.com/WeAreGenki/minna-ui/tree/master/utils/preprocess

When all you care about is getting rid of excessive whitespace it might be a viable option for you, otherwise html-minifier (like mrkishi mentioned) or even PostHTML might be better as they are more feature packed. Personally I could never get them to work well when I had objects in the template (e.g. {doSomething({ val: 1 })}), so I use a simpler whitespace only approach.

@simonbuchan
Copy link

Here's an example which shows the difference between collapsing and removing whitespace, and why it's not safe in general; and showing a case in JSX where it would behave differently, and a CSS workaround.

I feel like JSX has a good trade-off, once you understand it, in HTML removing the whitespace from the DOM can require very ugly formatting, but in practice you normally just need a bit more styling.

@creaven
Copy link

creaven commented May 14, 2019

yet one ugly workaround if want no space, but write in multiple lines - add comment

        <button>First</button><!--
     --><button>Second</button>

@frederikhors
Copy link
Author

I opened an issue on VSCode Svelte plugin repo (jamesbirtles/svelte-vscode#50).

The problem is I need this code to stay like this:

<div>
  Test (<span class="color">one</span>)
</div>

It becomes this instead:

<div>
  Test (
  <span class="color">one</span>
  )
</div>

Is it related to Svelte compiler or an issue with ghost spaces added by vscode plugin?

What can I do?

I'm not understanding.

@Conduitry Conduitry self-assigned this Jun 16, 2019
Conduitry added a commit that referenced this issue Jun 16, 2019
- collapse whitespace to single space when appropriate (#2745)
- escape template string characters in script and style tags
Conduitry added a commit that referenced this issue Jun 18, 2019
- collapse whitespace to single space when appropriate (#2745)
- escape template string characters in script and style tags
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants