From c4f6a7a06f9ec4b3203da1a372159f4807d13b08 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 11 Sep 2017 11:16:14 -0400 Subject: [PATCH] Add failing test case for object rest after array rest, and fix it. (#6213) * Add failing test case for object rest after array rest. Discovered while upgrading https://github.com/meteor/babel to Babel 7. The error is: 1) babel-plugin-transform-object-rest-spread/object rest with array rest: TypeError: /Users/ben/dev/babel/packages/babel-plugin-transform-object-rest-spread/test/fixtures/object-rest/with-array-rest/actual.js: Property id of VariableDeclarator expected node to be of a type ["LVal"] but instead got null at Object.validate (packages/babel-types/lib/definitions/index.js:73:13) at validate (packages/babel-types/lib/index.js:460:9) at Object.builder (packages/babel-types/lib/index.js:428:7) at Object.RestElement (packages/babel-plugin-transform-object-rest-spread/lib/index.js:157:41) at NodePath._call (packages/babel-traverse/lib/path/context.js:53:20) at NodePath.call (packages/babel-traverse/lib/path/context.js:40:17) at NodePath.visit (packages/babel-traverse/lib/path/context.js:84:12) ... * Fix object rest following array rest. (#6213) * Avoid treating array ...rest elements as object ...rest properties. * Also avoid treating ...rest parameters as object ...rest properties. Returning early if the parent was an ArrayPattern was not quite enough, since a RestElement can appear as a parameter in a Function as well. * Move RestElement parent check earlier in visitor method. --- .../src/index.js | 19 +++++++++++++++---- .../object-rest/with-array-rest/actual.js | 8 ++++++++ .../object-rest/with-array-rest/expected.js | 9 +++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 packages/babel-plugin-transform-object-rest-spread/test/fixtures/object-rest/with-array-rest/actual.js create mode 100644 packages/babel-plugin-transform-object-rest-spread/test/fixtures/object-rest/with-array-rest/expected.js diff --git a/packages/babel-plugin-transform-object-rest-spread/src/index.js b/packages/babel-plugin-transform-object-rest-spread/src/index.js index c96358fed1..a33cff0217 100644 --- a/packages/babel-plugin-transform-object-rest-spread/src/index.js +++ b/packages/babel-plugin-transform-object-rest-spread/src/index.js @@ -65,9 +65,11 @@ export default function({ types: t }) { //expects path to an object pattern function createObjectSpread(path, file, objRef) { - const last = path.get("properties").pop(); // note: popping does not mean removal from path + const props = path.get("properties"); + const last = props[props.length - 1]; + t.assertRestElement(last.node); const restElement = t.clone(last.node); - last.remove(); // remove restElement + last.remove(); const impureComputedPropertyDeclarators = replaceImpureComputedKeys(path); const { keys, allLiteral } = extractNormalizedKeys(path); @@ -144,6 +146,13 @@ export default function({ types: t }) { path.get("id").traverse( { RestElement(path) { + if (!path.parentPath.isObjectPattern()) { + // Return early if the parent is not an ObjectPattern, but + // (for example) an ArrayPattern or Function, because that + // means this RestElement is an not an object property. + return; + } + if ( // skip single-property case, e.g. // const { ...x } = foo(); @@ -197,6 +206,8 @@ export default function({ types: t }) { callExpression, ] = createObjectSpread(objectPatternPath, file, ref); + t.assertIdentifier(argument); + insertionPath.insertBefore(impureComputedPropertyDeclarators); insertionPath.insertAfter( @@ -205,8 +216,8 @@ export default function({ types: t }) { insertionPath = insertionPath.getSibling(insertionPath.key + 1); - if (path.parentPath.node.properties.length === 0) { - path + if (objectPatternPath.node.properties.length === 0) { + objectPatternPath .findParent( path => path.isObjectProperty() || path.isVariableDeclarator(), diff --git a/packages/babel-plugin-transform-object-rest-spread/test/fixtures/object-rest/with-array-rest/actual.js b/packages/babel-plugin-transform-object-rest-spread/test/fixtures/object-rest/with-array-rest/actual.js new file mode 100644 index 0000000000..acbe7b35fb --- /dev/null +++ b/packages/babel-plugin-transform-object-rest-spread/test/fixtures/object-rest/with-array-rest/actual.js @@ -0,0 +1,8 @@ +let { + a: [b, ...arrayRest], + c = function(...functionRest){}, + ...objectRest +} = { + a: [1, 2, 3, 4], + d: "oyez" +}; diff --git a/packages/babel-plugin-transform-object-rest-spread/test/fixtures/object-rest/with-array-rest/expected.js b/packages/babel-plugin-transform-object-rest-spread/test/fixtures/object-rest/with-array-rest/expected.js new file mode 100644 index 0000000000..743129f1bb --- /dev/null +++ b/packages/babel-plugin-transform-object-rest-spread/test/fixtures/object-rest/with-array-rest/expected.js @@ -0,0 +1,9 @@ +let _a$d = { + a: [1, 2, 3, 4], + d: "oyez" +}, + { + a: [b, ...arrayRest], + c = function (...functionRest) {} +} = _a$d, + objectRest = babelHelpers.objectWithoutProperties(_a$d, ["a", "c"]);