Build optimizer update

[filipesilva]

This transform exists as a workaround for the following TS issues:

• Better comment emit on tokens microsoft/TypeScript#17689
• Keep leadingComments of CallExpression microsoft/TypeScript#17606

It forces call expressions in a whitelist to have a leading PURE comment.

Once TS fixes this problem for call expressions, Build Optimizer should be updated and this transform removed.

[mhevery]

instead of listing them here, would it not be better to to leave some kind of comment in the original source code which would mark the function as pure? This way as functions are discovered they can be added automatically not just by Angular team but by application developers?

[mhevery]

I see this is a hack for the

• Better comment emit on tokens microsoft/TypeScript#17689
• Keep leadingComments of CallExpression microsoft/TypeScript#17606

But it sounds like from reading the Issue that the comment is there but it is in a wrong location?

[mhevery]

#__PURE__ is also valid per https://github.com/mishoo/UglifyJS2

[filipesilva]

@mhevery I tried to see if the comment was indeed somewhere else by adding the following code to a visitor:

const nodeStart = node.getStart();
const nodeEnd = node.getEnd();
const nodeText = sf.text.substr(nodeStart, nodeEnd - nodeStart);
console.log('comments for', nodeText)
console.log(ts.getSyntheticLeadingComments(node))
console.log(ts.getSyntheticLeadingComments(node))
console.log(ts.getLeadingCommentRanges(sf.text, node.pos))
console.log(ts.getTrailingCommentRanges(sf.text, node.pos))
console.log('###')

Then I tested for this input: var metaCtor = /*@__PURE__*/ makeMetadataCtor(props);:

comments for var metaCtor = /*@__PURE__*/ makeMetadataCtor(props);
undefined
undefined
undefined
undefined
###
comments for var metaCtor = /*@__PURE__*/ makeMetadataCtor(props)
undefined
undefined
undefined
undefined
###
comments for metaCtor = /*@__PURE__*/ makeMetadataCtor(props)
undefined
undefined
undefined
undefined
###
comments for metaCtor
undefined
undefined
undefined
undefined
###
comments for makeMetadataCtor(props)
undefined
undefined
undefined
[ { kind: 3, pos: 15, end: 28, hasTrailingNewLine: false } ]
###
comments for makeMetadataCtor
undefined
undefined
undefined
[ { kind: 3, pos: 15, end: 28, hasTrailingNewLine: false } ]
###
comments for props
undefined
undefined
undefined
undefined
###

The comment is indeed there.

I think it is possible to make a general purpose transform to add back lost comments but at that point it would be better to just invest time fixing the bug in TS instead.

It's important to note that Build Optimizer can update its TS version independently of the project TS, so it does not need to wait for Angular to support a newly released TS.

I'd also like some clarification on what is truly needed for the Bazel integration though... It seems to me that it is incidental that these comments are being added to those functions prior to Build Optimizer running. If all that is needed is that they be there prior to Uglify, then perhaps it is better to have Build Optimizer add them instead.

[clydin]

Uglify also has the pure_funcs option which allows an array of function names to be passed which are to be considered to always have no side effects. For minifying a bundle, it should have the same effect.

However, the danger of assuming that a function with a particular name has no side effects (whether using this transformer or the pure_funcs option) is that all functions with the name will be affected. This could be problematic when a bundle is used; as application code or third-party vendor code could potentially result in name collisions. This has the potential to result in functions with side effects being assumed to have no side effects and therefore removed.

[mhevery]

• Do you have an estimate on fixing it in TS?
• Let's get this in, and see what else gets unblocked

[filipesilva]

@mhevery I'm not sure this PR needs to be merged actually. In angular/angular#21880 what prevented Build Optimizer from shedding as much code was mostly configuration items. Parity with closure was reached without these changes.

[mhevery]

Ok, let's close it for now...

…

On Tue, Jan 30, 2018 at 9:16 AM, Filipe Silva ***@***.***> wrote: @mhevery <https://github.com/mhevery> I'm not sure this PR needs to be merged actually. In angular/angular#21880 <angular/angular#21880> what prevented Build Optimizer from shedding as much code was mostly configuration items. Parity with closure was reached without these changes. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#412 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAG1T7dhOXmo4gfOALGKFWz9F3eREJO9ks5tP05PgaJpZM4RuRFx> .