Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ngcc): handle minified ES5 code #33777

Closed

Conversation

@petebacondarwin
Copy link
Member

petebacondarwin commented Nov 12, 2019

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@googlebot googlebot added the cla: yes label Nov 12, 2019
@IgorMinar

This comment has been minimized.

Copy link
Member

IgorMinar commented Nov 12, 2019

can you please provide more info in the commit message?

@petebacondarwin

This comment has been minimized.

Copy link
Member Author

petebacondarwin commented Nov 13, 2019

Yes this is still Draft

@petebacondarwin petebacondarwin force-pushed the petebacondarwin:ngcc-decorate-variant branch from 77ec0af to bde2603 Nov 13, 2019
@petebacondarwin petebacondarwin force-pushed the petebacondarwin:ngcc-decorate-variant branch from bde2603 to 5c2d6e5 Nov 13, 2019
@ngbot ngbot bot added this to the needsTriage milestone Nov 13, 2019
@petebacondarwin petebacondarwin marked this pull request as ready for review Nov 13, 2019
@petebacondarwin petebacondarwin requested a review from angular/fw-ngcc as a code owner Nov 13, 2019
Copy link
Member

gkalpak left a comment

👍
Commit message typo: statemetn --> statement

…rn statement

Previously we only removed `__decorate()` calls that looked like:

```
SomeClass = __decorate(...);
```

But in some minified scenarios this call gets wrapped up with the
return statement of the IIFE.

```
return SomeClass = __decorate(...);
```

This is now removed also, leaving just the return statement:

```
return SomeClass;
```
The reflection hosts have been updated to support the following
code forms, which were found in some minified library code:

* The class IIFE not being wrapped in parentheses.
* Calls to `__decorate()` being combined with the IIFE return statement.
@petebacondarwin petebacondarwin force-pushed the petebacondarwin:ngcc-decorate-variant branch from 5c2d6e5 to 5e2758e Nov 13, 2019
kara added a commit that referenced this pull request Nov 13, 2019
kara added a commit that referenced this pull request Nov 13, 2019
kara added a commit that referenced this pull request Nov 13, 2019
…rn statement (#33777)

Previously we only removed `__decorate()` calls that looked like:

```
SomeClass = __decorate(...);
```

But in some minified scenarios this call gets wrapped up with the
return statement of the IIFE.

```
return SomeClass = __decorate(...);
```

This is now removed also, leaving just the return statement:

```
return SomeClass;
```

PR Close #33777
kara added a commit that referenced this pull request Nov 13, 2019
The reflection hosts have been updated to support the following
code forms, which were found in some minified library code:

* The class IIFE not being wrapped in parentheses.
* Calls to `__decorate()` being combined with the IIFE return statement.

PR Close #33777
@kara kara closed this in 7f24975 Nov 13, 2019
kara added a commit that referenced this pull request Nov 13, 2019
kara added a commit that referenced this pull request Nov 13, 2019
…rn statement (#33777)

Previously we only removed `__decorate()` calls that looked like:

```
SomeClass = __decorate(...);
```

But in some minified scenarios this call gets wrapped up with the
return statement of the IIFE.

```
return SomeClass = __decorate(...);
```

This is now removed also, leaving just the return statement:

```
return SomeClass;
```

PR Close #33777
kara added a commit that referenced this pull request Nov 13, 2019
The reflection hosts have been updated to support the following
code forms, which were found in some minified library code:

* The class IIFE not being wrapped in parentheses.
* Calls to `__decorate()` being combined with the IIFE return statement.

PR Close #33777
@petebacondarwin petebacondarwin deleted the petebacondarwin:ngcc-decorate-variant branch Nov 13, 2019
@dioslaska

This comment has been minimized.

Copy link

dioslaska commented Nov 22, 2019

@petebacondarwin Does this also supposed to make ngcc work with renamed variables? It still fails for me with the latest rc3 version. Example with a simple test module and component:

Fails:

    import { Component as n, NgModule as t } from "@angular/core";
    import { CommonModule as r } from "@angular/common";

    var o = function() {
      function t() {}
      t.decorators = [ {
        type: n,
        args: [ {
          selector: "test-input",
          template: "<label><ng-content></ng-content><input /></label>"
        } ]
      } ];
      return t;
    }();

    var e = function() {
      function n() {}
      n.decorators = [ {
        type: t,
        args: [ {
          imports: [ r ],
          declarations: [ o ],
          exports: [ o ]
        } ]
      } ];
      return n;
    }();

    export { o as TestComponent, e as TestModule };

Works:

    import { Component, NgModule } from '@angular/core';
    import { CommonModule } from '@angular/common';

    var TestComponent = /** @class */ (function () {
        function TestComponent() {
        }
        TestComponent.decorators = [
            { type: Component, args: [{
                        selector: 'test-input',
                        template: "<label><ng-content></ng-content><input /></label>"
                    }] }
        ];
        return TestComponent;
    }());

    var TestModule = /** @class */ (function () {
        function TestModule() {
        }
        TestModule.decorators = [
            { type: NgModule, args: [{
                        imports: [CommonModule],
                        declarations: [TestComponent],
                        exports: [TestComponent]
                    },] }
        ];
        return TestModule;
    }());

    export { TestComponent, TestModule };
@dioslaska

This comment has been minimized.

Copy link

dioslaska commented Nov 22, 2019

@petebacondarwin

This comment has been minimized.

Copy link
Member Author

petebacondarwin commented Nov 22, 2019

@dioslaska - please open a new ticket for this issue rather than tacking it on to a closed pull request. This will make it easier to deal with the problem and track the work required.

I think the problem you are experiencing is that ngcc does not appear cope with renames at the point of export:

export { o as TestComponent, e as TestModule };
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.