Custom decorators are stripped from AoT builds as of 0.0.47

[fiznool]

Short description of the problem:

An app which uses custom decorators (e.g. ngrx/effects) has those decorators stripped from the resulting production build.

What behavior are you expecting?

The custom decorators to not be stripped, as is the case in 0.0.46.

Steps to reproduce:

1. Clone the repo https://github.com/fiznool/counter-ionic2-ngrx-effects
2. Switch to the branch app-scripts-47
3. Run npm install
4. Run ionic run android --aot (or ios)
5. In the resulting app, increment the counter using the 'Increment' button.
6. Attempt to reset the counter with the 'Reset' button.

The 'Reset' button uses ngrx/effects, which in turn depends on a custom decorator, @Effect. This is stripped from the build with 0.0.47 so the effect does not run, and hence the counter is not reset.

Which @ionic/app-scripts version are you using?

0.0.47

Other information: (e.g. stacktraces, related issues, suggestions how to fix, stackoverflow links, forum links, etc)

Using ngrx/effects involves a custom decorator, @Effects. There are reports elsewhere in the Angular community that custom decorators are stripped from AoT builds in certain conditions (see here).

Here is an output of the compiled effects/counter.ts module in 0.0.46 and 0.0.47. Notice that the decorators are preserved in the former but stripped in the latter.

0.0.46

function(module, exports, __webpack_require__) {

"use strict";
/* harmony import */ var __WEBPACK_IMPORTED_MODULE_0__angular_core__ = __webpack_require__(0);
/* harmony import */ var __WEBPACK_IMPORTED_MODULE_1__ngrx_effects__ = __webpack_require__(254);
/* harmony import */ var __WEBPACK_IMPORTED_MODULE_2_rxjs_add_operator_map__ = __webpack_require__(538);
/* harmony import */ var __WEBPACK_IMPORTED_MODULE_2_rxjs_add_operator_map___default = __webpack_require__.n(__WEBPACK_IMPORTED_MODULE_2_rxjs_add_operator_map__);
/* harmony import */ var __WEBPACK_IMPORTED_MODULE_3__reducers_counter__ = __webpack_require__(104);
/* harmony export (binding) */ __webpack_require__.d(exports, "a", function() { return CounterEffects; });
var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) {
    var c = arguments.length, r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc, d;
    if (typeof Reflect === "object" && typeof Reflect.decorate === "function") r = Reflect.decorate(decorators, target, key, desc);
    else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r;
    return c > 3 && r && Object.defineProperty(target, key, r), r;
};
var __metadata = (this && this.__metadata) || function (k, v) {
    if (typeof Reflect === "object" && typeof Reflect.metadata === "function") return Reflect.metadata(k, v);
};




var CounterEffects = (function () {
    function CounterEffects(actions$) {
        this.actions$ = actions$;
        this.reset$ = this.actions$
            .ofType(__WEBPACK_IMPORTED_MODULE_3__reducers_counter__["c" /* RESET */])
            .map(function () { return ({ type: __WEBPACK_IMPORTED_MODULE_3__reducers_counter__["d" /* RESET_SUCCESS */] }); });
    }
    CounterEffects.decorators = [
        { type: __WEBPACK_IMPORTED_MODULE_0__angular_core__["c" /* Injectable */] },
    ];
    /** @nocollapse */
    CounterEffects.ctorParameters = [
        { type: __WEBPACK_IMPORTED_MODULE_1__ngrx_effects__["a" /* Actions */], },
    ];
    __decorate([
        __webpack_require__.i(__WEBPACK_IMPORTED_MODULE_1__ngrx_effects__["b" /* Effect */])(), 
        __metadata('design:type', Object)
    ], CounterEffects.prototype, "reset$", void 0);
    return CounterEffects;
}());

0.0.47

function(module, exports, __webpack_require__) {

"use strict";
/* harmony import */ var __WEBPACK_IMPORTED_MODULE_0_rxjs_add_operator_map__ = __webpack_require__(534);
/* harmony import */ var __WEBPACK_IMPORTED_MODULE_0_rxjs_add_operator_map___default = __webpack_require__.n(__WEBPACK_IMPORTED_MODULE_0_rxjs_add_operator_map__);
/* harmony import */ var __WEBPACK_IMPORTED_MODULE_1__reducers_counter__ = __webpack_require__(178);
/* harmony export (binding) */ __webpack_require__.d(exports, "a", function() { return CounterEffects; });


var CounterEffects = (function () {
    function CounterEffects(actions$) {
        this.actions$ = actions$;
        this.reset$ = this.actions$
            .ofType(__WEBPACK_IMPORTED_MODULE_1__reducers_counter__["a" /* RESET */])
            .map(function () { return ({ type: __WEBPACK_IMPORTED_MODULE_1__reducers_counter__["b" /* RESET_SUCCESS */] }); });
    }
    return CounterEffects;
}());

[iron9light]

I think this should be a bug of angular. angular/angular-cli#2799

[fiznool]

That issue only applies to the angular cli. Ionic doesn't use the angular cli so therefore I don't believe it is relevant.

[iron9light]

1. ionic-cli will call ngc
2. I have the same issue, no matter it's ionic-app-script v0.0.46 or v0.0.47 or even early version.

[fiznool]

ngc is the angular compiler. This isn't the same as the angular CLI.

I've produced a minimal reproduction via the repository at https://github.com/fiznool/counter-ionic2-ngrx-effects which demonstrates the issue for 0.0.47 but not 0.0.46. So chances are you are seeing some other issue. One thing to watch out for is the syntax for NgModule. I found this resource incredibly helpful to sort out my AoT issues.

[iron9light]

Maybe you're right.
But after I use my private version of ngrx/effects which will not depend on any decorator, everything works again.
It's a bug anyway, no matter whose. ;-)

[danbucholtz]

This is something we'll need to investigate. We are removing decorators because decorators prevent code from being tree shakeable. We'll chat with Rob for Angular about ngrx to see what the correct course of action to take is. Maybe we only remove decorators from code that is processed by AoT. I'm not entirely sure how one would determine that.

Thanks,
Dan

[Manduro]

Meanwhile, is there a config option we can use to disable this behaviour?

[fiznool]

I don't know if it makes any difference but there is a temporary fix available from the angular team to preserve custom decorators.

[nebarf]

Thank u for the suggestion @fiznool. What file do we need to modify in a Ionic project to get it still works?

[danbucholtz]

Hi all,

This is resolved in 0.0.48 which was just published.

Please let me know if you have any issues.

Thanks,
Dan

[nebarf]

I still have the same problem as before.

I've just given a try on @fiznool test repo upgrading app-scripts version to 0.0.48.

[fiznool]

@danbucholtz could you confirm that this fix is in place for webpack builds?

I can see from the relevant commit that this appears to be in place for rollup builds only, but perhaps I'm mistaken.

[danbucholtz]

@fiznool,

Yeah, that is the correct commit. I don't think we actually do that process for webpack yet.

Thanks,
Dan

[joewoodhouse]

I guess this needs to be re-opened then, if the change hasn't been applied to the default (webpack) build? Presumably webpack is what 99% of people are using?

[MarkChrisLevy]

@Francesco1992 @fiznool @joewoodhouse @danbucholtz Decorators are removed in webpack build as well - look at aot-compiler.ts, there is a method removeDecorators from utils/typescript-utils.ts, which is executed during AoT compilation. I've just tested and when removeDecorators are not executed everything works as expected.

https://github.com/driftyco/ionic-app-scripts/blob/master/src/aot/aot-compiler.ts#L127

[danbucholtz]

@joewoodhouse,

I am thinking I am misunderstanding how ngrx uses decorators. Can you share a very simple example from the docs?

Thanks,
Dan

[joewoodhouse]

Something like these? https://github.com/ngrx/effects/blob/master/docs/intro.md https://github.com/ngrx/example-app/blob/master/src/app/effects/collection.ts

[fiznool]

@danbucholtz the most minimal reproduction can be found at the repo linked above in the original issue. It's an Ionic app which uses ngrx/effects.

Here's the link again: https://github.com/fiznool/counter-ionic2-ngrx-effects

[MarkChrisLevy]

@danbucholtz Just to clarify - I'm not using ngrx, but rather I have my own decorators for JSON serialization and those decorators are removed as well. I really don't understand why decorators must be removed... I can understand, that angular decorators must be removed in AoT, as they are not needed, but all others are as important and needed as any other js code.

[danbucholtz]

Decorators fundamentally make code not tree shakable. We'll do some more experimentation. We're not big fans of decorators as it stands now. We'll have to figure this out.

Thanks,
Dan

[MarkChrisLevy]

@danbucholtz Ok, that is a good point, but... afaik webpack by default doesn't do tree shaking?

[danbucholtz]

@lleevvyy, it does but not a very good job of it. This is something we're focused on. We want 200KB bundles, not 2000KB bundles 😄

Thanks,
Dan

[MarkChrisLevy]

@danbucholtz Indeed, that would be something :-) In my app's case, js output is almost 3000KB, however... I need those decorators as well :-) :-) Thanks for you work guys, I'm really appreciated!

[pdrosos]

I have the exactly same issue. My app's production build is completely broken after the upgrade to ionic 2 rc.4 with ionic-app-scripts 0.0.48, as I use ngrx/store with ngrx/effects almost everywhere. At least now I know the reason. I must temporary downgrade until there's some solution of this issue :(.
@danbucholtz please let us know what to do as soon as possible. Thanks!

[fiznool]

If I'm following this correctly, decorators are preserved if you use rollup to build for production. As per the docs, setting "ionic_bundler": "rollup" in your package.json should solve this issue?

If you wanted to stick with webpack for dev mode and switch to rollup for production builds, I have no idea how that would work though!

[pdrosos]

@fiznool I have tried that, but I have issues with 3rd party libraries and rollup. Will see if I can fix them with custom config file, it's not working for now. Thanks!

[MarkChrisLevy]

@pdrosos @fiznool An ugly hack (but it works) is to change line 101 (ver 0.0.48) of aot-compiler.js (node_modules/@ionic/app-scripts/dist/aot) to below:
var cleanedFileContent = tsFile.content;

[Manduro]

Meanwhile you can use ionic-app-scripts build --aot --minifyJs --minifyCss, which essentially leaves out the --optimizeJs that is removing decorators compared to ionic-app-scripts build --prod.

[pdrosos]

I made a production build with rollup with ionic build --prod, but ngrx/effects still doesn't work...
@Manduro thanks, tried this both with rollup and webpack, but it also doesn't work.
It seems to me that the --aot makes it stop working, because dev builds work without a problem.

[pdrosos]

@lleevvyy Thanks a lot! This one is the only way, which works for me too. Most probably will use it as a temporary hack until a solution appears.

[fiznool]

@lleevvyy great work in figuring out a workaround. I've submitted a PR which adds this in. IMHO the workaround is sound, because this is the approach the Angular team are taking with the ng CLI tool.

[fiznool]

@danbucholtz has there been any further discussion / progress within the Ionic team regarding this issue?

[danbucholtz]

I can merge your PR for 0.0.49.

Thanks,
Dan

[danbucholtz]

This issue will be resolved in 0.0.49 which will go out tomorrow-ish. I'll close the issue when it does.

Thanks,
Dan

[danbucholtz]

We published 1.0.0 of app-scripts tonight! 🎉

Thanks,
Dan