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

Breaks Magnific Popup gallery #159

Closed
callym opened this issue Sep 13, 2016 · 4 comments
Closed

Breaks Magnific Popup gallery #159

callym opened this issue Sep 13, 2016 · 4 comments
Labels
bug Confirmed bug

Comments

@callym
Copy link

callym commented Sep 13, 2016

Using Babili with Magnific Popup breaks the gallery view, but not the rest of the script.

Using the script from here
https://codepen.io/callym/pen/JRGawr

Replacing that script with the minified output from babili
https://codepen.io/callym/pen/jrWvJW

@hzoo
Copy link
Member

hzoo commented Sep 13, 2016

Thanks for the report, but we'll need to have a reduced reproduction step. Right now we just know that something in that whole file isn't working. And did you just paste it into the repl?

@callym
Copy link
Author

callym commented Sep 13, 2016

I've been trying to replace sections of the unminified file with the minified one, and the function that causes it is:

        initGallery: function() {

            var gSt = mfp.st.gallery,
                ns = '.mfp-gallery';

            mfp.direction = true; // true - next, false - prev

            if(!gSt || !gSt.enabled ) return false;

            _wrapClasses += ' mfp-gallery';

            _mfpOn(OPEN_EVENT+ns, function() {

                if(gSt.navigateByImgClick) {
                    mfp.wrap.on('click'+ns, '.mfp-img', function() {
                        if(mfp.items.length > 1) {
                            mfp.next();
                            return false;
                        }
                    });
                }

                _document.on('keydown'+ns, function(e) {
                    if (e.keyCode === 37) {
                        mfp.prev();
                    } else if (e.keyCode === 39) {
                        mfp.next();
                    }
                });
            });

            _mfpOn('UpdateStatus'+ns, function(e, data) {
                if(data.text) {
                    data.text = _replaceCurrTotal(data.text, mfp.currItem.index, mfp.items.length);
                }
            });

            _mfpOn(MARKUP_PARSE_EVENT+ns, function(e, element, values, item) {
                var l = mfp.items.length;
                values.counter = l > 1 ? _replaceCurrTotal(gSt.tCounter, item.index, l) : '';
            });

            _mfpOn('BuildControls' + ns, function() {
                if(mfp.items.length > 1 && gSt.arrows && !mfp.arrowLeft) {
                    var markup = gSt.arrowMarkup,
                        arrowLeft = mfp.arrowLeft = $( markup.replace(/%title%/gi, gSt.tPrev).replace(/%dir%/gi, 'left') ).addClass(PREVENT_CLOSE_CLASS),
                        arrowRight = mfp.arrowRight = $( markup.replace(/%title%/gi, gSt.tNext).replace(/%dir%/gi, 'right') ).addClass(PREVENT_CLOSE_CLASS);

                    arrowLeft.click(function() {
                        mfp.prev();
                    });
                    arrowRight.click(function() {
                        mfp.next();
                    });

                    mfp.container.append(arrowLeft.add(arrowRight));
                }
            });

            _mfpOn(CHANGE_EVENT+ns, function() {
                if(mfp._preloadTimeout) clearTimeout(mfp._preloadTimeout);

                mfp._preloadTimeout = setTimeout(function() {
                    mfp.preloadNearbyImages();
                    mfp._preloadTimeout = null;
                }, 16);
            });


            _mfpOn(CLOSE_EVENT+ns, function() {
                _document.off(ns);
                mfp.wrap.off('click'+ns);
                mfp.arrowRight = mfp.arrowLeft = null;
            });

        }

which is being minified to

        initGallery: function initGallery() {
            var a = mfp.st.gallery,
                b = '.mfp-gallery'; // true - next, false - prev
            return mfp.direction = !0, a && a.enabled
        }

but if I remove this line (at the top of the function)

if(!gSt || !gSt.enabled ) return false;

then it all works as expected - in the codepen and in my own site!

@goto-bus-stop
Copy link
Contributor

goto-bus-stop commented Sep 14, 2016

I think there are two bugs here:

Preset:

plugins: [
  'babel-plugin-minify-guarded-expressions',
  'babel-plugin-minify-simplify',
]

Input:

function foo() {
  if (!gSt || !gSt.enabled) return false;
  _mfpOn(thing);
  _mfpOn(otherThing);
}

minify-simplify turns the function into:

function foo() {
  return gSt && gSt.enabled && void (_mfpOn(thing), _mfpOn(otherThing));
}

(which isn't quite correct, this will return 0 instead of false when gSt.enabled===0 for example. culprit: pattern match might change the return type)

Then minify-guarded-expressions sees gSt && gSt.enabled && void (something). The last part will always evaluate to undefined (falsy), so it drops it, even though it has side effects.

@callym
Copy link
Author

callym commented Sep 20, 2016

this is fixed in babel-preset-babili@0.0.3! 😄 thanks everyone!

@hzoo hzoo closed this as completed Sep 21, 2016
@hzoo hzoo added the bug Confirmed bug label Sep 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug
Projects
None yet
Development

No branches or pull requests

3 participants