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

Improve array destructuring spec compliance #15183

Merged
merged 15 commits into from Nov 25, 2022
17 changes: 14 additions & 3 deletions packages/babel-helpers/src/helpers.ts
Expand Up @@ -788,7 +788,13 @@ helpers.iterableToArrayLimit = helper("7.0.0-beta.0")`
var _d = false;
var _s, _e;
try {
for (_i = _i.call(arr); !(_n = (_s = _i.next()).done); _n = true) {
_i = _i.call(arr);
if (i === 0) {
if (typeof _i !== "object" && typeof _i !== "function") return;
Copy link
Member

Choose a reason for hiding this comment

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

Use Object(_i) !== _i, which checks for both functions and objects but it excludes null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed!

_n = false;
return _arr;
}
for (; !(_n = (_s = _i.next()).done); _n = true) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm from mobile so this code might be completely wrong, but does it work?

for (_i = _i.call(arr); i < _arr.length && !(_n = (_s = _i.next()).done); _n = true) {
  _arr.push(_s.value);
}

We should try keeping these helpers as small as possible :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I change it to this:

try {
  for (_i = _i.call(arr); _arr.length < i && !(_n = (_s = _i.next()).done); _n = true) {
    _arr.push(_s.value);
  }
  if (i === 0) {
    if (Object(_i) !== _i) return;
    _n = false;
  }
} // ...

Does this looks good?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, that seems to be not working since the order of _n = true is changed... I'll revert the change.

try {
  _i = _i.call(arr);
  if (i === 0) {
    if (Object(_i) !== _i) return;
    _n = false;
  } else {
    for (; !(_n = (_s = _i.next()).done); _n = true) {
      _arr.push(_s.value);
      if(_arr.length === i) break;
    }
  }
} // ...

_arr.push(_s.value);
if (i && _arr.length === i) break;
}
Expand All @@ -812,8 +818,13 @@ helpers.iterableToArrayLimitLoose = helper("7.0.0-beta.0")`
if (_i == null) return;

var _arr = [];
for (_i = _i.call(arr), _step; !(_step = _i.next()).done;) {
_arr.push(_step.value);
_i = _i.call(arr);
if (i === 0) {
if (typeof _i !== "object" && typeof _i !== "function") return;
return _arr;
}
for (var _s; !(_s = _i.next()).done;) {
_arr.push(_s.value);
if (i && _arr.length === i) break;
}
return _arr;
Expand Down
@@ -0,0 +1,36 @@
expect(function () {
var [] = null;
}).toThrow("Invalid attempt to destructure non-iterable instance.\nIn order to be iterable, non-array objects must have a [Symbol.iterator]() method.");

expect(function () {
var [] = 42;
}).toThrow("Invalid attempt to destructure non-iterable instance.\nIn order to be iterable, non-array objects must have a [Symbol.iterator]() method.");

expect(function () {
var [] = {};
}).toThrow("Invalid attempt to destructure non-iterable instance.\nIn order to be iterable, non-array objects must have a [Symbol.iterator]() method.");

expect(function () {
var [] = { [Symbol.iterator]: function() {} };
}).toThrow("Invalid attempt to destructure non-iterable instance.\nIn order to be iterable, non-array objects must have a [Symbol.iterator]() method.");

var [] = [];
var [] = [0, 1, 2];
var [] = "foo";
var [] = (function*() { throw new Error("Should not throw"); })();
var [] = { [Symbol.iterator]: function() { return {}; } }
var [] = { [Symbol.iterator]: function() { return function() {}; } }
var [] = { [Symbol.iterator]: async function*() {} }

var returnCalled = false;
var [] = {
[Symbol.iterator]: function() {
return {
return: function() {
returnCalled = true;
return {};
}
};
}
};
expect(returnCalled).toStrictEqual(true);
@@ -0,0 +1,24 @@
var [] = null;
var [] = 42;
var [] = {};
var [] = { [Symbol.iterator]: function() {} };

var [] = [];
var [] = [0, 1, 2];
var [] = "foo";
var [] = (function*() { throw new Error("Should not throw"); })();
var [] = { [Symbol.iterator]: function() { return {}; } }
var [] = { [Symbol.iterator]: function() { return function() {}; } }
var [] = { [Symbol.iterator]: async function*() {} }

var returnCalled = false;
var [] = {
[Symbol.iterator]: function() {
return {
return: function() {
returnCalled = true;
return {};
}
};
}
};
@@ -0,0 +1,65 @@
var _ref = null,
_ref2 = babelHelpers.slicedToArray(_ref, 0);
var _ = 42,
_ref3 = babelHelpers.slicedToArray(_, 0);
var _ref4 = {},
_ref5 = babelHelpers.slicedToArray(_ref4, 0);
var _Symbol$iterator = {
[Symbol.iterator]: function () {}
},
_Symbol$iterator2 = babelHelpers.slicedToArray(_Symbol$iterator, 0);
var _ref6 = [0, 1, 2];
var _foo = "foo",
_foo2 = babelHelpers.slicedToArray(_foo, 0);
var _ref7 = /*#__PURE__*/babelHelpers.regeneratorRuntime().mark(function _callee() {
return babelHelpers.regeneratorRuntime().wrap(function _callee$(_context) {
while (1) {
switch (_context.prev = _context.next) {
case 0:
throw new Error("Should not throw");
case 1:
case "end":
return _context.stop();
}
}
}, _callee);
})(),
_ref8 = babelHelpers.slicedToArray(_ref7, 0);
var _Symbol$iterator3 = {
[Symbol.iterator]: function () {
return {};
}
},
_Symbol$iterator4 = babelHelpers.slicedToArray(_Symbol$iterator3, 0);
var _Symbol$iterator5 = {
[Symbol.iterator]: function () {
return function () {};
}
},
_Symbol$iterator6 = babelHelpers.slicedToArray(_Symbol$iterator5, 0);
var _Symbol$iterator7 = {
[Symbol.iterator]: /*#__PURE__*/babelHelpers.regeneratorRuntime().mark(function _callee2() {
return babelHelpers.regeneratorRuntime().async(function _callee2$(_context2) {
while (1) {
switch (_context2.prev = _context2.next) {
case 0:
case "end":
return _context2.stop();
}
}
}, _callee2, null, null, Promise);
})
},
_Symbol$iterator8 = babelHelpers.slicedToArray(_Symbol$iterator7, 0);
var returnCalled = false;
var _Symbol$iterator9 = {
[Symbol.iterator]: function () {
return {
return: function () {
returnCalled = true;
return {};
}
};
}
},
_Symbol$iterator10 = babelHelpers.slicedToArray(_Symbol$iterator9, 0);
Expand Up @@ -45,7 +45,7 @@ try {
expect(thrown).toEqual(true);
try {
thrown = false;
var _ = babelHelpers.toArray(void 0);
var _ = babelHelpers.slicedToArray(void 0, 0);
} catch (e) {
thrown = true;
}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

@@ -1,5 +1,5 @@
function f(_ref) {
var _ref2 = babelHelpers.toArray(_ref);
var _ref2 = babelHelpers.slicedToArray(_ref, 0);
return /*#__PURE__*/babelHelpers.regeneratorRuntime().mark(function _callee() {
return babelHelpers.regeneratorRuntime().wrap(function _callee$(_context) {
while (1) {
Expand Down
2 changes: 1 addition & 1 deletion packages/babel-traverse/src/scope/index.ts
Expand Up @@ -686,7 +686,7 @@ export default class Scope {
if (i === true) {
// Used in array-spread to create an array.
helperName = "toConsumableArray";
} else if (i) {
} else if (typeof i === "number") {
args.push(numericLiteral(i));

// Used in array-rest to create an array from a subset of an iterable.
Expand Down