Angular Compiler messes up return statement and duplicates comment in some arrow functions

[interrobrian]

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report 
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question
[ ] Other... Please describe:

Current behavior

When targeting es5, the Angular Compiler (ngc) produces incorrect JavaScript when it encounters a single statement arrow function with a line comment between the arrow and the statement. This bug prevents the body of the arrow function from executing. For example, this:

const foo = () =>
  // Print "bar"
  console.log("bar");

compiles into this, when using ngc:

var foo = function () {
    // Print "bar"
    return 
    // Print "bar"
    console.log("bar");
};

Not only did it duplicate the comment, but it put the duplicated comment on its own line after return, which causes the function to return before the actual code is run.

Expected behavior

The Angular Compiler should produce the same JavaScript that the TypeScript compiler does with this code:

var foo = function () {
    // Print "bar"
    return console.log("bar");
};

Minimal reproduction of the problem with instructions

Compile the above TypeScript with ngc. For reference, here is the tsconfig.json I used:

{
  "compilerOptions": {
    "target": "es5",
    "experimentalDecorators": true
  }
}

EDIT: More detailed reproduction here

What is the motivation / use case for changing the behavior?

This bug creates unwanted behavior that is very difficult to trace, especially when creating an Angular library. For example, it can make arrow functions used with an array forEach or filter return nothing.

Environment

Angular version: 6.0.0

Browser:
All
 
For Tooling issues:
- Node version: 8.9.4
- Platform:  Mac

[IgorMinar]

Can you please provide a reproduction? We generally don't mess around with the function body of app code so I'm not sure how could we be responsible for this but maybe there is some kind of odd interaction that is not obvious. A repro would help.

[interrobrian]

Sure, I can provide a more detailed reproduction. Since this deals with library compilation, I can't figure out a way to get it into a StackBlitz. However, here's how to reproduce it in a more realistic situation:

1. Create a fresh app with ng new my-app
2. Enter the app's directory and create a library with ng g library my-lib
3. Open up projects/my-lib/src/lib/my-lib.service.ts and add the following function:

public filterLessThanThree(numList: number[]): number[] {
  return numList.filter(n =>
    // Hey look I'm a comment
    n < 3);
}

4. Run ng build my-lib (Note that it's very important that you test this by building the lib and not by running the app with ng serve, as ng serve uses a different compilation path for local libraries)
5. At this point, you can find the function has been compiled incorrectly in the following versions:

dist/my-lib/bundles/my-lib.umd.js

...
MyLibService.prototype.filterLessThanThree = /**
    ...
    */
    function (numList) {
        return numList.filter(function (n) {
            // Hey look I'm a comment
            return;
        });
    };
...

dist/my-lib/bundles/my-lib.umd.min.js

... e.prototype.filterLessThanThree=function(e){return e.filter(function(e){})}, ...

dist/my-lib/esm5/lib/my-lib.service.js

MyLibService.prototype.filterLessThanThree = /**
    ...
    */
function (numList) {
    return numList.filter(function (n) {
        // Hey look I'm a comment
        return 
        // Hey look I'm a comment
        n < 3;
    });
};

dist/my-lib/fesm5/my-lib.js

MyLibService.prototype.filterLessThanThree = /**
    ...
    */
function (numList) {
    return numList.filter(function (n) {
        // Hey look I'm a comment
        return 
    });
};

If you'd like to further see how this will effect an app at runtime, you'll need to create a second app and reference the packaged output (simulating as if you had published the package to npm and somebody else is using it):

1. In the directory containing my-app and run ng new my-second-app
2. Open the package.json for my-second-app and add "my-lib": "file:../my-app/dist/my-lib" to your dependencies
3. Run npm install
4. Add the following to app.component.ts:

import { MyLibService } from "my-lib"
...
export class AppComponent {
  ...
  numListJSON: string;
  constructor(myLibService: MyLibService) {
    this.numListJSON = JSON.stringify(myLibService.filterLessThanThree([1, 2, 3, 4, 5]));
  }
}

5. Add {{ numListJSON }} somewhere in app.component.html.
6. Run ng serve and take a look at the app. Note that the JSON string on the page is [] when it should be [1, 2]

[trotyl]

Could reproduce by ngc, seems to be a tsickle issue when transformDecorators enabled.

[interrobrian]

Good find, @trotyl! Looks like this might be related to tsickle issue 57. I'll create a specific issue for this one as well and file it on there.

[interrobrian]

Filed as tsickle issue 802.

[mprobst]

The original tsickle issue has been fixed; it's kind of related in that it's about arrow function emit being a bit fickle. I've sent out a PR to fix angular/tsickle#802, it's caused by the source map transformer synthesizing comments in a way that breaks TS emit (maybe TS emit is also slightly wrong here, see microsoft/TypeScript#24096).

[JoostK]

Closing as resolved in angular/tsickle#802

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}