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 all 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 20 additions & 3 deletions packages/babel-plugin-transform-spread/src/index.js
View file
Open in desktop
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,27 @@ 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(
process.env.BABEL_8_BREAKING
? "arrayLikeToArray"
: "arrayWithoutHoles",
),
[spreadLiteral],
);
}

nodes.push(spreadLiteral);
} else {
_props.push(prop);
}
Expand All @@ -62,7 +79,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 ...babel-plugin-transform-spread/test/fixtures/spread/nested-array-with-hole-babel-7/exec.js
View file
Open in desktop
@@ -0,0 +1,2 @@
let arr = ['a', ...['b',,'c']];
expect(2 in arr).toBe(true);
2 changes: 2 additions & 0 deletions ...abel-plugin-transform-spread/test/fixtures/spread/nested-array-with-hole-babel-7/input.js
View file
Open in desktop
@@ -0,0 +1,2 @@
var a = [...['a',, 'b']];
var b = ['a', ...['b',, 'c']];
3 changes: 3 additions & 0 deletions ...-plugin-transform-spread/test/fixtures/spread/nested-array-with-hole-babel-7/options.json
View file
Open in desktop
@@ -0,0 +1,3 @@
{
"BABEL_8_BREAKING": false
}
2 changes: 2 additions & 0 deletions ...bel-plugin-transform-spread/test/fixtures/spread/nested-array-with-hole-babel-7/output.js
View file
Open in desktop
@@ -0,0 +1,2 @@
var a = babelHelpers.arrayWithoutHoles(['a',, 'b']);
var b = ['a'].concat(babelHelpers.arrayWithoutHoles(['b',, 'c']));
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
@@ -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

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
@@ -0,0 +1,2 @@
var a = [...['a',, 'b']];
var b = ['a', ...['b',, 'c']];
3 changes: 3 additions & 0 deletions ...es/babel-plugin-transform-spread/test/fixtures/spread/nested-array-with-hole/options.json
View file
Open in desktop
@@ -0,0 +1,3 @@
{
"BABEL_8_BREAKING": true
}
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
@@ -0,0 +1,2 @@
var a = babelHelpers.arrayLikeToArray(['a',, 'b']);
var b = ['a'].concat(babelHelpers.arrayLikeToArray(['b',, 'c']));