Skip to content

@ngtools/webpack feature request: support code splitting by third parties #4541

Closed

Description

@christopherthielen

This is a feature request.

I'd like to start discussion around @ngtools/webpack supporting code splitting by third party libs (including ui-router-ng2).

Currently the code splitting support in @ngtools/webpack has a soft dependency on @angular/router.

My understanding is that:

Goal

I think the goal should be to eliminate the dependency on @angular/router and provide a router agnostic mechanism for third parties to supply code splitting information.

Today, UI-Router supports code splitting using @ngtools/webpack by importing the ROUTES token from @angular/router, then providing the ui-router states as that DI token, i.e., { provide: ROUTES, useValue: module.states, multi: true }. However, this forces ui-router to have a dependency on @angular/router. It also forces ui-router to emulate the structure of @angular/router by adding a loadChildren property.

Proposal 1

Make the DI token configurable by @ngtools/webpack.

Do not reference the token from @angular/router in ngtools-impl.ts, but pass the token in. UI-Router users could configure this somehow to use the STATES token, for example.

This proposal is rather limited in its usefulness. Only one library could support lazy loading + code splitting in a given app.

Proposal 2

Move and/or rename the DI token out of @angular/router

Perhaps the DI token could be moved to @angular/compiler-cli, @angular/common, or @angular/core. This eliminates the need to depend on @angular/router. Additionally, move the token to its own ES6 module. Today, importing the ROUTES token from @angular/router brings in unrelated symbols from @angular/router into the ui-router bundle even using rollup.

This proposal still conceptually ties lazy loading to the @angular/router. All terminology and code analysis assumes that lazy loading code processes @angular/router routes. Third parties have to emulate the router's structure.

Proposal 3

Introduce a routing-agnostic token such as ANALYZE_FOR_LAZY_LOAD.

Either @angular/router or third parties should provide lazy load and code splitting information using this token. I propose instead of providing an array of specifically structured objects such as ROUTES or STATES, that only the lazy load information need be provided, i.e.:

{ 
  provide: ANALYZE_FOR_LAZY_LOAD,
   useValue: [ 
    './child1/child1.module#Child1Module', 
    './child2/child2.module#Child2Module' 
  ]
}

The @angular/router would then provide the module's lazy load information by doing something like:

const lazyRoutes = ROUTES.map(x => x.loadChildren).filter(identity);
const provider = { provide: ANALYZE_FOR_LAZY_LOAD, useValue: lazyRoutes, multi: true };

The knowledge about the lazy load declaration object (i.e., what the loadChildren property means on a route) is moved from @angular/compiler-cli to the library that owns that structure (i.e., the @angular/router).

Of these three proposals, this one is my favorite as it separates concerns nicely and provides a clear mechanism for lib authors to use.

Activity

  1. filipesilva commented on Mar 9, 2017

    @filipesilva
    Contributor

    Heya @christopherthielen, thank you for making such a detailed issue about this topic.

    I haven't anything to contribute myself, but will loop the appropriate people in.

    @hansl @robwormald @IgorMinar what do you think of these proposals? Is there anything else planned that would address this?

  2. added
    P3An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
    featureLabel used to distinguish feature request from other issues
    on Mar 9, 2017
  3. fulls1z3 commented on Mar 20, 2017

    @fulls1z3

    It would be really useful, if @ngtools/webpack can discover routes provided with useFactory as well as it does with useValue:

    { provide: ROUTES, useFactory: (provideRoutesSomehow), deps: [CustomRouterService], multi: true },
    { provide: ANALYZE_FOR_ENTRY_COMPONENTS, useValue: routes, multi: true }
    
  4. hansl commented on Mar 27, 2017

    @hansl
    Contributor

    We cannot use this. End of story. There's absolutely no way we can know what provideRoutesSomehow does or what it returns while building the software (we would need to run it / halting problem), and we need static routing if we want to do code splitting in webpack. So even though your solution in angular/angular#15334 might do something, I'm very confident it does the wrong thing.

    I will discuss the issue itself with the Angular i18n team internally and figure out the best way to move forward.

  5. christopherthielen commented on Apr 24, 2017

    @christopherthielen
    Author

    This issue is still very important to me and ui-router users. With the new npm package structure in angular 4.x, ui-router users now have to bundle the entire @angular/router library simply to provide the ROUTES token (which enables ngtools/webpack support)

    I haven't seen any public discussion about this issue. Has there been any internal discussion @hansl @robwormald @IgorMinar and if so, are you amenable to any of the above three proposals (expecially number 3)?

  6. filipesilva commented on May 5, 2017

    @filipesilva
    Contributor

    #5981 has a similar issue:


    Bug Report or Feature Request (mark with an x)

    - [ bug report ]
    ERROR Error: Uncaught (in promise): Error: Cannot find module './app/example/template/test-template/test-template.module'.
    
    ### Versions.
    @angular/cli: 1.0.0
    node: 6.9.4
    os: darwin x64
    
    ### Repro steps.
    when load dynamic module, using SystemJsNgModuleLoader.load(this.path), in browser inspect, got this error: "ERROR Error: Uncaught (in promise): Error: Cannot find module "'./app/example/template/test-template/test-template.module'
    files path:
    -src/app/example/template/test-template/test-template.module.ts
    -src/app/example/example.component.ts
    
    In example.component.ts, I tried to use this.path as below but all cannot work :
    1. src/app/example/template/test-template/test-template.module
    2. app/example/template/test-template/test-template.module
    3. ./app/example/template/test-template/test-template.module
    4. ./template/test-template/test-template.module
    
    ### Mention any other details that might be useful.
    Does angular-cli allow us to load a module dynamically?
    
    Thanks.
    
    
  7. oparisy commented on May 5, 2017

    @oparisy

    I'm coming from #5981 since it was closed and merged with this issue.

    I understand the angular-cli design choice of treeshaking/statically analysing as much code as possible.

    But should that systematically forbid any way to load external modules (i.e. not available to angular-cli at build time)? Obviously we would not benefit from tree shaking for those modules.

  8. wardbell commented on May 6, 2017

    @wardbell
    Contributor

    @christopherthielen Sympathetic with your goal and proposal #3 seems on the right track although I haven't thought about it deeply.

    Perhaps unfairly, I want to hijack your efforts at solving this problem. Would you mind showing me how you used the Angular router infrastructure to lazy load one of your modules? I realize that you're goal is to be free of the Angular router dependence. For my purpose I don't require that freedom. I could get by with loading the RouterModule and hijacking its async module loading and the CLI's router-oriented code-splitting magic. And while I could figure this out myself, if you've already done it ... to steal is divine, yes? ;-)

  9. oparisy commented on May 8, 2017

    @oparisy

    I have just stumbled upon the possibility to define externals in webpack configuration, which are a way to define a set of libraries which are not bundled but loaded at runtime (using CommonJS as an example).

    This would definitely do the trick for my use case since regexp patterns can be used and I could definitely enforce some naming scheme for my "dynamic" modules. It seems I'd have to use another loader than SystemJS, but that's a fair price to pay.

    Now, I understand arbitrary modification of the webpack configuration in angular-cli is a big no-no (#1656).

    So is this externals mechanism exposed by angular-cli? How can I configure it?

  10. christopherthielen commented on May 9, 2017

    @christopherthielen
    Author

    @wardbell there are a few concerns to consider:

    1. lazy loading of an NgModule (supporting AoT and JIT)
    2. placement of lazy loaded NgModule injector in the injector tree
    3. code splitting by webpack + @ngtools/webpack
    4. lazy module discovery and compilation by ngc

    For module lazy loading, you need to:
    1a) lazy load the sources (System.import('./lazymodule'))
    1b) For JIT, get the NgModule and compile it
    1c) For AoT, get the NgModuleFactory

    Instantiate the lazy module as an NgModuleRef.
    2a) Pass the parent injector when creating (ngModuleFactory.create(parentInjector))
    Remember, we have a tree of injectors. When created, the NgModule will have a child injector associated with it.
    2b) Now you have to use the child injector from the NgModule to inject services defined by the lazy module.
    2c) You also have to use the child injector to access the lazy module's ComponentFactory when creating components from the lazy NgModule.

    It's been a while since I wrote this code, but this is my vague recollection:
    Both @ngtools/webpack code splitting and also ngc lazy module analysis and compilation is done in the same way.
    3a) Provide an array of objects using the ROUTES token (from @angular/router)
    3b) Make sure each object in the array has a loadChildren function (if it should lazy load)
    3c) In the loadChildren function, return a promise for the NgModule or NgModuleFactory, (or a string when using @ngtools/webpack loader's magic encantation)

    This allows ngc to statically analyze the dependency tree, finding child modules via loadChildren and then statically analyzing the child module recursively.

  11. filipesilva commented on Aug 1, 2017

    @filipesilva
    Contributor

    Just wanted to mention that there is some further conversation and workarounds in this thread: angular/angular#18093.

  12. Dunos commented on Oct 20, 2017

    @Dunos

    Any progress on this?

  13. filipesilva commented on Nov 15, 2017

    @filipesilva
    Contributor

    Also wanted to mention @gkalpak's work in angular/angular#18428 for lazy loaded embedded components.

  14. hansl commented on Jan 23, 2018

    @hansl
    Contributor

    Moving the discussion here to #9343 where we talk about the implementation.

  15. angular-automatic-lock-bot commented on Sep 9, 2019

    @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.

  16. locked and limited conversation to collaborators on Sep 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

P3An issue that is relevant to core functions, but does not impede progress. Important, but not urgentfeatureLabel used to distinguish feature request from other issuestype: faq

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions