Skip to content
This repository has been archived by the owner on Jun 18, 2022. It is now read-only.

Can you distribute a non-minified dist release? #37

Closed
peteygao opened this issue Sep 26, 2016 · 16 comments
Closed

Can you distribute a non-minified dist release? #37

peteygao opened this issue Sep 26, 2016 · 16 comments

Comments

@peteygao
Copy link

We're trying to use ofi.browser.js with an existing Rails project, but the built-in asset pipeline wants to re-minify all JS files, and unfortunately there seems to be some incompatibilities with our existing JS codebase and ofi.browser.js, which clobbers a function name in ofi.browser.js and throws a runtime exception.

With a non-minified dist .js, we can just let the Rails asset pipeline handle the minification process and hopefully avoid this issue.

@fregante
Copy link
Owner

fregante commented Sep 27, 2016

As explained in #36 (comment)

But I'd be interested to see the js file output by your pipeline, perhaps you could remove everything but OFI+a couple other functions before running the pipeline, and then paste it into pastebin.

Also, how does OFI arrive into your files? Are you using npm?

@peteygao
Copy link
Author

peteygao commented Sep 27, 2016

Sure, here's the output:

var objectFitImages=function(){"use strict";function t(t,r){if(!t[s].parsingSrcset){var i=l(t);if(i["object-fit"]=i["object-fit"]||"fill",!t[s].s){if("fill"===i["object-fit"])return;if(!t[s].skipTest&&o&&!i["object-position"])return}var c=t[s].ios7src||t.currentSrc||t.src;if(r)c=r;else if(t.srcset&&!a&&window.picturefill){var n=window.picturefill._.ns;t[s].parsingSrcset=!0,t[n]&&t[n].evaled||window.picturefill._.fillImg(t,{reselect:!0}),t[n].curSrc||(t[n].supported=!1,window.picturefill._.fillImg(t,{reselect:!0})),delete t[s].parsingSrcset,c=t[n].curSrc||c}if(t[s].s)t[s].s=c,r&&(t[s].srcAttr=r);else{t[s]={s:c,srcAttr:r||u.call(t,"src"),srcsetAttr:t.srcset},t.src=s;try{t.srcset&&(t.srcset="",Object.defineProperty(t,"srcset",{value:t[s].srcsetAttr})),e(t)}catch(l){t[s].ios7src=c}}t.style.backgroundImage='url("'+c+'")',t.style.backgroundPosition=i["object-position"]||"center",t.style.backgroundRepeat="no-repeat",/scale-down/.test(i["object-fit"])?(t[s].i||(t[s].i=new Image,t[s].i.src=c),function f(){return t[s].i.naturalWidth?void(t.style.backgroundSize=t[s].i.naturalWidth>t.width||t[s].i.naturalHeight>t.height?"contain":"auto"):void setTimeout(f,100)}()):t.style.backgroundSize=i["object-fit"].replace("none","auto").replace("fill","100% 100%")}}function e(e){var r={get:function(){return e[s].s},set:function(r){return delete e[s].i,t(e,r),r}};Object.defineProperty(e,"src",r),Object.defineProperty(e,"currentSrc",{get:r.get})}function r(){l||(HTMLImageElement.prototype.getAttribute=function(t){return!this[s]||"src"!==t&&"srcset"!==t?u.call(this,t):this[s][t+"Attr"]},HTMLImageElement.prototype.setAttribute=function(t,e){!this[s]||"src"!==t&&"srcset"!==t?f.call(this,t,e):this["src"===t?"src":t+"Attr"]=String(e)})}function i(e,r){var c=!A&&!e;if(r=r||{},e=e||"img",l&&!r.skipTest)return!1;"string"==typeof e?e=document.querySelectorAll("img"):e.length||(e=[e]);for(var n=0;n<e.length;n++)e[n][s]=e[n][s]||r,t(e[n]);c&&(document.body.addEventListener("load",function(t){"IMG"===t.target.tagName&&i(t.target,{skipTest:r.skipTest})},!0),A=!0,e="img"),r.watchMQ&&window.addEventListener("resize",i.bind(null,e,{skipTest:r.skipTest}))}var s="data:image/gif;base64,R0lGODlhAQABAIAAAP///wAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==",c=/(object-fit|object-position)\s*:\s*([-\w\s%]+)/g,n=new Image,o="object-fit"in n.style,l="object-position"in n.style,a="string"==typeof n.currentSrc,u=n.getAttribute,f=n.setAttribute,A=!1;return i.supportsObjectFit=o,i.supportsObjectPosition=l,r(),i}();

This file isn't even concatenated, it's simply passed through as a single vendor'd .js file into Sprockets' asset compilation pipeline and is rendered as a standalone file.

And no, I don't use NPM. The file in the dist folder of this repo is downloaded in raw form and added to our Rails app's vendor/assets/javascripts directory.

@fregante
Copy link
Owner

fregante commented Sep 27, 2016

What you pasted works without any issues

@peteygao
Copy link
Author

@bfred-it Really? How did you test it? Did you "test" it on a browser that already support object-fit, and thus actually skips the failure path? Use an older browser (or an iOS 9 device). You should see an exception noted below on line 6:

var objectFitImages = function() {
    "use strict";

    function t(t, r) {
        if (!t[s].parsingSrcset) {
            var i = l(t); /******* Runtime exception. Undefined function "l(t)" ******/
            if (i["object-fit"] = i["object-fit"] || "fill", !t[s].s) {
                if ("fill" === i["object-fit"]) return;
                if (!t[s].skipTest && o && !i["object-position"]) return
            }
            var c = t[s].ios7src || t.currentSrc || t.src;
            if (r) c = r;
            else if (t.srcset && !a && window.picturefill) {
                var n = window.picturefill._.ns;
                t[s].parsingSrcset = !0, t[n] && t[n].evaled || window.picturefill._.fillImg(t, {
                    reselect: !0
                }), t[n].curSrc || (t[n].supported = !1, window.picturefill._.fillImg(t, {
                    reselect: !0
                })), delete t[s].parsingSrcset, c = t[n].curSrc || c
            }
            if (t[s].s) t[s].s = c, r && (t[s].srcAttr = r);
            else {
                t[s] = {
                    s: c,
                    srcAttr: r || u.call(t, "src"),
                    srcsetAttr: t.srcset
                }, t.src = s;
                try {
                    t.srcset && (t.srcset = "", Object.defineProperty(t, "srcset", {
                        value: t[s].srcsetAttr
                    })), e(t)
                } catch (l) {
                    t[s].ios7src = c
                }
            }
            t.style.backgroundImage = 'url("' + c + '")', t.style.backgroundPosition = i["object-position"] || "center", t.style.backgroundRepeat = "no-repeat", /scale-down/.test(i["object-fit"]) ? (t[s].i || (t[s].i = new Image, t[s].i.src = c), function f() {
                return t[s].i.naturalWidth ? void(t.style.backgroundSize = t[s].i.naturalWidth > t.width || t[s].i.naturalHeight > t.height ? "contain" : "auto") : void setTimeout(f, 100)
            }()) : t.style.backgroundSize = i["object-fit"].replace("none", "auto").replace("fill", "100% 100%")
        }
    }

    function e(e) {
        var r = {
            get: function() {
                return e[s].s
            },
            set: function(r) {
                return delete e[s].i, t(e, r), r
            }
        };
        Object.defineProperty(e, "src", r), Object.defineProperty(e, "currentSrc", {
            get: r.get
        })
    }

    function r() {
        l || (HTMLImageElement.prototype.getAttribute = function(t) {
            return !this[s] || "src" !== t && "srcset" !== t ? u.call(this, t) : this[s][t + "Attr"]
        }, HTMLImageElement.prototype.setAttribute = function(t, e) {
            !this[s] || "src" !== t && "srcset" !== t ? f.call(this, t, e) : this["src" === t ? "src" : t + "Attr"] = String(e)
        })
    }

    function i(e, r) {
        var c = !A && !e;
        if (r = r || {}, e = e || "img", l && !r.skipTest) return !1;
        "string" == typeof e ? e = document.querySelectorAll("img") : e.length || (e = [e]);
        for (var n = 0; n < e.length; n++) e[n][s] = e[n][s] || r, t(e[n]);
        c && (document.body.addEventListener("load", function(t) {
            "IMG" === t.target.tagName && i(t.target, {
                skipTest: r.skipTest
            })
        }, !0), A = !0, e = "img"), r.watchMQ && window.addEventListener("resize", i.bind(null, e, {
            skipTest: r.skipTest
        }))
    }
    var s = "data:image/gif;base64,R0lGODlhAQABAIAAAP///wAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==",
        c = /(object-fit|object-position)\s*:\s*([-\w\s%]+)/g,
        n = new Image,
        o = "object-fit" in n.style,
        l = "object-position" in n.style,
        a = "string" == typeof n.currentSrc,
        u = n.getAttribute,
        f = n.setAttribute,
        A = !1;
    return i.supportsObjectFit = o, i.supportsObjectPosition = l, r(), i
}();

@fregante
Copy link
Owner

fregante commented Sep 28, 2016

What minifier are you using? A whole function is missing, it's not just about minification:

screen shot 2016-09-28 at 07 14 28

I'd be worried if my bundler dropped functions; for your own sake the issue should be solved at the source since the input is valid JavaScript.

If you can't fix that, here's how to generate an unminified ofi.browser.js:

git clone git@github.com:bfred-it/object-fit-images.git
cd object-fit-images
npm install
sed -i -e "s/('rollup-plugin-uglify/=>('/" */bfred-npm-bundler/*
npm run build

@peteygao
Copy link
Author

Thanks for that script!

I'm using whatever minifier Rails Sprockets depends on (I think it's uglify by default). I'll see if I can't reproduce this issue in isolation with a vanilla Rails app. If I can, I'll post this as an issue to Sprockets/uglify. Thanks for the help so far :)!

@mehanoid
Copy link

I've also run into this issue. It seems like a bug in UglifyJS, function is removed if option for removing unused code was enabled.
You can try it out for example using this online service:
https://skalman.github.io/UglifyJS-online/

@yratof
Copy link

yratof commented Oct 21, 2016

Same problem here, finding that grunt minify just rips out half of the functions needed to run this. Including the minified file in a workflow and excluding it from minification works, but is a pain to fit into previous projects, especially as this is a polyfill.

grunt-contrib-uglify strips it out for me. What are you using to minify in this project?

@fregante
Copy link
Owner

fregante commented Oct 21, 2016

Seems to be an uglify error that strips functions as multiple passes of minification, trying on that online service as well. I'm using uglify internally.

How are you people accessing this library? Do you use npm, git urls or do you download the file manually from github?

If UglifyJS is stripping some function that is being used, this is an UglifyJS issue. It's stripping code that's not supposed to strip.

@fregante fregante reopened this Oct 21, 2016
@yratof
Copy link

yratof commented Oct 21, 2016

At the moment, i've been including the file directly by downloading it and keeping part of another repo that updates several sites

@fregante
Copy link
Owner

I found the UglifyJS bug mishoo/UglifyJS#1337

@fregante
Copy link
Owner

What version of grunt-contrib-uglify are you using and what's your config? I'm unable to replicate the bug from uglifyjs' command line

@mehanoid
Copy link

Issue occurs with ruby gem uglifier v2.5.0 (which is using UglifyJS 2.4.13). It seems to be fixed in the latest version.

@fregante
Copy link
Owner

fregante commented Oct 21, 2016

Correct. It's fixed in Uglify 2.7.0

This is being used in:

@patricknelson
Copy link
Contributor

Thanks a ton @bfred-it for helping chase this down! I knew there was something fucky going on back in #36 but couldn't quite figure it out. Thanks for following up and letting us know! The fix seems simple enough.

@fregante
Copy link
Owner

fregante commented Feb 6, 2017

Non-minified version is now in v3 ;)

https://github.com/bfred-it/object-fit-images/blob/master/dist/ofi.js

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

5 participants