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

Trim whitespaces from texts in vue-template-compiler output #3934

Closed
SebastianS90 opened this issue Oct 13, 2016 · 15 comments
Closed

Trim whitespaces from texts in vue-template-compiler output #3934

SebastianS90 opened this issue Oct 13, 2016 · 15 comments

Comments

@SebastianS90
Copy link

I love the feature of single file components. They allow for proper syntax highlighting and we can write HTML without putting everything in awkward javascript strings.
A proper indentation within the template is useful to improve readability of .vue files.

Consider this example:

<template>
  <div>
    <p>
      This is my first text
    </p>
    <p>
      This is my second text
    </p>
  </div>
</template>

Compile it with webpack. The resulting bundle contains a compiled version of the template, you will find the following line:

return _h('div', [_h('p', ["\n    This is my first text\n  "]), " ", _h('p', ["\n    This is my second text\n  "])])

There is this preserveWhitespace option described here. If you configure webpack with loader: 'vue?preserveWhitespace=false', then you get the following:

return _h('div', [_h('p', ["\n    This is my first text\n  "]), _h('p', ["\n    This is my second text\n  "])])

Note that one array entry " " has gone, i.e. the whitespace between </p> and <p> has been removed.
But how about the leading and trailing whitespaces in the string "\n This is my first text\n " ?

My Feature Rrequest

Allow to trim these texts. The compilation result should look like this:

return _h('div', [_h('p', ["This is my first text"]), _h('p', ["This is my second text"])])

Changing this line as follows achieves what I want:

       text = inPre || text.trim()
-        ? decodeHTMLCached(text)
+        ? decodeHTMLCached(inPre || preserveWhitespace ? text : text.trim())
         // only preserve whitespace if its not right after a starting tag
         : preserveWhitespace && currentParent.children.length ? ' ' : '';

But I do not know whether it might break more complex layouts, so I posted it as issue here for discussion.

@fnlctrl
Copy link
Member

fnlctrl commented Oct 13, 2016

https://github.com/vuejs/vue/releases/tag/v2.0.3

I guess it's already added in 2.0.3?

vue-template-compiler
Exposed preserveWhitespace option.

@SebastianS90
Copy link
Author

@fnlctrl please re-read my entry post, the option is mentioned there 😉

@defcc
Copy link
Member

defcc commented Oct 13, 2016

@fnlctrl @SebastianS90 , means the leading and trailing whitespaces in a html tag ? I am looking into this.

@yyx990803
Copy link
Member

FYI I don't think this is a good idea, in the sense that it could be "unsafe" - the user may want to ignore whitespaces between tags, but not necessarily trimming all plain text, e.g inside <pre>. Whitespace between tags and whitespace around plain text are different in this sense.

On the other hand I don't see practical advantage for this. Ignoring additional whitespace nodes has a small perf gain, but trimming text doesn't do much in that aspect.

@cerlestes
Copy link

cerlestes commented Aug 1, 2017

@yyx990803 Hey Evan. Just to follow up on this issue: we find lots of pieces like this in our generated code:

[_vm._v("\n\t\t\t\t\t\t" + _vm._s(item.title) + "\n\t\t\t\t\t")]

We have thousands of those in our project, so they add up to quite some kBs. Compression should take care of them quite easily, but never the less it's just not nice to have this in the generated output when we could simply remove them during compile-time.

I'll see if I can add an option to the template compiler myself and will give you a pull request if I succeed.
I'm thinking of something like purgeWhitespace: ['div', 'span', ...]

@kennardconsulting
Copy link

kennardconsulting commented Jan 31, 2018

+1

I think this is actually a correctness issue, not just a performance one.

It seems that, in Chrome at least, doing:

_c('p',[_vm._v("\r\n\t\tMy gross annual salary is\r\n\t\t")

Will actually result in a small indent at the start of the paragraph (presumably it is honouring the \t). When rendered natively by the browser, such HTML:

<p>
	My gross annual salary is
</p>

normally has no indent.

@cerlestes
Copy link

cerlestes commented Feb 3, 2018

I've implemented template compiler options to trim whitespaces around text nodes, and to collapse sequences of whitespace characters within text nodes.

Production build sizes have reduced by 4-10% in different projects (5-20kB in absolute terms).

Please have a look at PR #7598

@davidrot
Copy link

davidrot commented Feb 4, 2018

Thank you @cerlestes for the good work! I'm excited to see this feature in vue. I was wondering and had some difficulties to find this topic... Can you explain how to activate this neat feature when the PR is accepted?

@cerlestes
Copy link

cerlestes commented Feb 4, 2018

@davidrot Sadly that's a bit more tricky until the Vue devs have completely integrated it. There's currently no generic way to specify those option parameters outside of manually calling the template compiler. The PR I've added to vue-loader would at least make it possible to change those values in the webpack configuration file, similiar to how preserveWhitespace could be toggled so far.

The quickest way for now will be to manually patch the vue-template-compiler package (actually you'll just need to patch build.js) with the patch and forcefully set the option constants to your desired values, within your project's node_modules directory.

I've also created another commit that I'd like to push to vue-loader, which would allow you to disable and enable those whitespace-relevant options as attribute of <template> inside each SFC, like so: <template whitespaces="trim,collapse,not-preserve">. This should be useful when turning on those options globally and then disabling it in components that for example require preserveWhitespace:true due to display:inline elements. But I'll wait for further actions from the devs before I push that, as I'm sure there'll be some changes.

IMHO in the long run, ideally those options should be turned on by default. It just doesn't make any sense to me to send \t\r\n\s in a compiled template like this down the network (note that my patch disables these optimizations within <pre> elements by default), when the browser will strip those away anyway. It just adds overhead to the bundle size and lowers performance. With the popularity of Vue, this probably amounts to Megabits of junk whitespaces being transferred over the internet at any given time.

Edit: I was talking about vue-router when I obviously meant vue-loader.

@OperKH
Copy link

OperKH commented Apr 25, 2018

I have a bugs on production builds that produces extra spaces before text or makes extra line for buttons text when I have templates like that:

<p>
    Some long text
</p>

<button>
    Close
</button>

I don't why this bug exists only on production, and I don't see it on while development.

@foxbunny
Copy link

foxbunny commented Sep 17, 2018

EDIT: I apologize for spamming prematurely. This is probably an issue on my end. Please disregard.

Not sure if my issue is related to this or not, but I get that, in JSX, when I insert a single space anywhere, it gets stripped.

Examples:

<span> </span>
      ^

or

{someFn()} <span>{someOtherFn()}</span>
          ^

Expected behavior based on how browsers deal with HTML (and I believe it's also in the spec) is that the single space is always preserved. This is clearly not the same as just calling trim() which does not collapse, but strip away any whitespace.

I have not tested this with templates, as I do not use them.

To clarify, this is a case where a child vnode is a string with just whitespace.

@ky-is
Copy link

ky-is commented Sep 22, 2018

I saw reports of weird spacing inconsistencies from some users, so to be safe I wanted to normalize this (and save a couple bytes). I've added a sed step to my build command to post-process js files as a partial fix:

"build": "vue-cli-service build --modern && sed -E -i '' -e 's/\\\\n(\\\\t)+/\\ /g' $(find dist/js -type f)",

This replaces /\\n(\\t)+/g with a single space (you can probably adjust it for space indentation too). Just ensure that regex doesn't match any of your source code or things will break. dist/js is the directory of files to process. Seems to be working for me, but be sure to test the output for your project.

@Dewep
Copy link

Dewep commented Nov 27, 2018

Until now, to correct this issue, we added a <template> between each of our texts (and yes, it is an issue, as said several times, in several issues or pull requests, not only the generated code is unnecessarily heavier, but especially, it is not compatible with the white-space style and it can add additional spaces in the rendering).

But with the recent eslint-vue rules (in particular vue/multiline-html-element-content-newline), we had <template>...</template> everywhere in our HTML.

Seeing that adding this kind of option doesn't seem to be appreciated by @yyx990803, we decided to find a better solution, without having to fork Vue. In the webpack configuration, we used a custom compiler, which remove the extra whitespaces, and call the real compiler:

const VueTemplateCompiler = require('vue-template-compiler')

const VueTemplateCompilerProxy = {
  ...VueTemplateCompiler,
  compile: function (template, options) {
    template = template.replace(/>[^<]+</g, function (substring) {
      return substring.replace(/\r?\n\s*/g, '')
    })
    return VueTemplateCompiler.compile(template, options)
  }
}

const webpackConfig = {
  // ...
  {
    test: /\.vue$/,
    loader: 'vue-loader',
    options: {
      compiler: VueTemplateCompilerProxy,
      compilerOptions: {
        preserveWhitespace: false
      }
    }
  }
  // ...
}

There are certainly some cases where this is not perfect, or that you would like to have some exceptions, but I think it can help, until you have an option in Vue directly (we keep hope).

@lucasmpaim
Copy link

Using the vue-cli like:

  chainWebpack: config => {
    config.module.rule('vue')
      .use('vue-loader')
      .loader('vue-loader')
      .tap(options => {
        options.compiler = VueTemplateCompilerProxy
        options.compilerOptions.preserveWhitespace = false;
        return options
      })
  }

on build process I receive the exception: compiler.parseComponent is not a function, In development is working perfectly, in vue-loader/lib/index.js I put an console.log to check why this is happening, and I notice in sometimes the compiler is a blank object {}.
The log:

App.vue { parseComponent: [Function: parseComponent],
  compile: [Function: compile],
  compileToFunctions: [Function: compileToFunctions],
  ssrCompile: [Function: compile],
  ssrCompileToFunctions: [Function: compileToFunctions] }
App.vue { parseComponent: [Function: parseComponent],
  compile: [Function: compile],
  compileToFunctions: [Function: compileToFunctions],
  ssrCompile: [Function: compile],
  ssrCompileToFunctions: [Function: compileToFunctions] }
App.vue { parseComponent: [Function: parseComponent],
  compile: [Function: compile],
  compileToFunctions: [Function: compileToFunctions],
  ssrCompile: [Function: compile],
  ssrCompileToFunctions: [Function: compileToFunctions] }
App.vue {}
.....

@yyx990803
Copy link
Member

yyx990803 commented Dec 26, 2018

Relevant: #9208 (comment)

Please comment in the newer issue if you still have feedback regarding the behavior.

@vuejs vuejs locked as resolved and limited conversation to collaborators Dec 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests