Repository: babel/babili
Pull request #523 (2017-05-05 19:10:33)

Fix collapse with circular reference
This PR fix the bug for:

```js
var foo = {};
foo.bar = foo;

var foo2 = [];
foo2[2] = foo2;

var foo3 = new Set([])
foo3.add(foo3)
```

Will convert to:

```js
var foo = {bar:foo};

var foo2 = [,,foo2];

var foo3 = new Set([foo3])
```

It will get `undefined` for `var`, a ReferenceError for `let` and `const`.

I catched the problem with [`prop-types`](https://github.com/reactjs/prop-types/blob/master/factoryWithThrowingShims.js#L51) module.

codecov[bot]: # [Codecov](https://codecov.io/gh/babel/babili/pull/523?src=pr&el=h1) Report
> Merging [#523](https://codecov.io/gh/babel/babili/pull/523?src=pr&el=desc) into [master](https://codecov.io/gh/babel/babili/commit/9a5bf2aa323b71b04aade0ea2f83dde4db58a29f?src=pr&el=desc) will **increase** coverage by `0.03%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/babel/babili/pull/523/graphs/tree.svg?height=150&width=650&token=yjfidKkIaL&src=pr)](https://codecov.io/gh/babel/babili/pull/523?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #523      +/-   ##
==========================================
+ Coverage   83.06%   83.09%   +0.03%     
==========================================
  Files          34       34              
  Lines        2539     2544       +5     
  Branches      907      911       +4     
==========================================
+ Hits         2109     2114       +5     
  Misses        257      257              
  Partials      173      173
```


| [Impacted Files](https://codecov.io/gh/babel/babili/pull/523?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...rm-inline-consecutive-adds/src/object-collapser.js](https://codecov.io/gh/babel/babili/pull/523?src=pr&el=tree#diff-cGFja2FnZXMvYmFiZWwtcGx1Z2luLXRyYW5zZm9ybS1pbmxpbmUtY29uc2VjdXRpdmUtYWRkcy9zcmMvb2JqZWN0LWNvbGxhcHNlci5qcw==) | `94.11% <100%> (+1.01%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/babel/babili/pull/523?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/babel/babili/pull/523?src=pr&el=footer). Last update [9a5bf2a...c0710a9](https://codecov.io/gh/babel/babili/pull/523?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/packages/babel-plugin-transform-inline-consecutive-adds/src/index.js b/packages/babel-plugin-transform-inline-consecutive-adds/src/index.js
index 345d678..0f9a598 100644
--- a/packages/babel-plugin-transform-inline-consecutive-adds/src/index.js
+++ b/packages/babel-plugin-transform-inline-consecutive-adds/src/index.js
@@ -142,7 +142,7 @@ function getContiguousStatementsAndExpressions(
 
 function getReferenceChecker(references) {
   // returns a function s.t. given an expr, returns true iff expr is an ancestor of a reference
-  return expr => references.some(r => r.isDescendant(expr));
+  return expr => references.some(r => r === expr || r.isDescendant(expr));
 }
 
 function tryUseCollapser(t, collapser, varDecl, topLevel, checkReference) {
diff --git a/packages/babel-plugin-transform-inline-consecutive-adds/src/set-collapser.js b/packages/babel-plugin-transform-inline-consecutive-adds/src/set-collapser.js
index 313cad3..c23bfe4 100644
--- a/packages/babel-plugin-transform-inline-consecutive-adds/src/set-collapser.js
+++ b/packages/babel-plugin-transform-inline-consecutive-adds/src/set-collapser.js
@@ -44,7 +44,7 @@ class SetCollapser extends Collapser {
       if (args.length !== 1) {
         return false;
       }
-      if (checkReference(args)) {
+      if (checkReference(args[0])) {
         return false;
       }
       return true;


Test patch:
diff --git a/packages/babel-plugin-transform-inline-consecutive-adds/__tests__/transform-inline-consecutive-adds-test.js b/packages/babel-plugin-transform-inline-consecutive-adds/__tests__/transform-inline-consecutive-adds-test.js
index b25b389..6ce2ce2 100644
--- a/packages/babel-plugin-transform-inline-consecutive-adds/__tests__/transform-inline-consecutive-adds-test.js
+++ b/packages/babel-plugin-transform-inline-consecutive-adds/__tests__/transform-inline-consecutive-adds-test.js
@@ -212,6 +212,16 @@ describe("transform-inline-consecutive-adds-plugin", () => {
     expect(transform(source)).toBe(source);
   });
 
+  it("should not collapse computed properties with circular reference", () => {
+    const source = unpad(
+      `
+      var foo = {};
+      foo.bar = foo;
+    `
+    );
+    expect(transform(source)).toBe(source);
+  });
+
   it("should collapse statements with multiple assignments", () => {
     const source = unpad(
       `
@@ -291,6 +301,16 @@ describe("transform-inline-consecutive-adds-plugin", () => {
     expect(transform(source)).toBe(expected);
   });
 
+  it("should not collapse statements for array-initialized sets with circular reference", () => {
+    const source = unpad(
+      `
+      var foo = new Set([1, 2]);
+      foo.add(foo);
+    `
+    );
+    expect(transform(source)).toBe(source);
+  });
+
   it("should collapse array property assignments", () => {
     const source = unpad(
       `
@@ -373,4 +393,14 @@ describe("transform-inline-consecutive-adds-plugin", () => {
     );
     expect(transform(source)).toBe(expected);
   });
+
+  it("should not collapse array property assignments if it is circular reference", () => {
+    const source = unpad(
+      `
+      var foo = [];
+      foo[2] = foo;
+    `
+    );
+    expect(transform(source)).toBe(source);
+  });
 });