Skip to content

Tree-shaking for production environment drops component referred dynamically #2965

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

Closed
jaykhimani opened this issue Oct 31, 2016 · 12 comments
Closed

Comments

@jaykhimani
Copy link

jaykhimani commented Oct 31, 2016

Please provide us with the following information:

OS?

macOS Sierra, Ubuntu 16.04

Versions.

angular-cli: 1.0.0-beta.19-3
node: 6.9.1
os: darwin x64

Repro steps.

I've an angular-2 app build with angular-cli and one of the functionality is to list down a list of all the events and clicking on one event displays event details in 3 tabs - Overview, Photos & Tickets.
The way I've developed this is I've all the events stored in my db and also the content for Overview, Photos and Tickets tabs are in DB. So this way I can just have one component which renders 3 tabs and populates each tab content fetched from DB. Now the interesting bit is most of the events will have same layout and sections for each tab, but for some there can be different layout required. So to address this what I'm doing is along with the content of each tab (overview, photos, tickets), I also store the name of the component which will render this event.
I somehow managed to get the component from the name of the component using ComponentFactoryResolver and ViewContainerRef and all works fine, but problem arises when I try to make a build for production environment - ng build --prod - where angular-cli kicks webpack's tree-shaking and with that all my components which are not actually referenced/imported in the code but are dynamically created/rendered based on the data retrieved from db are dropped and my app crashes since it cannot find those components. In my NgModule definition, I've declared these dynamic components under entryComponents as well.

The log given by the failure.

error_handler.js:53Error: No component factory found for undefined
at e [as constructor] (errors.js:24)
at new e (component_factory_resolver.js:21)
at t.resolveComponentFactory (component_factory_resolver.js:30)
at tInjector.t.resolveComponentFactory (component_factory_resolver.js:55)
at t.ngOnChanges (control-factory.directive.ts:29)
at Wrapper_t.detectChangesInternal (wrapper.ngfactory.js:34)
at _View_t1.detectChangesInternal (component.ngfactory.js:146)
at _View_t1.t.detectChanges (view.js:229)
at _View_t0.t.detectContentChildrenChanges (view.js:247)
at _View_t0.detectChangesInternal (component.ngfactory.js:78)

@clydin
Copy link
Member

clydin commented Oct 31, 2016

Production builds minify the javascript so you can't and shouldn't rely on the type name. Also, using a private member of ComponentFactoryResolver will more than likely cause problems in the future.

For your use case (at least from what is described), using an ngSwitch in the template would probably be effective and cut down the complexity as well.

@Meligy
Copy link
Contributor

Meligy commented Nov 1, 2016

This is the expected Angular behaviour.

You need to add the components to entryComponents property in your @NgModule.

Here's the FAQ for entry components:
https://angular.io/docs/ts/latest/cookbook/ngmodule-faq.html#!#q-entry-component-defined

Relevant quotes from that page:

Most application components are loaded declaratively. Angular uses the component's selector to locate the element in the template. It then creates the HTML representation of the component and inserts it into the DOM at the selected element. These are not entry components.

A few components are only loaded dynamically and are never referenced in a component template.

The bootstrapped root AppComponent is an entry component. True, its selector matches an element tag in index.html. But index.html is not a component template and the AppComponent selector doesn't match an element in any component template.

The compiler can't discover these entry components by looking for them in other component templates. We must tell it about them ... by adding them to the entryComponents list.

If your app happens to bootstrap or dynamically load a component by type in some other manner, you'll have to add it to entryComponents explicitly.

Why does Angular need entryComponents?

Entry components are also declared. Why doesn't the Angular compiler generate code for every component in @NgModule.declarations? Then we wouldn't need entry components.

The reason is tree shaking. For production apps we want to load the smallest, fastest code possible. The code should contain only the classes that we actually need. It should exclude a component that's never used, whether or not that component is declared.

In fact, many libraries declare and export components we'll never use. The tree shaker will drop these components from the final code package if we don't reference them.

If the Angular compiler generated code for every declared component, it would defeat the purpose of the tree shaker.

Instead, the compiler adopts a recursive strategy that generates code only for the components we use.

It starts with the entry components, then it generates code for the declared components it finds in an entry component's template, then for the declared components it discovers in the templates of previously compiled components, and so on. At the end of the process, it has generated code for every entry component and every component reachable from an entry component.

If a component isn't an entry component or wasn't found in a template, the compiler omits it.

@jaykhimani
Copy link
Author

jaykhimani commented Nov 1, 2016

@clydin yes ngSwitch is an option, its only that I didnt want to change the parent component structure every time I add a new component for different layout, that just adds more maintenance. And yes I agree using using a private member is not a good idea, when the impl changes and my code will break. Regardless, point remains the same, since the component is imported in @NgModule and is also part of entryComponents, why its dropped when treeshaking kicks in.

@Meligy regarding your suggestion You need to add the components to entryComponents property in your @NgModule., I already have it (last line of my original comment).

@clydin
Copy link
Member

clydin commented Nov 1, 2016

The name gets mangled during minification; you can't rely on it. You'll either need to keep a lookup table to map names to types or use the ngSwitch method.
Personally, I would prefer the minimal extra maintenance to the entire app breaking after a future angular point release.

@jaykhimani
Copy link
Author

The name gets mangled during minification

dont understand this quite well (may be because am quite to new to webpack/angular world) hence not sure even if to consider this as a real issue or incorrect way of implementation. I've changed my implementation to have a internal mapping of name to component and moved forward. Anyway thanks for your input @clydin

@filipesilva
Copy link
Contributor

Very helpful answers from @clydin and @Meligy! I'm closing as you seem to have it sorted and because it's not something CLI specific per se. Most production builds would affect your code in the same way.

@matheo
Copy link

matheo commented Mar 29, 2017

Well, I have bad news about this.
I just followed the example on Dynamic Component Loading, and trusting this:

If your app happens to bootstrap or dynamically load a component by type in some other manner, you must add it to entryComponents explicitly.

I expected my App to work, but nope. When I build it for production with:

ng build -prod -bh ./

My dynamic components are dropped by webpack, and I get the same error reported on this issue:

Error: No component factory found for undefined. Did you add it to @NgModule.entryComponents?

But I did:


@NgModule({
  declarations: [
    AppComponent,
    WizardComponent,
    WizardStepComponent,
    WizardStepDirective,
    InitialStepComponent,
    SecondStepComponent
  ],
  imports: [
    BrowserModule,
    ReactiveFormsModule,
    StoreModule.provideStore(Reducers),
    NgbModule.forRoot()
  ],
  entryComponents: [
    InitialStepComponent,
    SecondStepComponent
  ],
  providers: [],
  bootstrap: [AppComponent]
})
export class AppModule { }

then I load them with:

loadStep() {
    if (this.step) {
      this.step.destroy();
    }

    let componentFactory = this._componentFactoryResolver.resolveComponentFactory(this.state.component);

    let viewContainerRef = this.stepHost.viewContainerRef;
    viewContainerRef.clear();

    this.step = viewContainerRef.createComponent(componentFactory);
  }

and the bundle for development works, -proddoesn't.

Thanks in advance for any tip @filipesilva

@clydin
Copy link
Member

clydin commented Mar 29, 2017

If 'this.state.component' is a string then my comment above applies: the names are mangled in production builds. Good practice is to not rely on them directly as this is intentional and desired; it reduces the deployed size and obfuscates the code.
As to implementation suggestions, you can switch to a declarative approach to defining your wizard or use an API that uses the component types directly instead of their names. For the second, you could potentially look at material2's dialog API for inspiration.

@matheo
Copy link

matheo commented Mar 29, 2017

this.state.component is a Component Type(?) not a String.
I'm passing InitialStepComponent and SecondStepComponent there.

@matheo
Copy link

matheo commented Mar 29, 2017

I'm using ngrx/store to manage the states, and the initial state:

const initial: WizardState = {
  component: InitialStepComponent
}

results in { component: undefined }, but as said, works with ng build without -prod.

@peterfeddo
Copy link

Had the exact same issue with similar architecture and had to hack a fix to remove uglify plugin from the build which is a suboptimal measure. Definitely need a more permanent resolution on this as using dynamic components is crucial to many applications.

@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants