Discarding whitespace from templates about rocker HOT 14 CLOSED

Hi Jens,

I took a look at a feature to remove additional whitespace and ultimately
there were a few issues with implementing it. In HTML, not all whitespace
is equal, so it seems like you'd need to parse it in order to know where to
remove it. Doing that during/after a render is computationally expensive
and frankly IMHO not worth the CPU cycles. Doing it during the initial
template compiling process would be the ideal place, but again you run into
edge cases where you really don't know if its safe. For example:

variable endTag = ">"

<a href="blah"@endtag

Since the intermixed dynamic code may generate tag data, you wouldn't
really be able to detect where the whitespace should be removed. Thus, I
believe you can only really safely remove whitespace after a render --
which again would negate much of the high performance & memory saving
features of Rocker.

I am of the mindset that gzip'ing the content either on your server or with
a reverse proxy would probably save enough / just as much bandwidth, and
you'd still keep all the high performance.

If you come up with a better approach, I'd be all for it! Just my 2 cents.

-Joe

On Tue, Feb 2, 2016 at 7:39 AM, Jens Fendler [email protected]
wrote:

Hi,

when creating HTML templates, I'm using indentation, empty lines, etc
quite generously to help with the readability during development However,
such is not needed (and unnecessarily blows up the resulting page) in
production

What would be nice is if Rocker could be configured (comparable to @option
https://github.com/option discardLogicWhitespace) to automatically
discard all (or some) whitespace, including but not necessarily limited to,

• empty lines
• spaces and/or tabs at the beginning and end of every line
• sets of more than one space/tab within lines

One simple approach would be to simply add code for this into
PlainTextunescape(), but at least for the items above special cases such as

elements or CDATA sections should probably not receive such treatment and would have to be parsed separately There might be other cases where the original strings should be left unaltered which I can't think of right now

Also, for non-HTML templates this might be an issue, but then again it should probably be configurable on/off on a per-template basis anyway

What do you think? Or is there any other (preferable) solution to this in your opinion? (I'm using Rocker within Ninja applications, but I don't think messing with the Result renderables there would be the right approach either)

—
Reply to this email directly or view it on GitHub
#16.

from rocker.

Hi,

I'm new to rocker coming from JSP. I generated java from the JSP and compile it ahead-of-time.

Like Jens I experienced a lot of whitespace from both the JSP tags but also from indentation etc.

So much that Google Pagespeed Insight complains and penalize our pages for being bloated. I Pagespeed Insights penalize good changes that google bot do the same...

I fixed this by between the java code is generated and it is compiled I do a search and replace in the java files which:

• remove all whitespaces before first non-whitespace character on a line
• replacing multi-whitespaces with a single whitespace
• remove whitespaces between tags

Here is the maven snippet I use:

<!-- Post-JSPC process generede java filer for at fjerne whitespaces --> <plugin> <groupId>com.google.code.maven-replacer-plugin</groupId> <artifactId>replacer</artifactId> <version>1.5.3</version> <executions> <execution> <phase>process-classes</phase> <goals> <goal>replace</goal> </goals> </execution> </executions> <configuration> <basedir>${project.build.directory}</basedir> <encoding>UTF-8</encoding> <includes> <include>jsp-source/jsp/**/*.java</include> </includes> <replacements> <replacement> <token>\\t</token> <value></value> </replacement> <!--<replacement> <token>\t</token> <value></value> </replacement>--> <replacement> <token>\\r\\n</token> <value>\\n</value> </replacement> <replacement> <token>\\n[ ]+</token> <value>\\n</value> </replacement> <replacement> <token>(\\n)+</token> <value>\\n</value> </replacement> <replacement> <token>out.write\(''\);</token> <value></value> </replacement> <replacement> <token>out.write\('\\n'\);</token> <value></value> </replacement> <replacement> <token>out.write\('\t'\);</token> <value></value> </replacement> <replacement> <token>out.write\("\\n\s*"\);</token> <value></value> </replacement> <replacement> <token><![CDATA[>[ ]+<]]></token> <value><![CDATA[><]]></value> </replacement> <replacement> <token><![CDATA[[ ]+\\n]]></token> <value><![CDATA[\\n]]></value> </replacement> <replacement> <token><![CDATA[out.write\("\\n<]]></token> <value><![CDATA[out.write\("<]]></value> </replacement> <replacement> <token><![CDATA[[ ]+>]]></token> <value><![CDATA[>]]></value> </replacement> <replacement> <token>private org\.apache\.AnnotationProcessor _jsp_annotationprocessor;</token> <value></value> </replacement> <replacement> <token>_jsp_annotationprocessor = \(org.apache.AnnotationProcessor\) getServletConfig\(\).getServletContext\(\).getAttribute\(org.apache.AnnotationProcessor.class.getName\(\)\);</token> <value></value> </replacement> </replacements> </configuration> </plugin>

from rocker.

@jjlauer Yes, you're right. That's also my line of reasoning: any removal (or pretty much any processing for that matter) should happen at compile time of the templates, not during page delivery to clients.

Of course, the traffic is not a big issue - my setup usually involves a reverse proxy with GZIP support. But looking at the code on the client side, plenty of blank lines and tabs just looks ugly and IMHO should be avoided - obviously: not at all costs. (In the end it's more of an issue of aesthetics, rather than a functional requirement). (EDIT: looking at the previous post, it might actually be an important feature to avoid penalties from search engines)

Anyway, perhaps we can still get somewhere..
I didn't dive too deep down into the compiler code, but we might be able to leave the dynamic processing as is, and only focus on the static text parts for starters?

There would still be potential issues as mentioned above, but what about a new Rocker "environment" like:

@removeWhitespace {
  ...some static text with whitespaces which will be removed during compile...
}
... some more static text whose whitespaces will be left intact.

This would leave it to the discretion of the user (template developer) to ensure the results are what they should be, and would not change any current default behaviour.
The downside would be that such a block would have to be treated as verbatim text by the parser, with any dynamic evaluation suspended, or we'd get the same issues you pointed out and which will probably be a nightmare to trace in more complex templates.

However, such (and similar) environments might make for some nice additional features. I'm thinking of a Rocker environment "registry", which allows for such environments to map to user-defined functions with (optional arguments and) the block content as a String (or byte[] or whatever) for any kind of processing which should happen at compile-time (e.g. whitespace removal, inclusion of external files, BASE64 encoding, or whatever people might come up with).

from rocker.

Interesting approach, bjornbak. I'll have a closer look at this plugin.

But it seems the problem of determining in which case tabs/spaces/newlines can safely be removed is still present - and probably even harder to solve than while the Rocker compiler is parsing the original template (and thus has some context to take into account).

from rocker.

I have not looked at the java code the rocker compiler generates but I guess the static parts of the templates are written with like JSP's out.write('\n'); which should be easy remove or replace...

from rocker.

@bjornbak Hmm... not necessarily I guess... Rocker generates something like this:

private static class PlainText {
        static private final String PLAIN_TEXT_0_0 = " ";
        static private final String PLAIN_TEXT_1_0 = "Hello test: ";
        static private final String PLAIN_TEXT_2_0 = ".\nHello temp: ";
        static private final String PLAIN_TEXT_3_0 = "\nHello raw temp: ";
        static private final String PLAIN_TEXT_4_0 = "\n\n\n";
}

within each compiled template. Matching only these strings with regular expressions is straight-forward. But I assume there's no guarantee by Rocker that these strings are broken at defined points (e.g. before each newline). In fact, Rocker often does NOT break at new lines which leaves the original issue of "is my string part of an HTML pre-element or CDATA section, or anything similar?".

Of course, if one really must have a no-whitespace output, applying your solution to selected files and/or critically checking the results before deployment is still an option.

from rocker.

@jfendler @bjornbak There are just too many potential issues with trying to parse the static text in order to remove whitespace for it to be a correct & general solution. @bjornbak your method works for you, but it seems to rely on you being awfully careful with what is static vs. dynamic for it to produce a correct result, 100% of the time.

from rocker.

@jjlauer any thoughts on the environment approach I suggested? Shouldn't be too tricky to implement..

from rocker.

jens you suggest a number of different approaches -- not sure what you mean
by "environment approach".

On Wed, Feb 3, 2016 at 3:10 PM, Jens Fendler [email protected]
wrote:

@jjlauer https://github.com/jjlauer any thoughts on the environment
approach I suggested? Shouldn't be too tricky to implement..

—
Reply to this email directly or view it on GitHub
#16 (comment).

from rocker.

@jjlauer I meant the introduction of (possibly user-definable) "block environments" in templates, which can perform a compile-time transformation of the block content, e.g.

@function(removeWhitespace) {
    Whitespace from this part of text will be stripped away during compilation.
}

or

@function(import, "myFile.txt")
@* the contents of myFile.txt will be imported as static text here *@

or anything else Rocker or the user provides as "functions".

from rocker.

Hi Jens,

It'd be easy to add a new compiler method like an @function, but have you
taken a look at what a block "compiles" to? It's not text, its a series of
lines of java code. For example, this should be valid inside your example
block:

@function(removeWhitespace) {

@if (true) {

  Whitespace from this part of text  @value will be stripped away

during compilation.

 }

}

So blocks can contain other blocks and I'm not exactly sure what you're
function would be getting called with. That example code above would be
parsed into a couple blocks and likely 4 plain text blurbs. They'd be
something like

<spaces & newlines>
Whitespace from this part of text
will be stripped away during compilation.
<spaces & newlines>

Maybe what you're really after is the ability to plugin to the compiler or
generator parts of the system. I'm not sure that's a @function type of
solution, but maybe a simple subclassing of the TemplateCompiler or
TemplateParser that let's you customize stuff. To be perfectly honest, it
seems like a lot of work and complexity for getting rid of whitespace.

As far as your @function(import, "myFile.txt") example. If you really want
to pull another file in, why not compile that other file as a template?
Then you could do this:

@myFile.template() and that file would be rendered inline with the template
lightning fast. Or if you really care about newlines, then call a java
function @ runtime to pull your myFile.txt in. Something like
@MyUtils.import("myFile.txt") would be fine. You could even have that
method return a Raw type so it bypasses Rocker's stringify escaping.

I'm happy to consider PRs on this stuff. Why don't you take a look at this
class:

https://github.com/fizzed/rocker/blob/master/compiler/src/test/java/com/fizzed/rocker/compiler/ParserMain.java

It parses a rocker template and spits out the AST model in-order. I'm
guessing that's the best place to start for you to flesh out an idea.

-Joe

On Sat, Feb 6, 2016 at 8:18 AM, Jens Fendler [email protected]
wrote:

@jjlauer https://github.com/jjlauer I meant the introduction of
(possibly user-definable) "block environments" in templates, which can
perform a compile-time transformation of the block content, e.g.

@function(removeWhitespace) {
Whitespace from this part of text will be stripped away during compilation.
}

or

@function(import, "myFile.txt")
@* the contents of myFile.txt will be imported as static text here *@

or anything else Rocker or the user provides as "functions".

—
Reply to this email directly or view it on GitHub
#16 (comment).

from rocker.

@jjlauer I'm aware that the nesting of other Rocker-logic in such an environment is exactly the (non-trivial) issue we were talking about earlier.
What I am proposing is in fact a much simplified (but as I believe: still useful) approach: "function" (perhaps not the best name) environments would be assumed NOT to contain any further logic to be evaluated, but to consist of ONLY static content (with "}" probably being the only character which would have to be escaped if it should be used within the environment). This content would be processed only during compile-time, and would result exclusively in static Strings of the template class.

You are thinking of use run-time processing (e.g. with the file import function example) - this is not what I had in mind. The results of the functions I am proposing would never be another template or method call, but would always result in simple, static strings.
As for the "why not make the other file a template": the other file might be a "live" configuration file, a "real" HTML file, or something else which I would want to render as-is without actually touching it. When using templates, and if such files contain any "@" chars, users would have to escape them, thereby probably violating the syntax of the original file. Secondly, there is simply no need to compile/use yet another template if all I need is to include non-changing content.

I'll have a look at the code you suggested and see if I can get something working. Please don't expect a PR too soon, but I believe this would make for a nice feature and will definitely try and get something working in this direction.

from rocker.

#20
Hi @jjlauer . To make some progress I'm proposing this PR, providing an option "removeWhitespace" to remove extra (i.e. multiple) whitespace from static strings during the compilation phase.

I don't believe this will hurt anybody, but will already help great deals with removing the amount of whitespace in a lot of HTML files. Of course, whitespace introduced from the dynamic elements of Rocker templates is not dealt with. However, the static content usually outweighs the dynamic content in templates by far; so I believe this to be quite a useful addition for starters in this regard.

I'll continue looking into the "environment"/"function" stuff mentioned, but this might take a bit more time.

from rocker.

PR #21 finally solves this issue rather well for me. Template models can now be post-processed (in this case by the WhitespaceRemovalProcessor) before their Java code is created.
When using this processor, all constant strings undergo a whitespace "reduction" in that multiple occurrences of newlines and/or spaces/tabs are reduced to single newlines and spaces, respectively.

@bjornbak I hope this helps you as well.

from rocker.