-
Notifications
You must be signed in to change notification settings - Fork 25.5k
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
fix(upgrade): component injectors should not link the module injector… #15385
Conversation
return notFoundValue; | ||
} | ||
|
||
return this.injector.get(token); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pass notFoundValue
here too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, thanks !
7cd099c
to
cb11def
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (just a couple of nits)
|
||
let componentInjector: Injector; | ||
|
||
@Component({selector: 'ng2', template: '[<ng-content></ng-content>]'}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: No need for <ng-content>
(afaict). You can just have 'ng2'
(or an empty template if you don't care about it).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, I tried many things before coming to this test... will remove
} | ||
|
||
const ng1Module = angular.module('ng1', []) | ||
.directive('ng1', () => ({template: '<ng2></ng2>'})) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: No need for this directive. You can use const element = html('<ng2></ng2>')
below instead.
|
||
const element = html('<ng1></ng1>'); | ||
|
||
platformBrowserDynamic().bootstrapModule(Ng2Module).then((modRef) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: You could probably use the bootstrap helper (and save two whole lines of code 😛):
bootstrap(platformBrowserDynamic(), Ng2Module, element, ng1Module).then(upgrade => {
const modInjector = upgrade.injector;
// Emulate the router lazy loading ...
...
});
constructor(private modInjector: Injector) {} | ||
|
||
// When Angular locate a service in the component injector tree, the not found value is set to | ||
// `NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR` in such a case we should not walk up to the module |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a .
before in such case
const compiler = modInjector.get(Compiler); | ||
const modFactory = compiler.compileModuleSync(LazyLoadedModule); | ||
const childMod = modFactory.create(modInjector); | ||
const cmpFactory = childMod.injector.get(ComponentFactoryResolver) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
childMod.componentFactoryResolver
@@ -361,5 +361,55 @@ export function main() { | |||
expect(multiTrim(document.body.textContent)).toBe('parent(child)'); | |||
}); | |||
})); | |||
|
|||
it('should work with ng2 lazy loaded components=', async(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: components= --> components
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. |
No description provided.