prototype assignments create side-effects

[alexeagle]

Given this input:

export function makeDecorator(name, parentClass) {
    function DecoratorFactory(objOrType) {}
    DecoratorFactory.prototype = Object.create(parentClass.prototype);
    DecoratorFactory.prototype.toString = () => `@${name}`;
    return DecoratorFactory;
}
function Parent() {}

const foo = makeDecorator('foo', Parent);

We should tree-shake away everything to 0, but we don't: https://goo.gl/fqmTHt

Note that commenting out the two lines with .prototype does tree-shake.

/cc @mhevery

[alexeagle]

Here's another repro from @concavelenz https://goo.gl/3a7xLN

[dimvar]

John's repro can be slightly reduced to this:

function Parent() {}
/** @constructor */
function DecoratorFactory(objOrType) {}
DecoratorFactory.prototype = Object.create(Parent.prototype);
alert(Parent);

When I removed the Object.create call, everything compiled away. The issue might be that Object.create is defined in externs.

[mhevery]

Any update on this? We (angular team) would like to get some progress on this.

[MatrixFrog]

I think this would get removed if you set up the prototype chain using one of the methods the compiler knows about ($jscomp.inherits / goog.inherits) but it's not familiar with this way of doing that. I'd rather not add a new CodingConvention just for Angular but maybe it's necessary in this case.

[alexeagle]

To be fair, this isn't an Angular-specific coding convention. Closure Compiler should tree-shake code that doesn't depend on Closure Library, right?

[mhevery]

How could we do that in a way which would for in both closure as non-closure environment? We want to optimize our code for closure, but we can't require it. So calling goog.inherits becomes a problem.

[MatrixFrog]

Good point. I guess ClassName.prototype = Object.create(OtherClassName.prototype) could be a pattern that is recognized in one of the built-in coding conventions.

[alexeagle]

cool, can you make that change or do you want a PR/CL for it (in the latter case, can you point to an example change that hits the right tests?)

[Dominator008]

This might be related to https://github.com/google/closure-compiler/issues/1745

[MatrixFrog]

A PR would be great. The compiler knows that a call to goog.inherits(X, Y) is defining a class called X which inherits from Y, because it passes that call expression to the CodingConvention's getClassesDefinedByCall method: https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/ClosureCodingConvention.java#L98

Since ClassName.prototype = Object.create(OtherClassName.prototype) is an assignment rather than a call, we might have to refactor that to be more general (getClassesDefinedByNode) and then have the base CodingConvention return the appropriate thing for that kind of assignment statement.

But I may be on the wrong track entirely; I haven't looked at https://github.com/google/closure-compiler/issues/1745 yet.