new Class with side effects erroneously removed

[kzc]

Input

$ cat new.js

class Foo {
    constructor() {
        console.log("Foo");
    }
}
new Foo;

Expected output

$ node new.js

Foo

rollup@0.41.0 output

$ bin/rollup new.js -f es | node

⚠️   Generated an empty bundle

Suggested fix

--- a/src/ast/nodes/NewExpression.js
+++ b/src/ast/nodes/NewExpression.js
@@ -2,7 +2,7 @@ import Node from '../Node.js';
 import callHasEffects from './shared/callHasEffects.js';
 
 export default class NewExpression extends Node {
-       hasEffects ( scope ) {
-               return callHasEffects( scope, this.callee, true );
+       hasEffects () {
+               return true;
        }
 }

Rollup output with fix

$ bin/rollup new.js -f es

class Foo {
    constructor() {
        console.log("Foo");
    }
}
new Foo;

$ bin/rollup new.js -f es | node

Foo

A few existing rollup tests with incorrect results would have to be updated.

[kzc]

Related bug: #980

[Rich-Harris]

Ideally we'd be able to determine that the function to test for side-effects is the constructor. In the shorter term, what if we just include all new expressions where the callee is a class? #1234

[kzc]

The fix I suggested was a bit drastic because of the seriousness of the error. But if you want to refine it for side effects, by all means. The related bug #980 had a side effect in the super constructor.

[kzc]

what if we just include all new expressions where the callee is a class?

Even if the callee is a function we still want the new Func included if it has a side effect.

[Rich-Harris]

That already happens, it's only classes that were behaving badly.

About to leave my desk so will cut a release with the fix and we can address the false positives another time