ObjectBindingPatterns Implementation#376
Conversation
|
You can now mix and nest array/object destructuring patterns. let [a, { really, strange, tbl: { data, structure } }] =
[0, { really: 1, strange: 2, tbl: { data: 3, structure: 4 }}];
print(a, really, strange, data, structure); // 0, 1, 2, 3, 4This can also happen inside functions/methods. You can't use ellipse destructuring for objects or arrays. let {...x} = {x: 1, y: 2}; // Disallowed
let [...x] = [0, 1, 2]; // Disallowed, this happened beforeI've kept the behaviour of And instead of throwing an exception when there are nested bindings the transpiler falls back to this new method. |
Perryvw
left a comment
There was a problem hiding this comment.
I like these changes, also happy with the tests. I added a few minor suggestions regarding naming and style, but overall very good job!
src/LuaTransformer.ts
Outdated
| // Binding patterns become _table0, _table1, etc as function parameters | ||
| // See transformFunctionBody for how these values are destructured | ||
| const paramName = ts.isObjectBindingPattern(param.name) || ts.isArrayBindingPattern(param.name) | ||
| ? tstl.createIdentifier(`_table${identifierIndex++}`) |
There was a problem hiding this comment.
I don't think _table is a good name, to be consistent with the rest of our extra variables I would prefer something like ____TS_bindingPattern
src/LuaTransformer.ts
Outdated
| const bindingPatternDeclarations: tstl.Statement[] = []; | ||
| parameters.forEach(binding => { | ||
| if (ts.isObjectBindingPattern(binding.name) || ts.isArrayBindingPattern(binding.name)) { | ||
| const identifier = tstl.createIdentifier(`_table${identifierIndex++}`); |
There was a problem hiding this comment.
Same as other comment regarding name
src/LuaTransformer.ts
Outdated
| // nested binding pattern | ||
| const propertyName = isObjectBindingPattern | ||
| ? element.propertyName | ||
| : ts.createNumericLiteral(`${index}`); |
There was a problem hiding this comment.
I don't like this `${index}` just to cast to string, I think String(index) would be better.
src/LuaTransformer.ts
Outdated
| const isObjectBindingPattern = ts.isObjectBindingPattern(pattern); | ||
| let index = 0; | ||
| for (const element of pattern.elements) { | ||
| index++; |
There was a problem hiding this comment.
I'm not sure about this loop, might as well just make this:
for (var index = 0; index < pattern.elements.length; index++) {
const element = pattern.elements[index];|
Thanks for the reviews guys! |
I've implemented Object Binding Patterns.
They can be used in methods, functions and declarations.
They can have default values / initializers.
They can be nested.
They can have aliases.
They can also be exported.
I removed the throw from
transformVariableDeclarationsince TypeScript knows that all possible cases have been covered so it thinks the code is unreachable. It can be reached forcefully as evidenced in one specific test case. I removed this since TypeScript will never allow you to use afalsekeyword for a variable identifier.