File / class deleted during tree shake despite the fact it's being used

[ssmartin]

Is this a regression?

Yes, the previous version in which this bug was not present was: .Angular 7

Description

I have a service / configuration that we provide dynamically based on decorator.

Looks like this:

@Injectable()
@Configuration({ deps: [] })
export class VehiclePickerConfiguration extends NewPickerConfiguration {

    constructor() {
        super(VEHICLE_CONFIG);
    }
}

Basically we register (add to a static map) this configuration / service metadata (and many others) by using our custom annotation / decorator. Later on, this configuration / service is provided on the fly using the following code:

 injectorProviders = injectorProviders.concat({ provide: metaData.constructor, useClass: metaData.constructor, deps: metaData.deps });
        this.createInjector(injectorProviders);

Then this VehicleConfigurationis is requested / used in two other different files / classes like this:

let configuration: VehiclePickerConfiguration = this.getPickerConfiguration<VehiclePickerConfiguration>(FieldContainers.RentContract.RentContract_booking_vehiclePriceList_vehicle);

Long story short, we have some generic code to provide these services / configurations on the fly and this worked very well until Angular 8.
But since Angular 8 the whole file (vehicle.configuration) where this configuration is written is basically tree shake (deleted) during compile time and this gives us extra headaches and causing bus in our application.

IF i provide this VehicleConfiguration manually anywhere in the app then the file is not deleted anymore, but this in our case defeats the purpose of providing and creating the instances of these services ONLY when they are needed.

🔥 Exception or Error

The file is deleted most probably during the tree shake process even if it's used (imported and referenced in two other files / classess) and due to this we will have unexpected results (the vehicle configuration is not created and we get and undefined exception)

🌍 Your Environment

Angular CLI: 8.3.17
Node: 10.15.3
OS: win32 x64
Angular: 8.2.13
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.803.17
@angular-devkit/build-angular     0.803.17
@angular-devkit/build-optimizer   0.803.17
@angular-devkit/build-webpack     0.803.17
@angular-devkit/core              8.3.17
@angular-devkit/schematics        8.3.17
@angular/cli                      8.3.17
@ngtools/webpack                  8.3.17
@schematics/angular               8.3.17
@schematics/update                0.803.17
rxjs                              6.4.0
typescript                        3.4.5
webpack                           4.41.2

Thank you.

[alan-agius4]

This seems to be caused because previously you leveraged on a bug which caused classes that had custom decorators not being tree-shaken away.

This bug was fixes in version 8 and now unused classes with custom decorators are also tree-shaken away.

The file is deleted most probably during the tree shake process even if it's used (imported and referenced in two other files / classess)

Are you referring to the below?

let configuration: VehiclePickerConfiguration = 
this.getPickerConfiguration<VehiclePickerConfiguration>(FieldContainers.RentContract.RentContract_booking_vehiclePriceList_vehicle);

The above are type references only which will cause the VehiclePickerConfiguration to be tree shaken away unless it is referenced such as injected via DI.

[ssmartin]

The above are type references only which will cause the VehiclePickerConfiguration to be tree shaken away unless it is referenced such as injected via DI.

I understand and also noticed this ( that if I provide it manually or inject it then is not tree shake ), but this is not ok from mu point of view. Basically with this limitation the dynamic providing and injecting is not possible anymore. This for us is a major downgrade.
We also face similar issuues with Components that are created dynamically using the ComponentResolver and which is also not working fine anymore event thought the component is declared in the entryComponents section of its module. This also worked just fine until we upgraded to Angular 8. But for that I will open a different ticket.

Back to this topic, we don’t want to statically inject these services, we want to provide them when needed only and seems that currently this is not possible anymore?

[alan-agius4]

@ssmartin did you mean to share some code?

[alan-agius4]

Indeed, it is not possible to retain a piece of code which is not statically referenced.

A workaround would be to mark this used something along;

window.retain = [
  VehicleConfiguration 
]

Or add to VehicleConfiguration to the list of providers in your module.

The @inject without a reference is designed to tree-shake unused service. This was introduced in Angular version 6 under the name Tree Shakeable Providers. Tree Shakeable Providers are a way to define services and other things to be used by Angular’s dependency injection system that will be tree-shaken unless used.

https://angular.io/guide/dependency-injection-providers#tree-shakable-providers

[ssmartin]

Hi @alan-agius4

Sorry for the typo before - just wanted to highlight some part of your first response.

I am aware of these workarounds (I see them really as workarounds and not as a solution to my problem), but I still think that is really bad to delete the whole file when that type is imported and referenced in other parts of the applications, parts which is not dead code at all.

I'm not even sure if I should open the other ticket (which I mentioned above) because probably I'll get the same reply as here, to use the window.retain which for me it's really just an ugly workaround...

[alan-agius4]

but I still think that is really bad to delete the whole file when that type is imported and referenced in other parts of the applications, parts which is not dead code at all.

A type doesn't have any JS representation and is used for type-checking:

For example;

class Foo {}
let foo: Foo|undefined;
console.log(foo)

Will produce

class Foo { }
let foo;
console.log(foo);

Notice that In the above example class Foo in used.

Now when passing the above to a minifier like terser. It will produce the below;

console.log(void 0);

[ssmartin]

Hi @alan-agius4

I understand that, but I hope you can also understand that something that was supposed to help us (the tree shake) is now causing some ugly side effects when you want to do more advanced stuff like providing dynamically some services ONLY when they are needed.

[dgp1130]

Since decorators are assumed to not have side effects, this isn't really supported behavior within Angular.

[ssmartin]

Hi @dgp1130

OK, that's fine, but this is not really about decorators - it's about providing a way to dynamically provide some services via StaticInjector and then later being able to use / create instances of them by requesting them from the injector. This was working very well for us, and now it's not working anymore ...and not because Angular would not support it, but because the file is tree shake during the build process.
For us this is at least weird and kind of defeats the whole purpose of the tree shaking concept.

[alan-agius4]

Hi again @ssmartin,

For us this is at least weird and kind of defeats the whole purpose of the tree shaking concept.

Tree-shaking will remove unused side-effect free code. In Angular custom decorators on classes are treated to be side effect free meaning that classes with a custom decorators that are unused will be removed by design.

This was working very well for us, and now it's not working anymore ...and not because Angular would not support it,

Previously this used to work because of a bug in @angular-devkit/build-optimizer which caused classes with custom decorators not to be marked as pure and thus always retained in the final bundle. This bug used to cause custom decorators on the classes to allow side-effects at a global level, which in your cases were leveraging.

The current behaviour in Angular is that custom decorators are treated as side-effect free.

[ssmartin]

Hi @alan-agius4 and thank you for the reply and patience.

I understand all this, but would be nice if you would provide a special decorator or annotation that would prevent tree shaking to remove that class. The window.retain is an ugly hack / workaround, from my point of view.

Best regards,
Sergiu