Fixed false positive syntax errors with in inside for #54801
Conversation
Co-authored-by: Mateusz Burzyński <[email protected]>
| parserForInStatement8.ts(3,10): error TS2491: The left-hand side of a 'for...in' statement cannot be a destructuring pattern. | ||
| parserForInStatement8.ts(4,10): error TS2491: The left-hand side of a 'for...in' statement cannot be a destructuring pattern. |
There was a problem hiding this comment.
Those errors are not correct but that's a separate issue
There was a problem hiding this comment.
How challenging is that to fix?
There was a problem hiding this comment.
I'm looking into this but it's the first time that I even opened the files related to downleveling syntax here so it might take a moment (if I don't lose interest in this, that is).
- the error has to be likely removed completely (I removed it from one location but there is another one and I didn't even look into it yet)
- this
undefinedshouldn't be emitted (at least in this case) - and the declaration list can't be "replaced" in place, since downleveling binding patterns requires creating temp bindings and for-in only supports a single binding (
Uncaught SyntaxError: Invalid left-hand side in for-in loop: Must have a single binding.) - to fix 2 and 3 it's likely the easiest to move the binding "expansion" to the loop's body, this is what Babel does:
for (var _ref3 in {
'': 0
}) {
var _ref2 = _toArray(_ref3);
var _ref2$ = _ref2[0];
var x = _ref2$ === void 0 ? 'a' in {} : _ref2$;
var kaka = _ref2[1];
var rest = _ref2.slice(2);
console.log(x);
}
There was a problem hiding this comment.
It’s legal to use destructuring in for..in? What would that even mean since the value is always a string (or symbol)?
There was a problem hiding this comment.
Strings are iterable so u could read its characters through an array pattern, or some prototype methods/properties through an object pattern.
Is it useful? Not rly. Is it legal? Apparently yes :P
There was a problem hiding this comment.
Just close the loop on this one, if you don't think you'll get around to fixing this in this PR, can you add a note to the test case that says "hey, this is wrong" and file an issue, or similar?
There was a problem hiding this comment.
Er, actually, is #54853 that PR?
There was a problem hiding this comment.
Alright, I guess we can just leave the PR as-is?
| for (var _a = (void 0)[0], x = _a === void 0 ? 'a' in {} : _a in { '': 0 }) | ||
| console.log(x); | ||
| for (var _b = (void 0).x, x = _b === void 0 ? 'a' in {} : _b in { '': 0 }) | ||
| console.log(x); |
There was a problem hiding this comment.
this isn't the correct output but since binding patterns on the left side of for-in are not supported by TS today, I think that it's fine to leave it as is for now
| parserForInStatement8.ts(3,10): error TS2491: The left-hand side of a 'for...in' statement cannot be a destructuring pattern. | ||
| parserForInStatement8.ts(4,10): error TS2491: The left-hand side of a 'for...in' statement cannot be a destructuring pattern. |
There was a problem hiding this comment.
Just close the loop on this one, if you don't think you'll get around to fixing this in this PR, can you add a note to the test case that says "hey, this is wrong" and file an issue, or similar?
fixes #54769