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

[bug] [safari] Causes a let declaration Syntax Error in Safari. #681

Open
trusktr opened this issue Aug 29, 2017 · 12 comments
Open

[bug] [safari] Causes a let declaration Syntax Error in Safari. #681

trusktr opened this issue Aug 29, 2017 · 12 comments
Labels
needs info The details mentioned are NOT clear or NOT enough. Please provide more information.
Milestone

Comments

@trusktr
Copy link

trusktr commented Aug 29, 2017

Oddly, this error doesn't happen in Chrome, only in Safari.

Input Code

Here's the non-minified code:

https://cdn.rawgit.com/trusktr/3248affba4a01a7ebf15f3cb0e68f18c/raw/b6b8258ced55798113d852c5b457f0df3bd033c5/not-minified.js

Actual Output

Here's the minified code:

https://cdn.rawgit.com/trusktr/3248affba4a01a7ebf15f3cb0e68f18c/raw/b6b8258ced55798113d852c5b457f0df3bd033c5/minified.js

Expected Output

It should work, not sure what the output should be. Here's the gist containing both files: https://gist.github.com/trusktr/3248affba4a01a7ebf15f3cb0e68f18c

Details

The following codepen uses the non-minified version. It works in Safari, you'll see a spinning square:
https://codepen.io/anon/pen/rzQmpK?editors=0010

The following pen is exactly the same, but it is using the minified version:
https://codepen.io/anon/pen/EvOmJM?editors=0010

In Safari, you will see errors with the minified version, that look like this:

screen shot 2017-08-29 at 11 12 18 am

If you run both of those pens in Chrome, they work fine. Only the non-minified pen works in Safari.

@trusktr
Copy link
Author

trusktr commented Aug 29, 2017

Additionally, you can reproduce by simply pasting the minified code in your Safari console.

@trusktr
Copy link
Author

trusktr commented Aug 29, 2017

I'm currently on node-minify 2.3.0 (I'm using very recent versions of Rollup and rollup-plugin-babili to make these files, and I see node-minify in my node_modules).

@vigneshshanmugam
Copy link
Member

vigneshshanmugam commented Aug 29, 2017

We have hit this bug before and had a workaround which fixes this issue on safari #567. The fix has been released as part of 0.1.3

Also i see that your code contains both var and let declarations and I am not sure whether that is causing the issue. Could you tell me how you are using this?

@trusktr
Copy link
Author

trusktr commented Aug 29, 2017

All vars (except for some vars I use used in the top level of modules for module export hoisting) are either from other node_modules or from build output during my Rollup+Buble transpile/bundle step.

Basically, what I am doing is npm run build-global in http://GitHub.com/trusktr/infamous which basically does (with Rollup+Buble) what is configured in rollup.config.js.

@trusktr
Copy link
Author

trusktr commented Aug 29, 2017

This outputs the global.js file you see in that repo. If you comment out the lines that use babili you can disable minification.

trusktr added a commit to lume/lume that referenced this issue Aug 30, 2017
@trusktr
Copy link
Author

trusktr commented Aug 30, 2017

As a quickfix, I can just set the target to ie:10 and the Safari is able to run the code with no errors.

This is the minified code targetting ie10:
https://cdn.rawgit.com/trusktr/3248affba4a01a7ebf15f3cb0e68f18c/raw/09ee1b15b982ab1048a8efaff857149396adf7fc/minified-target-ie10.js

And here's a pen with minified code now working in Safari:
https://codepen.io/anon/pen/EvOpNq?editors=0010

I've just gotta run my tests to make sure nothing broke due to variable shadowing, but I bet Buble probably handled this.

@Oliboy50
Copy link

Oliboy50 commented Sep 21, 2017

I'm using babel-preset-minify version 0.2.0 and the bug still occurs on Mac OS El Capitan - Safari 10.1.1 and on iOS 10.3.3 - Safari.

So the fix mentioned by @vigneshshanmugam in 0.1.3 does not seem to fix it.

Here is my error message :

SyntaxError: Cannot declare a const variable twice: 'd'.

(if you want to see my code, here you go => https://github.com/LinkValue/Appbuild/blob/master/webpack.config.js#L137)

As a workaround, i've set mangle: false in babel-preset-minify options... and it works, but it obviously makes my files bigger.

@boopathi
Copy link
Member

boopathi commented Oct 25, 2017

@trusktr @Oliboy50

Can you give a minimal repro version of your code?

@boopathi boopathi added the needs info The details mentioned are NOT clear or NOT enough. Please provide more information. label Oct 25, 2017
@marmanold
Copy link

I'm experiencing this issue, too.

@feuerste
Copy link

feuerste commented Nov 3, 2017

I have this issue, too. It seems to appear when mangling following file:
https://github.com/Polymer/polymer/blob/master/lib/mixins/property-accessors.html

@JakeDluhy
Copy link
Contributor

JakeDluhy commented Nov 10, 2017

I have run into this as well. Here's one example of the code that looks like it would cause problems: (my error is SyntaxError: Cannot declare a let variable twice: 'n'.)

var n = c(e.coordinates),
    i = fl(n, 3);
const s = i[0],
    o = i[2];
var l = c(t.coordinates),
    d = fl(l, 3);
const p = d[0],
    m = d[2],
    u = [_l(s, p, 0.1), e.height, _l(o, m, 0.1)],
    E = [_l(s, p, 0.9), t.height, _l(o, m, 0.9)],
    g = new yl.Y(E[2] - u[2], 0, u[0] - E[0]).normalize(),
    f = [],
    y = [],
    h = Array(...[, , , , , ]).map(() => []);
for (let n = 0; n <= Tl; n++) {
    var v = r(n / xl),
        T = fl(v, 3);
    const e = T[0],
        t = T[1],
        a = T[2];
    f.push(new yl.Y(e + g.x * vl, t, a + g.z * vl), new yl.Y(e - g.x * vl, t, a - g.z * vl)), h.forEach((e, t) => {
        var a = r(n / xl * ((t + 1) / Sl)),
            i = fl(a, 3);
        const s = i[0],
            o = i[1],
            l = i[2];
        e.push(new yl.Y(s + g.x * vl, o, l + g.z * vl), new yl.Y(s - g.x * vl, o, l - g.z * vl))
    })
}
for (let n = 0; n < bl; n++) {
    const e = (n + Tl) / xl,
        t = 2 * vl * (1 - n / bl);
    var b = r(e),
        S = fl(b, 3);
    const a = S[0],
        i = S[1],
        s = S[2];
    f.push(new yl.Y(a + g.x * t, i, s + g.z * t), new yl.Y(a - g.x * t, i, s - g.z * t)), h.forEach((a, n) => {
        var i = r(e * ((n + 1) / Sl)),
            s = fl(i, 3);
        const o = s[0],
            l = s[1],
            d = s[2];
        a.push(new yl.Y(o + g.x * t, l, d + g.z * t), new yl.Y(o - g.x * t, l, d - g.z * t))
    })
}

unminified:

const [x1, , z1] = axialToTHREE(from.coordinates);
const [x2, , z2] = axialToTHREE(to.coordinates);

const start = [
  linear(x1, x2, 0.1),
  from.height,
  linear(z1, z2, 0.1),
];

const end = [
  linear(x1, x2, 0.9),
  to.height,
  linear(z1, z2, 0.9),
];

const norm = new Vector3(
  end[2] - start[2],
  0,
  start[0] - end[0],
).normalize();

function getCurvePt(t) {
  const toff = t - 0.5;
  const quad = (1 - (4 * (toff ** 2)));

  return [
    linear(start[0], end[0], t),
    toff <= 0 ? linear(start[1], HEIGHT, quad) : linear(end[1], HEIGHT, quad),
    linear(start[2], end[2], t),
  ];
}

const verts = [];
const indices = [];
const morphVerticeArrays = Array(...Array(NUM_MORPH_STEPS)).map(() => []);

for(let i = 0; i <= NUM_STRAIGHT; i++) {
  const [x, y, z] = getCurvePt(i / NUM_TOTAL);

  verts.push(
    new Vector3(x + (norm.x * HALF_WIDTH), y, z + (norm.z * HALF_WIDTH)),
    new Vector3(x - (norm.x * HALF_WIDTH), y, z - (norm.z * HALF_WIDTH)),
  );

  morphVerticeArrays.forEach((arr, idx) => {
    const [xm, ym, zm] = getCurvePt((i / NUM_TOTAL) * ((idx + 1) / NUM_MORPH_STEPS));

    arr.push(
      new Vector3(xm + (norm.x * HALF_WIDTH), ym, zm + (norm.z * HALF_WIDTH)),
      new Vector3(xm - (norm.x * HALF_WIDTH), ym, zm - (norm.z * HALF_WIDTH)),
    );
  });
}

for(let i = 0; i < NUM_DIAGONAL; i++) {
  const t = (i + NUM_STRAIGHT) / NUM_TOTAL;
  const width = (HALF_WIDTH * 2) * (1 - (i / NUM_DIAGONAL));

  const [x, y, z] = getCurvePt(t);

  verts.push(
    new Vector3(x + (norm.x * width), y, z + (norm.z * width)),
    new Vector3(x - (norm.x * width), y, z - (norm.z * width)),
  );

  morphVerticeArrays.forEach((arr, idx) => {
    const [xm, ym, zm] = getCurvePt(t * ((idx + 1) / NUM_MORPH_STEPS));

    arr.push(
      new Vector3(xm + (norm.x * width), ym, zm + (norm.z * width)),
      new Vector3(xm - (norm.x * width), ym, zm - (norm.z * width)),
    );
  });
}

I also believe this would cause a problem too:

function n(e, t, a) {
    const n = new fi.p(e, t, a);
    r(n, [l[e], l[t], l[a]]), i.push(n)
}
const s = t / e,
    l = [],
    i = [];
l.push(new fi.Y(0, a({
    i: 0,
    j: 0,
    z: 0,
    x: 0
}), 0));
for (let n = 1; n <= e; n++) {
    const e = 6 * o(n),
        t = 6 * o(n - 1),
        r = e - t;
    for (let e = 1; e <= r; e++) {
        let o,
            d;
        const p = (e - 1) % n;
        if (0 == p) {
            const t = (e - 1) * (2 * ot / r);
            o = s * n * lt(t), d = s * n * st(t)
        } else {
            const t = it((e - 1) / n) * (ot / 3),
                a = Math.ceil((e - 1) / n) * (ot / 3);
            o = s * n * ((lt(a) - lt(t)) * (p / n) + lt(t)), d = s * n * ((st(a) - st(t)) * (p / n) + st(t))
        }
        l.push(new fi.Y(d, a({
            i: n,
            j: t + e,
            z: o,
            x: d
        }), o))
    }
}
for (let s = 1; s <= e; s++) {
    const t = 6 * o(s),
        a = 6 * o(s - 1),
        r = t - a;
    let i = 0;
    for (let l = 1; l <= r; l++) {
        let d;
        1 === s ? d = 0 : (d = 6 * o(s - 2) + (l - i), d = d > a ? 6 * o(s - 2) + 1 : d), n(a + l, a + (l % r + 1), d), s !== e && n(a + l, t + l + 1 + i, a + (l % r + 1)), 0 == l % s && i++
    }
}
const d = new fi.u;
return d.vertices = l, d.faces = i, d.computeFaceNormals(), d

I hope this helps!

@boopathi boopathi added this to the 1.0 milestone Dec 9, 2017
@LinusU
Copy link

LinusU commented Jun 21, 2018

This is a bug in Safari 10.x:

https://bugs.webkit.org/show_bug.cgi?id=171041

Uglify is working around the bug:

mishoo/UglifyJS#1753

Would be very nice if we could do the same here, this is currently affecting me in production 😱

Here is the commit that added the workaround to Uglify:

mishoo/UglifyJS@fcd90db

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs info The details mentioned are NOT clear or NOT enough. Please provide more information.
Projects
None yet
Development

No branches or pull requests

8 participants