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

Jump to bottom

Correctly transform spreads of arrays with holes #13439

Merged
merged 4 commits into from
Jun 10, 2021
Merged

Correctly transform spreads of arrays with holes #13439

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
87c5f03
Correctly transform spreads of arrays with holes
phapp88 Dec 7, 2018
4d927de
Use `arrayWithoutHoles` helper
nicolo-ribaudo Jun 9, 2021
f604e84
Add exec test
nicolo-ribaudo Jun 9, 2021
c685177
Update for Babel 8
nicolo-ribaudo Jun 9, 2021
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
19 changes: 16 additions & 3 deletions packages/babel-plugin-transform-spread/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ export default declare((api, options) => {
}
}

function hasHole(spread) {
return spread.elements.some(el => el === null);
}

function hasSpread(nodes) {
for (let i = 0; i < nodes.length; i++) {
if (t.isSpreadElement(nodes[i])) {
Expand All @@ -35,14 +39,23 @@ export default declare((api, options) => {
return [];
}

function build(props: Array, scope) {
function build(props: Array, scope, file) {
const nodes = [];
let _props = [];

for (const prop of props) {
if (t.isSpreadElement(prop)) {
_props = push(_props, nodes);
nodes.push(getSpreadLiteral(prop, scope));
let spreadLiteral = getSpreadLiteral(prop, scope);

if (t.isArrayExpression(spreadLiteral) && hasHole(spreadLiteral)) {
spreadLiteral = t.callExpression(
file.addHelper("arrayWithoutHoles"),
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we can call arrayLikeToArray since we know for sure spreadLiteral is an array.

Copy link
Member Author

Choose a reason for hiding this comment

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

arrayLikeToArray isn't available in every Babel 7 version, I went for the easy route. I'll add a switch to use arrayLikeToArray in Babel 8.

[spreadLiteral],
);
}

nodes.push(spreadLiteral);
} else {
_props.push(prop);
}
Expand All @@ -62,7 +75,7 @@ export default declare((api, options) => {
const elements = node.elements;
if (!hasSpread(elements)) return;

const nodes = build(elements, scope);
const nodes = build(elements, scope, this);
let first = nodes[0];

// If there is only one element in the ArrayExpression and
Expand Down
2 changes: 2 additions & 0 deletions packages/babel-plugin-transform-spread/test/fixtures/spread/nested-array-with-hole/exec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
let arr = ['a', ...['b',,'c']];
Copy link
Member

Choose a reason for hiding this comment

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

What happens if this isn't statically analyzable?

Copy link
Member Author

Choose a reason for hiding this comment

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

It uses toConsumableArray(spreadArgument), which does all the necessary checks to ensure that it's correct. arrayWithoutHoles is part of those checks.

Copy link
Member Author

@nicolo-ribaudo nicolo-ribaudo Jun 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

https://babeljs.io/repl#?browsers=defaults%2C%20not%20ie%2011%2C%20not%20ie_mob%2011&build=&builtIns=false&corejs=3.6&spec=false&loose=false&code_lz=DYUwLgBAhhC8EG0CMAaCA6TAPAukA&forceAllTransforms=true&presets=env

expect(2 in arr).toBe(true);
2 changes: 2 additions & 0 deletions packages/babel-plugin-transform-spread/test/fixtures/spread/nested-array-with-hole/input.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
var a = [...['a',, 'b']];
var b = ['a', ...['b',, 'c']];
2 changes: 2 additions & 0 deletions packages/babel-plugin-transform-spread/test/fixtures/spread/nested-array-with-hole/output.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
var a = babelHelpers.arrayWithoutHoles(['a',, 'b']);
var b = ['a'].concat(babelHelpers.arrayWithoutHoles(['b',, 'c']));