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

Inline spread elements where possible #179

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion src/program/types/ArrayExpression.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import Node from '../Node.js';
import spread, { isArguments } from '../../utils/spread.js';
import spread, { isArguments, inlineSpreads } from '../../utils/spread.js';

export default class ArrayExpression extends Node {
initialise(transforms) {
Expand All @@ -26,6 +26,7 @@ export default class ArrayExpression extends Node {
super.transpile(code, transforms);

if (transforms.spreadRest) {
inlineSpreads(code, this, this.elements);
// erase trailing comma after last array element if not an array hole
if (this.elements.length) {
const lastElement = this.elements[this.elements.length - 1];
Expand Down
7 changes: 6 additions & 1 deletion src/program/types/CallExpression.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import Node from '../Node.js';
import spread, { isArguments } from '../../utils/spread.js';
import spread, { isArguments, inlineSpreads } from '../../utils/spread.js';
import removeTrailingComma from '../../utils/removeTrailingComma.js';

export default class CallExpression extends Node {
Expand All @@ -20,6 +20,11 @@ export default class CallExpression extends Node {
}

transpile(code, transforms) {
if (transforms.spreadRest && this.arguments.length) {
inlineSpreads(code, this, this.arguments);
// this.arguments.length may have changed, must retest.
}

if (transforms.spreadRest && this.arguments.length) {
let hasSpreadElements = false;
let context;
Expand Down
7 changes: 6 additions & 1 deletion src/program/types/NewExpression.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import Node from '../Node.js';
import spread, { isArguments } from '../../utils/spread.js';
import spread, { isArguments, inlineSpreads } from '../../utils/spread.js';
import removeTrailingComma from '../../utils/removeTrailingComma.js';

export default class NewExpression extends Node {
Expand All @@ -23,6 +23,11 @@ export default class NewExpression extends Node {
transpile(code, transforms) {
super.transpile(code, transforms);

if (transforms.spreadRest && this.arguments.length) {
inlineSpreads(code, this, this.arguments);
// this.arguments.length may have changed, must retest.
}

if (transforms.spreadRest && this.arguments.length) {
const firstArgument = this.arguments[0];
const isNew = true;
Expand Down
31 changes: 29 additions & 2 deletions src/program/types/ObjectExpression.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,35 @@ export default class ObjectExpression extends Node {
for (let i = 0; i < this.properties.length; ++i) {
const prop = this.properties[i];
if (prop.type === 'SpreadElement') {
spreadPropertyCount += 1;
if (firstSpreadProperty === null) firstSpreadProperty = i;
// First see if we can inline the spread, to save needing objectAssign.
const argument = prop.argument;
if (
argument.type === 'ObjectExpression' || (
argument.type === 'Literal' &&
typeof argument.value !== 'string'
)
) {
if (argument.type === 'ObjectExpression' && argument.properties.length > 0) {
// Strip the `...{` and the `}` with a possible trailing comma before it,
// leaving just the possible trailing comma after it.
code.remove(prop.start, argument.properties[0].start);
code.remove(argument.properties[argument.properties.length - 1].end, prop.end);
this.properties.splice(i, 1, ...argument.properties);
i--;
} else {
// An empty object, boolean, null, undefined, number or regexp (but NOT
// string) will spread to nothing, so just remove the element altogether,
// including a possible trailing comma.
code.remove(prop.start, i === this.properties.length - 1
? prop.end
: this.properties[i + 1].start);
this.properties.splice(i, 1);
i--;
}
} else {
spreadPropertyCount += 1;
if (firstSpreadProperty === null) firstSpreadProperty = i;
}
} else if (prop.computed && transforms.computedProperty) {
computedPropertyCount += 1;
if (firstComputedProperty === null) firstComputedProperty = i;
Expand Down
53 changes: 53 additions & 0 deletions src/utils/spread.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,59 @@ export function isArguments(node) {
return node.type === 'Identifier' && node.name === 'arguments';
}

export function inlineSpreads(
code,
node,
elements
) {
let i = elements.length;

while (i--) {
const element = elements[i];
if (!element || element.type !== 'SpreadElement') {
continue;
}
const argument = element.argument;
if (argument.type !== 'ArrayExpression') {
continue;
}
const subelements = argument.elements;
if (subelements.some(subelement => subelement === null)) {
// Not even going to try inlining spread arrays with holes.
// It's a lot of work (got to be VERY careful in comma counting for
// ArrayExpression, and turn blanks into undefined for
// CallExpression and NewExpression), and probably literally no one
// would ever benefit from it.
continue;
}
// We can inline it: drop the `...[` and `]` and sort out any commas.
const isLast = i === elements.length - 1;
if (subelements.length === 0) {
code.remove(
isLast && i !== 0
? elements[i - 1].end // Take the previous comma too
: element.start,
isLast
? node.end - 1 // Must remove trailing comma; element.end wouldn’t
: elements[i + 1].start);
} else {
// Strip the `...[` and the `]` with a possible trailing comma before it,
// leaving just the possible trailing comma after it.
code.remove(element.start, subelements[0].start);
code.remove(
// Strip a possible trailing comma after the last element
subelements[subelements.length - 1].end,
// And also a possible trailing comma after the spread
isLast
? node.end - 1
: element.end
);
}
elements.splice(i, 1, ...subelements);
i += subelements.length;
}
}

export default function spread(
code,
elements,
Expand Down
4 changes: 2 additions & 2 deletions test/samples/computed-properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -238,15 +238,15 @@ module.exports = [
let a = {
[foo] (x, y) {
return {
...{abc: '123'}
...c
};
},
};
`,
output: `
var a = {};
a[foo] = function (x, y) {
return Object.assign({}, {abc: '123'});
return Object.assign({}, c);
};
`
},
Expand Down
57 changes: 56 additions & 1 deletion test/samples/object-rest-spread.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,61 @@ for( var a = c.a, rest = objectWithoutProperties( c, ["a"] ), b = rest;; ) {}`
input: `for( var {...b} = c;; ) {}`,
output: `function objectWithoutProperties (obj, exclude) { var target = {}; for (var k in obj) if (Object.prototype.hasOwnProperty.call(obj, k) && exclude.indexOf(k) === -1) target[k] = obj[k]; return target; }
for( var rest = objectWithoutProperties( c, [] ), b = rest;; ) {}`
}
},

{
description: 'inlines object spread with one object',
input: `var obj = {...{a: 1}};`,
output: `var obj = {a: 1};`
},

{
description: 'inlines object spread with two objects',
input: `var obj = {...{a: 1}, ...{b: 2}};`,
output: `var obj = {a: 1, b: 2};`
},

{
description: 'inlines object spread with regular keys in between',
input: `var obj = { ...{a: 1}, b: 2, c: 3 };`,
output: `var obj = { a: 1, b: 2, c: 3 };`
},

{
description: 'inlines object spread mixed',
input: `var obj = { ...{a: 1}, b: 2, ...{c: 3}, e};`,
output: `var obj = { a: 1, b: 2, c: 3, e: e};`
},

{
description: 'inlines object spread very mixed',
options: {
objectAssign: 'Object.assign'
},
input: `var obj = { ...{a: 1}, b: 2, ...c, e};`,
output: `var obj = Object.assign({}, {a: 1, b: 2}, c, {e: e});`
},

{
description: 'inlines object spread without extraneous trailing commas',
options: {
objectAssign: 'Object.assign'
},
input: `
var obj = { ...{a: 1,}, b: 2, ...{c: 3,}, ...d, e, ...{f: 6,},};
obj = { a: 1, b: 2, };
obj = { a: 1, ...{b: 2} };
obj = { a: 1, ...{b: 2,} };
obj = { a: 1, ...{b: 2}, };
obj = { a: 1, ...{b: 2,}, };
`,
output: `
var obj = Object.assign({}, {a: 1, b: 2, c: 3}, d, {e: e, f: 6});
obj = { a: 1, b: 2, };
obj = { a: 1, b: 2 };
obj = { a: 1, b: 2 };
obj = { a: 1, b: 2, };
obj = { a: 1, b: 2, };
`
}
];
Loading