refactor(router): avoid using deprecated Compiler symbol in Router
#44194
Conversation
AndrewKushnir
added
state: WIP
refactoring
| } else { | ||
| return from(this.compiler.compileModuleAsync(t)); | ||
| } | ||
| return isNgModuleFactory(t) ? of(t) : from(Promise.resolve(t)); |
There was a problem hiding this comment.
Could we omit the Promise.resolve or is the additional microtick load-bearing, e.g. for tests? I don't think this code path is necessarily async now that we don't need to call into the async compiler anymore.
There was a problem hiding this comment.
I tried to avoid any diff from the execution order perspective. Still, there are few CI failures that I'll need to investigate more and I will take this comment into account as well and will factor the code more if this is not load-bearing (as you mentioned).
packages/router/src/router.ts
Outdated
| @@ -595,7 +596,7 @@ export class Router { | |||
| this.rawUrlTree = this.currentUrlTree; | |||
| this.browserUrlTree = this.currentUrlTree; | |||
|
|
|||
| this.configLoader = new RouterConfigLoader(injector, compiler, onLoadStart, onLoadEnd); | |||
| this.configLoader = new RouterConfigLoader(onLoadStart, onLoadEnd); | |||
There was a problem hiding this comment.
Heads up that the injector constructor parameter is depended upon by a g3 patch, so you may want to keep it in the source code to keep the impact of the patch small.
There was a problem hiding this comment.
Thanks for the comment! Yeah, @atscott also mentioned that (offline), I will bring it back and add some comments in the code (it wasn't clear to me why we have an unused argument :)).
AndrewKushnir
force-pushed
the
router_factory_less
branch
from
38547ec to
84ca1af
Compare
AndrewKushnir
force-pushed
the
router_factory_less
branch
from
84ca1af to
f845892
Compare
AndrewKushnir
force-pushed
the
router_factory_less
branch
from
f845892 to
ff4d67d
Compare
AndrewKushnir
force-pushed
the
router_factory_less
branch
from
ff4d67d to
d24cb57
Compare
The `Compiler` symbol was deprecated in v13 as a part of the factory-less cleanup in public APIs. This commit updates the logic inside of the Router package to avoid using it in favor of replacement API (the `createNgModuleRef` function).
AndrewKushnir
force-pushed
the
router_factory_less
branch
from
d24cb57 to
e85e810
Compare
| constructor( | ||
| private rootComponentType: Type<any>|null, private urlSerializer: UrlSerializer, | ||
| private rootContexts: ChildrenOutletContexts, private location: Location, injector: Injector, | ||
| // Note: the `compiler` argument is unused, but dropping it would be a breaking change. | ||
| // TODO: remove the `compiler` argument in the next major release (v14). |
There was a problem hiding this comment.
Is this going to be merged before v14?
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
The
Compilersymbol was deprecated in v13 as a part of the factory-less cleanup in public APIs. This commit updates the logic inside of the Router package to avoid using it in favor of replacement API (thecreateNgModuleReffunction).PR Type
What kind of change does this PR introduce?
Does this PR introduce a breaking change?