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

Need guidance to implement minify functionality #1021

Closed
jeevatkm opened this Issue Jan 14, 2016 · 33 comments

Comments

Projects
None yet
6 participants
@jeevatkm
Contributor

jeevatkm commented Jan 14, 2016

@core-team - Can you please provide help/guidance on building Minify functionality?

@xpbliss

This comment has been minimized.

xpbliss commented Jan 16, 2016

In the app.conf ,
Set results.compressed=true?

@jeevatkm

This comment has been minimized.

Contributor

jeevatkm commented Jan 16, 2016

Not a gzip functionality. Need HTML minify for my revel app. Planning to use https://github.com/tdewolff/minify.

Any guidance?

@pedromorgan

This comment has been minimized.

Member

pedromorgan commented Jan 17, 2016

Not sure if this falls within the scope of revel..

Are u suggesting processing stuff on the fly? shoulnt this kinda stuff be served statically ?

@jeevatkm

This comment has been minimized.

Contributor

jeevatkm commented Jan 17, 2016

I'm not sure about revel scope. however if revel provides HTML minify (not JS or CSS) like compress filter. It would be great :)

You're correct for JS & CSS, it should be statically served. But I'm looking for HTML only.

@jeevatkm

This comment has been minimized.

Contributor

jeevatkm commented Jan 17, 2016

I'm trying to create one, but facing Content-Length issue. Will post my code later.

@jeevatkm

This comment has been minimized.

Contributor

jeevatkm commented Jan 25, 2016

Sorted out Content-Length issue. Created Revel MinifyFilter by referring revel.CompressFilter.

Now MinifyFilter is working fine. Can core team have a look and comment on it? if I'm missing something!

import (
    "io"
    "net/http"
    "regexp"

    "github.com/revel/revel"

    "github.com/tdewolff/minify"
    "github.com/tdewolff/minify/css"
    "github.com/tdewolff/minify/html"
    "github.com/tdewolff/minify/js"
)

var (
    m            *minify.M
    htmlMimeType *regexp.Regexp
)

type MinifyWriter struct {
    http.ResponseWriter
    minifyWriter   io.WriteCloser
    shouldMinify   bool
    headersWritten bool
    closeNotify    chan bool
    parentNotify   <-chan bool
    closed         bool
}

func Minify(c *revel.Controller, fc []revel.Filter) {
    fc[0](c, fc[1:])

    if revel.Config.BoolDefault("results.minify", false) {
        if c.Response.Status != http.StatusNoContent && c.Response.Status != http.StatusNotModified {
            mw := m.Writer("text/html", c.Response.Out)
            writer := MinifyWriter{c.Response.Out, mw, false, false, make(chan bool, 1), nil, false}

            w, ok := c.Response.Out.(http.CloseNotifier)
            if ok {
                writer.parentNotify = w.CloseNotify()
            }

            c.Response.Out = &writer
        } else {
            revel.TRACE.Printf("Minify disabled for response status (%d)", c.Response.Status)
        }
    }
}

func (c MinifyWriter) CloseNotify() <-chan bool {
    if c.parentNotify != nil {
        return c.parentNotify
    }
    return c.closeNotify
}

func (c *MinifyWriter) WriteHeader(status int) {
    c.headersWritten = true
    c.prepareHeader()
    c.ResponseWriter.WriteHeader(status)
}

func (c *MinifyWriter) prepareHeader() {
    // minify is only applied to HTML content
    if htmlMimeType.MatchString(c.Header().Get("Content-Type")) {
        c.shouldMinify = true
        c.Header().Del("Content-Length")
    }
}

func (c *MinifyWriter) Close() error {
    if c.shouldMinify {
        c.minifyWriter.Close()
    }

    if w, ok := c.ResponseWriter.(io.Closer); ok {
        w.Close()
    }

    // Non-blocking write to the closenotifier, if we for some reason should
    // get called multiple times
    select {
    case c.closeNotify <- true:
    default:
    }
    c.closed = true
    return nil
}

func (c *MinifyWriter) Write(b []byte) (int, error) {
    // Abort if parent has been closed
    if c.parentNotify != nil {
        select {
        case <-c.parentNotify:
            return 0, io.ErrClosedPipe
        default:
        }
    }

    // Abort if we ourselves have been closed
    if c.closed {
        return 0, io.ErrClosedPipe
    }

    if !c.headersWritten {
        c.prepareHeader()
        c.headersWritten = true
    }

    if c.shouldMinify {
        return c.minifyWriter.Write(b)
    } else {
        return c.ResponseWriter.Write(b)
    }
}

func init() {
    m = minify.New()

    m.AddFunc("text/css", css.Minify)
    m.AddFunc("text/html", html.Minify)
    m.AddFunc("text/javascript", js.Minify)

    htmlMimeType = regexp.MustCompile("[text|appliation]/html")
}
@jeevatkm

This comment has been minimized.

Contributor

jeevatkm commented Jan 25, 2016

It minify's HTML, inline CSS and inline JS in the HTML content.

@pedromorgan

This comment has been minimized.

Member

pedromorgan commented Feb 3, 2016

Ok so I get the drift.. its packing siles and js minify and gzip and compress...

The only "realm" that revel could minify in, in in the compilation of the template,, and not its variables..

However it is interesting to consider dynamic css and build stuff as part of packaging is exiting idea..

This also goes along with the "embedded data", and having a dev enviroment reading siles, and an executable with all within.. ;-)))

In go 1.6 there the {{block}} and the {{foo -}} strip me naked {{-foo}} from other temaplaing languages..

The templating and stuff is also kinda in limbo as

  • smarty (confess as developer)
  • django/jinga2
  • golang is a pain in ass.. Am in with pongo2 with there was jinga2
@pedromorgan

This comment has been minimized.

Member

pedromorgan commented Feb 8, 2016

If we gonna implemeny minify etc.. then please come up with some more detail, code an even an RFC.. for now I am closing this issue. .and apologies...

@brendensoares

This comment has been minimized.

Member

brendensoares commented May 20, 2016

You have a type: "appliation".

How does this affect performance? Can you cache results?

Perhaps we can discuss this later and even add it to Revel. You might consider a PR. Also, we recommend using the RFC format in revel/rfcs to outline the reasons for adding this to Revel.

@goors

This comment has been minimized.

goors commented May 3, 2017

@jeevatkm How do you use this Minify in Revel? I have no idea where to put this and how to call it. Maybe in app/init.go (not working: getting some error runtime error: slice bounds out of range goroutine
)?
revel.Filters = []revel.Filter{ Minify, }

@notzippy

This comment has been minimized.

Collaborator

notzippy commented May 3, 2017

You should have an init file in your project that looks like this add in a line with MinifyWriter

@notzippy

This comment has been minimized.

Collaborator

notzippy commented May 3, 2017

p.s. there is also a configuration option that needs to be set as well results.minify=true

@jeevatkm

This comment has been minimized.

Contributor

jeevatkm commented May 3, 2017

@goors create a package called filter using above code snippet or this gist and you have register like this. Finally set the config results.minify=true as describe by @notzippy.

revel.Filters = []revel.Filter{
                revel.PanicFilter,             // Recover from panics and display an error page instead.
                revel.RouterFilter,            // Use the routing table to select the right Action
                revel.FilterConfiguringFilter, // A hook for adding or removing per-Action filters.
                revel.ParamsFilter,            // Parse parameters into Controller.Params.
                revel.SessionFilter,           // Restore and write the session cookie.
                revel.FlashFilter,             // Restore and write the flash cookie.
                revel.ValidationFilter,        // Restore kept validation errors and save new ones from cookie.
                revel.I18nFilter,              // Resolve the requested language
                HeaderFilter,                  // Add some security based headers
                revel.InterceptorFilter,       // Run interceptors around the action.
                revel.CompressFilter,          // Compress the result.
                filter.Minify,                 // Minify filter
                revel.ActionInvoker,           // Invoke the action.
        }
@goors

This comment has been minimized.

goors commented May 3, 2017

@jeevatkm I have add package filter and then add revel.Filters but i am getting empty view.
No idea why. I can see that init is never fired in filter package (i have put some log in it).

@jeevatkm

This comment has been minimized.

Contributor

jeevatkm commented May 3, 2017

@goors if init is not fried. You will be get panic since object is not yet initialized. do you see any?

@goors

This comment has been minimized.

goors commented May 3, 2017

@jeevatkm no, i do not see any panic.

@goors

This comment has been minimized.

goors commented May 3, 2017

When I log c.Response.Out in func Minify(c *revel.Controller, fc []revel.Filter) {} i can see this output:

&{0xc4202ce3c0 0xc42000bc00 {} 0x10ed820 false false false false 0xc420308180 {0xc4202462a0 map[] false false} map[Develop:[true] X-Frame-Options:[SAMEORIGIN] X-Xss-Protection:[1; mode=block] X-Content-Type-Options:[nosniff]] true 0 -1 0 false false [] 0 [0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] [0 0 0 0 0 0 0 0 0 0] 0xc420019110 0}

But view is empty. In controllers i use return c.Render() or something else?

@goors

This comment has been minimized.

goors commented May 3, 2017

When i disable m.AddFunc("text/html", html.Minify) it renders page. But not with minified html.

@jeevatkm

This comment has been minimized.

Contributor

jeevatkm commented May 3, 2017

@goors I just had a look on tdewolff/minify repo. It seems library have been improved. That code snippet was created year ago. So if you pull the repo you get latest library not the one I have used.

Just refer latest doc from repo and modify the minify initialization part. Then it would sort out your issue.

@goors

This comment has been minimized.

goors commented May 3, 2017

@jeevatkm xm, i am getting error now:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x141fc9f]
goroutine 95 [running]:
github.com/tdewolff/minify.(*M).MinifyMimetype(0x0, 0xc4202d5350, 0x9, 0x10, 0x186f540, 0xc420322000, 0x186ec40, 0xc420344558, 0x0, 0xc420330f40, ...)
	/Users/johnny/projects/go/src/github.com/tdewolff/minify/minify.go:138 +0x4f
github.com/tdewolff/minify.(*M).Minify(0x0, 0x15f42ba, 0x9, 0x186f540, 0xc420322000, 0x186ec40, 0xc420344558, 0x6, 0x15edd42)
	/Users/johnny/projects/go/src/github.com/tdewolff/minify/minify.go:130 +0xce
github.com/tdewolff/minify.(*M).Writer.func1(0xc4203c4db0, 0x0, 0x15f42ba, 0x9, 0x186f540, 0xc420322000, 0xc420344558)
	/Users/johnny/projects/go/src/github.com/tdewolff/minify/minify.go:214 +0xa9
created by github.com/tdewolff/minify.(*M).Writer
	/Users/johnny/projects/go/src/github.com/tdewolff/minify/minify.go:219 +0x1b4
2017/05/04 00:10:51 reverseproxy.go:316: http: proxy error: EOF
@goors

This comment has been minimized.

goors commented May 3, 2017

This part of minify.go in this repo https://github.com/tdewolff/minify give me some error:

m.literal[string(mimetype)]. Funny thing is that I can not figure out what that does.

@jeevatkm

This comment has been minimized.

Contributor

jeevatkm commented May 4, 2017

@goors as per the error info, it seems nothing wrong in the code.

My guess is probably previously compiled .a referring somewhere. Just clean respective repo directory under $GOPATH/pkg/.../... and run it again.

@goors

This comment has been minimized.

goors commented May 4, 2017

@jeevatkm i am really sorry for badgering you but, no, deleted and again same error. this guy @ tdewolff told me "Did you properly initiate M using https://github.com/tdewolff/minify/blob/master/minify.go#L68? Looks like literal has not been set."

@goors

This comment has been minimized.

goors commented May 4, 2017

@jeevatkm funny thing is that i do not see yet output of log in init func minify filter that you wrote.

@goors

This comment has been minimized.

goors commented May 4, 2017

@jeevatkm i just removed everything to check out if init is not executed because of some error. xm, init never executes in filter. strange?

@goors

This comment has been minimized.

goors commented May 4, 2017

@jeevatkm finally i get to bottom of this and minifier works (i need only html), here is gist for your filter.

I have put m := minify.New() in filter it self and remove init function. I have opend issue on revel for this init not executing in filter here: #1169

From 2.6kb to 1.8kb. Nice :)

@jeevatkm

This comment has been minimized.

Contributor

jeevatkm commented May 4, 2017

Glad you made it work. However initializing minify instance for every request is not good idea.

As @notzippy mentioned in #1169 func init called by golang.

@goors

This comment has been minimized.

goors commented May 5, 2017

@jeevatkm xm, i do not get it then , why it is not firing in filter .... doing another debug.

@notzippy

This comment has been minimized.

Collaborator

notzippy commented May 5, 2017

Check high up in the output log @goors , it will likely be one of the first things that is printed out.

@goors

This comment has been minimized.

goors commented May 6, 2017

@notzippy yes, sorry for that one. stupid. i did not see it.

@jeevatkm but when in init it is not working. same old error.

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x141fc9f]
goroutine 95 [running]:
github.com/tdewolff/minify.(*M).MinifyMimetype(0x0, 0xc4202d5350, 0x9, 0x10, 0x186f540, 0xc420322000, 0x186ec40, 0xc420344558, 0x0, 0xc420330f40, ...)
	/Users/johnny/projects/go/src/github.com/tdewolff/minify/minify.go:138 +0x4f
github.com/tdewolff/minify.(*M).Minify(0x0, 0x15f42ba, 0x9, 0x186f540, 0xc420322000, 0x186ec40, 0xc420344558, 0x6, 0x15edd42)
	/Users/johnny/projects/go/src/github.com/tdewolff/minify/minify.go:130 +0xce
github.com/tdewolff/minify.(*M).Writer.func1(0xc4203c4db0, 0x0, 0x15f42ba, 0x9, 0x186f540, 0xc420322000, 0xc420344558)
	/Users/johnny/projects/go/src/github.com/tdewolff/minify/minify.go:214 +0xa9
created by github.com/tdewolff/minify.(*M).Writer
	/Users/johnny/projects/go/src/github.com/tdewolff/minify/minify.go:219 +0x1b4
2017/05/04 00:10:51 reverseproxy.go:316: http: proxy error: EOF
@goors

This comment has been minimized.

goors commented May 6, 2017

@jeevatkm fcking ... stupid instead of m:= just m= sorry. now it works when in init.

@jeevatkm

This comment has been minimized.

Contributor

jeevatkm commented May 6, 2017

Thanks @goors 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment