fix: element injector vs module injector by vicb · Pull Request #15044 · angular/angular

vicb · 2017-03-09T19:09:07Z

TODO:

add refs to issues in commit message,
describe API change in commit message
link design doc
test for ViewContainerRef.parentInjector

vicb · 2017-03-09T22:20:39Z

parentModule

vicb · 2017-03-09T22:35:39Z

add tests

override provider in a child module

vicb · 2017-03-09T22:43:48Z

test:

create a child module and bootstrap a cmp from it, should see child providers.

only pass a module if component factory has no

vicb · 2017-03-09T22:45:47Z

ngModule should be public on ComponentFactory

not for now

vicb · 2017-03-09T22:56:06Z

throw if no ngModule

vicb · 2017-03-09T22:56:36Z

vicb · 2017-03-09T22:58:21Z

revert
add test: inject ViewContainerRef on a cmp TestBed.create(Comp).get()

vicb · 2017-03-09T22:59:33Z

TestBed.get() -> injector

vicb · 2017-03-09T23:01:18Z

check if we can pass the module instead

vicb · 2017-03-09T23:08:30Z

injector -> ModuleRef only

no -> API break

vicb · 2017-03-09T23:15:51Z

add simpler method & deprecate

outlet.activate(future, resolver, outletMap);

vicb · 2017-03-09T23:17:08Z

inject NgModuleRef

no, API break

vicb · 2017-03-09T23:20:36Z

add module can no longer shadow el

tbosch · 2017-03-10T17:56:25Z

Maybe Illegal sate: NgModule has to be provided?

(as its an error that users never should run into)...

tbosch

Reviewed in person...

thaoula · 2017-03-11T02:44:52Z

hi vicb,

Thanks for your hard work.

Do you have any idea when this will be merged into V4?

Regards,
Tarek

IgorMinar

lgtm for api changes

vicb · 2017-03-13T17:41:38Z

@thaoula should be part of the next RC release

fixes angular#12869 fixes angular#12889 fixes angular#13885 fixes angular#13870 Before this change there was a single injector tree. Now we have 2 injector trees, one for the modules and one for the components. This fixes lazy loading modules. See the design docs for details: https://docs.google.com/document/d/1OEUIwc-s69l1o97K0wBd_-Lth5BBxir1KuCRWklTlI4 BREAKING CHANGES `ComponentFactory.create()` takes an extra optional `NgModuleRef` parameter. No change should be required in user code as the correct module will be used when none is provided DEPRECATIONS The following methods were used internally and are no more required: - `RouterOutlet.locationFactoryResolver` - `RouterOutlet.locationInjector`

thaoula · 2017-03-14T05:45:44Z

Hi Vicb,

Awesome!!!. Do you know when that will be? Been waiting for a fix to this issue since November.

Regards,
Tarek

al-pt-old · 2017-03-17T14:01:22Z

-      rootSelectorOrNode: string|any = null): ComponentRef<any> {
+      injector: Injector, projectableNodes?: any[][], rootSelectorOrNode?: string|any,
+      ngModule?: NgModuleRef<any>): ComponentRef<any> {
+    if (!ngModule) {


hi!

ngModule is defined as an optional variable, but here we see throw instruction.

Why?

After upgrading to 4.0.rc-4 we got an Exception below

ngModule should be provided

export declare abstract class ViewContainerRef { abstract createComponent<C>(componentFactory: ComponentFactory<C>,

This class implements the ComponentFactory<any> which requires this parameter be optional (https://github.com/angular/angular/blob/4.0.0-rc.4/packages/core/src/linker/component_factory.ts#L76-L78). However, this is an internal implementation that should never be called without a module so we check for internal consistency.

caused by: angular/angular#15044

fixes angular#12869 fixes angular#12889 fixes angular#13885 fixes angular#13870 Before this change there was a single injector tree. Now we have 2 injector trees, one for the modules and one for the components. This fixes lazy loading modules. See the design docs for details: https://docs.google.com/document/d/1OEUIwc-s69l1o97K0wBd_-Lth5BBxir1KuCRWklTlI4 BREAKING CHANGES `ComponentFactory.create()` takes an extra optional `NgModuleRef` parameter. No change should be required in user code as the correct module will be used when none is provided DEPRECATIONS The following methods were used internally and are no more required: - `RouterOutlet.locationFactoryResolver` - `RouterOutlet.locationInjector`

angular-automatic-lock-bot · 2019-09-13T09:51:12Z

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

googlebot added the cla: yes label Mar 9, 2017

vicb force-pushed the 0306-testinj branch 4 times, most recently from ec67a6a to 9358917 Compare March 9, 2017 22:11

vicb commented Mar 9, 2017

View reviewed changes

Comment thread modules/@angular/core/src/view/refs.ts Outdated

vicb Mar 9, 2017

Copy link
Copy Markdown

Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

throw if no ngModule

vicb commented Mar 9, 2017

View reviewed changes

Comment thread modules/@angular/router/src/apply_redirects.ts Outdated

vicb Mar 9, 2017

Copy link
Copy Markdown

Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check if we can pass the module instead

vicb commented Mar 9, 2017

View reviewed changes

vicb mentioned this pull request Mar 10, 2017

WIP #14895

Closed

vicb force-pushed the 0306-testinj branch from 6cc9a3f to e85f0b1 Compare March 10, 2017 02:07

tbosch reviewed Mar 10, 2017

View reviewed changes

vicb force-pushed the 0306-testinj branch from e85f0b1 to 92b09ff Compare March 10, 2017 18:46

vicb requested a review from tbosch March 10, 2017 18:48

tbosch approved these changes Mar 10, 2017

View reviewed changes

vicb force-pushed the 0306-testinj branch from 92b09ff to 0a134fe Compare March 10, 2017 19:16

vicb added the action: merge The PR is ready for merge by the caretaker label Mar 10, 2017

vicb force-pushed the 0306-testinj branch from 0a134fe to 9273051 Compare March 10, 2017 19:55

IgorMinar approved these changes Mar 13, 2017

View reviewed changes

vicb force-pushed the 0306-testinj branch from 9273051 to 607a88a Compare March 13, 2017 17:45

chuckjaz added the pr_action: queued label Mar 13, 2017

chuckjaz merged commit 13686bb into angular:master Mar 14, 2017

al-pt-old reviewed Mar 17, 2017

View reviewed changes

sis0k0 added a commit to NativeScript/nativescript-angular that referenced this pull request Mar 20, 2017

fix(page-router-outlet): activateWith instead of activate method

b5f3f4d

caused by: angular/angular#15044

sis0k0 added a commit to NativeScript/nativescript-angular that referenced this pull request Mar 20, 2017

fix(page-router-outlet): activateWith instead of activate method

8d832bc

caused by: angular/angular#15044

colavitam mentioned this pull request Jul 27, 2017

NgComponentOutlet should allow module injectors in addition to component injectors #18387

Closed

vicb deleted the 0306-testinj branch March 6, 2018 22:23

angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 13, 2019

Conversation

vicb commented Mar 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vicb Mar 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vicb Mar 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tbosch left a comment

Choose a reason for hiding this comment

Uh oh!

thaoula commented Mar 11, 2017

Uh oh!

IgorMinar left a comment

Choose a reason for hiding this comment

Uh oh!

vicb commented Mar 13, 2017

Uh oh!

thaoula commented Mar 14, 2017

Uh oh!

al-pt-old Mar 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

angular-automatic-lock-bot Bot commented Sep 13, 2019

Uh oh!

Reviewers

Assignees

vicb commented Mar 9, 2017 •

edited

Loading

vicb Mar 9, 2017 •

edited

Loading

vicb Mar 9, 2017 •

edited

Loading

al-pt-old Mar 17, 2017 •

edited

Loading