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.

Sign up for GitHub

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

Move var declaration to in-place for spread in expressions #95

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Move var declaration to in-place for spread in expressions #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/program/types/CallExpression.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,20 @@ export default class CallExpression extends Node {
if (this.callee.object.type === 'Identifier') {
context = this.callee.object.name;
} else {
context = this.findScope(true).createDeclaration('ref');
const callExpression = this.callee.object;
code.prependRight(callExpression.start, `(${context} = `);

// Prepend the declaraction in place to prevent an expression
// begining with a parenthesis. Other types are guarded by
// an expression before the parenthesis so the declaration
// is created at the top of the scope.
if (this.parent.type === 'ExpressionStatement') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is not enough if the CallExpression is not the expression of the ExpressionStatement, like this:

const foo = { bar: [] }
const baz = true ? [1,2,3] : []
foo.bar.push(...baz)()

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. A better check here would be if the start of parent is also the start of the statement.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried the following a few days ago:

                                        } else {
                                                context = this.findScope(true).createDeclaration('ref');
                                                const callExpression = this.callee.object;
-                                           code.prependRight(callExpression.start, `(${context} = `);
+                                         const statement = this.findNearest(/Statement/);
+                                         code.prependRight(callExpression.start, (statement.start === callExpression.start ? ';' : '') + `(${context} = `);
                                                code.appendLeft(callExpression.end, `)`);
                                        }

That broke the following test, though:

function foo (x) {
        if ( x )
          return ref => (bar || baz).Test( ref, ...arguments );
      }

context = this.findScope(true).createIdentifier('ref');
code.prependRight(callExpression.start, `var ${context}; (${context} = `);
} else {
context = this.findScope(true).createDeclaration('ref');
code.prependRight(callExpression.start, `(${context} = `);
}

code.appendLeft(callExpression.end, `)`);
}
} else {
Expand Down
21 changes: 17 additions & 4 deletions test/samples/spread-operator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ module.exports = [
( foo || bar ).baz( ...values );`,

output: `
var ref;

(ref = ( foo || bar )).baz.apply( ref, values );`
var ref; (ref = ( foo || bar )).baz.apply( ref, values );`
},

{
Expand Down Expand Up @@ -585,5 +583,20 @@ module.exports = [
}; }
}
`
}
},

{
description:
'transpiles a spread operator with semicolon before parens',

input: `
const foo = { bar: [] }
const baz = true ? [1,2,3] : []
foo.bar.push(...baz)`,

output: `
var foo = { bar: [] }
var baz = true ? [1,2,3] : []
var ref; (ref = foo.bar).push.apply(ref, baz)`
},
];