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
Cannot redeclare let variable in Safari #559
Comments
Thanks. This looks like is a bug. It shouldn't be reused with fn param in the same scope. Can you give the source code and config of how you used babili that reproduces this bug ? I'm not able to reproduce with a simple function. function foo(bar) {
let baz = 1;
console.log(baz);
} gives function foo(a) {
let b = 1;
console.log(b);
} |
The source code is huge (I pass babili over rollup bundle with mixed es6 / es3 content), and it produces a lot of that results, but all times with Config is below: // we will pass bundled file via babili
({ code, map } = transform(code, {
babelrc: false,
sourceMaps: true,
presets: [
[
'env',
{
targets: {
safari: '10.1',
firefox: 53,
chrome: 57,
edge: 15,
},
modules: false,
loose: true,
useBuiltIns: true,
preferBuiltins: true,
debug: false,
},
],
[
'babili',
{
keepFnName: false,
keepClassName: false,
removeConsole: true,
removeDebugger: true,
},
],
],
compact: true,
minified: true,
comments: false,
plugins: [
[
'transform-define',
{
'tino.env': 'production',
'tino.es6': true,
'typeof window': 'object',
},
],
],
})); the first function is just an example, the second function is output of this: function createStub(str, maxLen) {
const lc = str.toLowerCase();
const stubsArray = lc.split(/\s+/);
const res = [];
for (
let i = 0, currentLen = 0;
currentLen < maxLen && i < stubsArray.length;
(currentLen += stubsArray[i].length), i++
) {
res.push(
stubsArray[i]
.split('')
.map(a => a.normalize('NFKD').charAt(0))
.filter(a => /[A-z]/.test(a))
.join('')
);
}
return res.join('-');
} |
I think it is a safari bug. function foo(a = 1) {
for (let a = 2;;) {
console.log("loop", a); // 2
break;
}
console.log("fn param", a); // 1
}
foo(); is valid ES6. The |
Nobody will write such code in real life, why minify should? Just take next letter if this is used for function param already? |
as per https://kangax.github.io/compat-table/es6/ Safari 10.1 is the browser with most complete ES6 support. If it has a bug, it's better to adopt for it, instead of fighting with a bleeding edge browser. |
Safari's block scoping implementation is by far the buggiest. There is a lot of valid code that crashes in safari, both at parse and run time. The only reason I haven't reported is because I haven't been able to get it to reproduce in non-proprietary code (or with bundles smaller than "entire work website codebase"), but hopefully this bug gets Safari to look better into their block scoping implementation. |
FYI, https://webkit.org/blog/7622/release-notes-for-safari-technology-preview-31/ under |
@boopathi That's great, but the bug is already in several shipped versions, so, I think it must be avoided for a next couple of years... No fix? |
Safari raises a syntax error for a `let` or `const` declaration in a `for` loop initalization that shadows a parent function's parameter: ```js function a(b) { let (b;;); } ``` This is valid code, but Safari throws a syntax error. The bug has been [reported](https://bugs.webkit.org/show_bug.cgi?id=171041) and since [fixed](https://trac.webkit.org/changeset/217200/webkit/trunk/Source) in WebKit, so future versions of Safari will not have this problem. This modifies the scope tracker's `canUseInReferencedScopes` method to detect cases where a rename would trigger this bug and use a different name. Fixes babel#559
Safari raises a syntax error for a `let` or `const` declaration in a `for` loop initalization that shadows a parent function's parameter: ```js function a(b) { for (let b;;); } ``` This is valid code, but Safari throws a syntax error. The bug has been [reported](https://bugs.webkit.org/show_bug.cgi?id=171041) and since [fixed](https://trac.webkit.org/changeset/217200/webkit/trunk/Source) in WebKit, so future versions of Safari will not have this problem. This modifies the scope tracker's `canUseInReferencedScopes` method to detect cases where a rename would trigger this bug and use a different name. Fixes babel#559
Safari raises a syntax error for a `let` or `const` declaration in a `for` loop initalization that shadows a parent function's parameter: ```js function a(b) { for (let b;;); } ``` This is valid code, but Safari throws a syntax error. The bug has been [reported](https://bugs.webkit.org/show_bug.cgi?id=171041) and since [fixed](https://trac.webkit.org/changeset/217200/webkit/trunk/Source) in WebKit, so future versions of Safari will not have this problem. This modifies the scope tracker's `canUseInReferencedScopes` method to detect cases where a rename would trigger this bug and use a different name. Fixes babel#559
We hit this bug at work. I've put together what I hope is a successful workaround and filed a PR: #567. |
* Add workaround for Safari for loop lexical scope bug Safari raises a syntax error for a `let` or `const` declaration in a `for` loop initalization that shadows a parent function's parameter: ```js function a(b) { for (let b;;); } ``` This is valid code, but Safari throws a syntax error. The bug has been [reported](https://bugs.webkit.org/show_bug.cgi?id=171041) and since [fixed](https://trac.webkit.org/changeset/217200/webkit/trunk/Source) in WebKit, so future versions of Safari will not have this problem. This modifies the scope tracker's `canUseInReferencedScopes` method to detect cases where a rename would trigger this bug and use a different name. Fixes #559 * More idiomatic implementation * More extensive tests * Better use path API
After upgrading to babili 0.1.2 following error appears in Safari 10.1 (running ES6 native):
SyntaxError: Cannot declare a let variable twice: 'e'.
. It happens on fragments like this (after minify I have a lot of this):and it seems for me that Safari counts function parameters as declared by
let
. Neither Chrome, Firefox or Edge have such problem. Is there any workaround apart of switching off minify (as I'm forced to do now)?The text was updated successfully, but these errors were encountered: