fix(platform-browser-dynamic): provide a subclass of Compiler

[JLHwung]

Providing the Compiler will interfere with the offlineMode check in SystemJsNgModuleLoader.load. Instead we should provide a subclass of Compiler so that

• the SystemJsNgModuleLoader will, as expected, find correct factory on AOT builds
• we can still use JitCompiler on lazy modules

Related issue: #20639 (comment)

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Given Compiler provided in root module and building with --aot, the loadChildren route declaration will fail to find the correct module path.

The culprit is the interference with offlineMode check in SystemJsNgModuleLoader. (code is here) Since the offlineMode is always false, the AOT compiled factories are never loaded, and the module loader loads the incorrect path on --aot builds.

Issue Number: #20639 (comment)

What is the new behavior?

The loadChildren works as expected on --aot builds in case the user properly provides a subclass of Compiler.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

I am not quite sure if it is a bug fix or a documentation change since the issue can be solved in user land only.

Shall we add loadChildren to the dynamic-compiler example?

[mlc-mlapis]

@JLHwung ... the line const offlineMode = this._compiler instanceof Compiler; decides if any.module.js or any.module.ngfactory.js is loaded on a route {path: 'any', loadChildren: 'any.module#AnyModule'}, on the fact if the compiler instance is present or not. It is done automatically. How would you differentiate between loading any.module.js or any.module.ngfactory.js ... are you adding some new syntax to be able to load AOT and JIT modules in different places?

Btw ... what would be the sense of loading a non AOT lazy loaded module when the potential key of using of JitCompiler in AOT app is mainly to take any component template HTML code and compile it in run-time in a browser?

[JLHwung]

@mlc-mlapis In most cases the Compiler is provided in the bootstrap process (code is here) and AOT will replace the bootstrap process (code is here), removing the Compiler provider so that the the offlineMode check works as expected by checking if the Compiler has been provided.

However, in the situation demonstrated by the dynamic-compiler example, the user always provide the Compiler so the offlineMode check does not work as expected on --aot builds. As this provider is used to load lazy dynamic module, I revise the example to always providing a subclass of Compiler so that the offlineMode check still works and we can still use the subclassed Compiler to load lazy modules.

Are you adding some new syntax to be able to load AOT and JIT modules in different places?

No, I am revising the integration example to make sure the offlineMode still works.

What would be the sense of loading a non AOT lazy loaded module when the potential key of using of JitCompiler in AOT app is mainly to take any component template HTML code and compile it in run-time in a browser?

I guess you mean the sense of loading AOT modules while keeping a JitCompiler reference in app. Correct me if wrong.

My company uses angular on enterprise software with a plugin design. By plugin I mean the dynamic modules developed by third-party vendors. The host site is compiled without any knowledge of plugin, so we need a JitCompiler to compile these modules on-the-fly.

On the other hands, the system has many builtin business modules, so loadChildren is used on code splitting for first loading time performance. The builtin modules is built with --aot for better performance.

[mlc-mlapis]

@JLHwung ... so your case is:

• compile the app core as AOT app and load it,
• the next loading of any lazy loaded module loads their JIT version,
• it's not possible to load AOT lazy loaded module because all of them are loaded in JIT mode.

So that's why I was talking about a new syntax because I supposed still the ability to load either AOT or JIT lazy loaded modules in any time from the core AOT app and it was unimaginable that it wouldn't be possible.

Because of your case and loading of plugin third parties JIT modules which have to be independent on your build core process ... wouldn't better to use UMD Angular packages (loaded on demand) and have the possibility to AOT compile even those third parties lazy modules? Because then you are able to load them in any time later and the only condition is that you and all third parties module authors have to use just the same Angular version. And this preposition shouldn't be a real complication for you because even today you can't mix Angular 2, 4, 5, 6 because of using some fixed Angular compiler version.

[JLHwung]

@mlc-mlapis

wouldn't better to use UMD Angular packages (loaded on demand) and have the possibility to AOT compile even those third parties lazy modules?

The APF v6 does not meet our requirements. AFAIK the package need to compile with the main app so that the dependencies, i.e. @angular/* can be satisfied. Some may argue that dependencies can be also loaded via System.js from unpkg.com. However it does not work since the app is mostly living in intranet. Instead we use DLL bundles to bundle @angular/* and have the third party module compile against the dll libraries. In this way there should be only one angular instance in the app, and in all, any updates of third party module would not require the recompilation of main app. That is why we do not use APF.

a new syntax ... to load either AOT or JIT lazy loaded modules in any time from the core AOT app

This is a cool idea. The main barrier would be how to leak these instructions to webpack compiler. The @ngtools/webpack has a done a great effort on handling loadChildren. tmeneau has a detailed explanation here.

If you are interested at more granularity on module lazy loading, which is a great topic for sure, I suggest we discuss at this thread so this PR would not detour from the topic. 😊

/cc @ocombe for reviewing the revised example. Thanks.

[mlc-mlapis]

@JLHwung

... dependencies can be also loaded via System.js from unpkg.com. However it does not work since the app is mostly living in intranet.

You can load them from any server you want so why should it be a problem on intranet? It would be the same server as you are using to serve your app.

... the package need to compile with the main app so that the dependencies, i.e. @angular/* can be satisfied.

Not necessarily. Especially when you use UMD Angular native packages because you have ALL Angular code ready in a browser so the lazy loaded module can be compiled independently ... using ngc compiler.

[ocombe]

I don't think we are going to introduce this kind of changes when we are already working on ivy which will change all of that, what do you think @vicb ?

[JLHwung]

@mlc-mlapis Thanks very much for the replies.

You can load them from any server you want so why should it be a problem on intranet? It would be the same server as you are using to serve your app.

You are right. The key point is actually on bundling. I just realize I mixed compiling and bundling in my previous comments. On the main site, the @angular/* has been bundled by webpack, if we load dynamic modules via System.js or bundler other than webpack, there will be two angular instances, rendering many issues definitely. Actually that is why the dynamic-compiler example use rollup to bundle both main site and lazy modules. This could be solved by whitelisting dependencies in ng-packagr.

Especially when you use UMD Angular native packages because you have ALL Angular code ready in a browser so the lazy loaded module can be compiled independently.

I agree that the UMD packages would work given that every dependencies of dynamic modules have a UMD bundles. However the main site should still emit some stats to tell ng-packagr that which dependency should be whitelisted as the dynamic module can use any npm modules. These modules should be bundled if it is not bundled in main site.

@ocombe The dynamic-compiler is a living mini example application. The current example will render loadChildren unuseable in --aot builds (see the related issue), this is not intuitive and not expected. Although the ivy is in the horizon, I think it also make sense to tweak previous examples and solve the known issues in the example.

[mlc-mlapis]

@JLHwung

However the main site should still emit some stats to tell ng-packagr that which dependency should be whitelisted as the dynamic module can use any npm modules. These modules should be bundled if it is not bundled in main site.

I meant it really as ... it's not necessary to bundle anything from Angular native packages to lazy or dynamic modules ... it is main point ... that any third party developers don't know anything about others and still are able to develop their part ... because they don't worry what to bundle or don't bundle into their module. Their module contains only their code and nothing from Angular native packages because all of that is guaranteed by loaded UMD Angular packages.

[JLHwung]

Closed since ivy will change all of that. The technique show in this MR should be viewed more of a workaround instead of a must.

Great thanks to @mlc-mlapis on inspiring discussion.

[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.}