Move NgModuleFactoryLoader from core to router

[dawidgarus]

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Performance issue
[x] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question
[ ] Other... Please describe:

Current behavior

NgModuleFactoryLoader and SystemJsNgModuleLoader are defined in @angular/core

Expected behavior

They should be defined in @angular/router or other module.

Minimal reproduction of the problem with instructions

N/A

What is the motivation / use case for changing the behavior?

1. NgModuleFactoryLoader is not used anywhere in core or in any module other than router
2. NgModuleFactoryLoader is provided in router
3. This change would probably allow us to use DllPlugin with @ngtools/webpack. See this issue: @ngtools/webpack :: Lazy Loaded Routes Not Built In Build With DLL Bundles angular-cli#4565.
   Currently @angular/core cannot be compiled separately from main app bundle as dll, because lazy loading chunks are not generated then. And because everything depends on @angular/core, all angular packages cannot be in dll. If SystemJsNgModuleLoader was in @angular/router, all other angular packages could be included in dll.

Environment

Angular version: 6.0.0

[mlc-mlapis]

@dawidgarus ... we use NgModuleFactoryLoader and SystemJsNgModuleLoader regardless of the Router ... for programmatic loading of lazy loaded modules. For us it is just a core functionality.

[dawidgarus]

@mlc-mlapis May I ask where? I didn't find any reference to NgModuleFactoryLoader or SystemJsNgModuleLoader in animations, common, compiler, compiler-cli, core (only definition, not used), forms, platform-browser, platform-browser-dynamic, upgrade.
Anyway, maybe consider moving it to @angular/core/factory-loader (or other name) then? It would still allow us to use DllPlugin for all the module that don't depend on it, which are all except router.

[mlc-mlapis]

@dawidgarus ... ah, my comment was that we use them in our own code - we import them from core right now. I tried to say that the code is not specific just only for Angular Router module.

And @angular/core/factory-loader would be fine. I am just not sure if moving to router module and importing from there instead of core would also mean increasing size of bundles that don't use Router ... and how much.

[jasonaden]

Thank you for the issue. However, these are consumed by the router, but can also be consumed by applications, and are a piece of core functionality. Therefore they should stay in @angular/core.

[mlc-mlapis]

@dawidgarus ... I read your arguments why you have a problem with lazy loading chunks are not generated then but I still can't fully understand the reason how placing NgModuleFactoryLoader and SystemJsNgModuleLoader to Router module can help you. Especially when routes for lazy loaded modules are based on the principle of text path syntax.

[dawidgarus]

@jasonaden If it's a core functionality, why do You have to import RouterModule to use it? To me, core functionality is something that is required for framework to work and it's not. It's just extra feature. Move it to separate package then, maybe '@angular/core/factory-loader` or something.

@mlc-mlapis in angular-cli issue referenced it's explained why. TL;DR version is that module which contains SystemJsNgModuleLoader cannot be prebuild when using webpack - it must be build together with app. That's why You cannot use DllPlugin or umd bundles that angular provides.

[mlc-mlapis]

@dawidgarus ... so 2 questions:

• what are those LOC where RouterModule is imported to use core functionality?
• what makes SystemJsNgModuleLoader so special that it can't be prebuild using Webpack? Is it the relation to SystemJS declare var System: any; ?

  angular/packages/core/src/linker/system_js_ng_module_factory_loader.ts

  Line 19 in c8a1a14

  declare var System: any;

[dawidgarus]

@mlc-mlapis AngularWebpackPlugin somehow change module dependencies during build to include lazy loaded modules. Obviously it can do so only if it's build together with app. What's so special about SystemJsNgModuleLoader it's that it creates webpack's ContextModuleFactory.

[mlc-mlapis]

@dawidgarus ... hmm, thanks for additional info. We don't use Webpack so that's why the questions.

[trotyl]

@jasonaden NgModuleFactoryLoader is indeed part of @angular/core, but SystemJsNgModuleLoader is a platform-specific implementation and should be in @angular/platform-browser.

Anything being platform-independent would only consume abstract NgModuleFactoryLoader token rather than SystemJsNgModuleLoader provider, and user can always provide a different implementation.