fix(ivy): expose module injector ivy's TestBed impl by pkozlowski-opensource · Pull Request #25685 · angular/angular

pkozlowski-opensource · 2018-08-27T14:09:09Z

This PR changes the ivy's TestBed implementation - previously no injector was available for the root node (one created to contain test component). Now such node gets module injector.

This change unblocks some of the tests for ngTemplateOutlet.

mary-poppins · 2018-08-27T14:15:56Z

You can preview f5d6b68 at https://pr25685-f5d6b68.ngbuilds.io/.

mary-poppins · 2018-08-27T14:37:02Z

You can preview 90f9ec0 at https://pr25685-90f9ec0.ngbuilds.io/.

pkozlowski-opensource · 2018-08-27T15:43:30Z

+  constructor(private moduleInjector: Injector, private viewData: LViewData|null) {
+    // viewData can be null for the root element created to contain test component (such element is
+    // created before ivy kicks in and creates root view).
+    this.nodeIndex = viewData ? viewData.length - 1 : null;


This is WRONG as reading length of the viewData does NOT get us index of where the corresponding LNode will be stored. Needs design discussion with @mhevery

Would this help:

angular/packages/core/src/render3/element_discovery.ts

Lines 39 to 66 in 29761ea

export function getElementContext(element: RElement): ElementContext|null {

let context = (element as any)[MONKEY_PATCH_KEY_NAME] as ElementContext | LViewData | null;

if (context) {

if (Array.isArray(context)) {

const lViewData = context as LViewData;

const index = findMatchingElement(element, lViewData);

context = {index, native: element, lViewData};

attachLViewDataToNode(element, context);

}

} else {

let parent = element as any;

while (parent = parent.parentNode) {

const parentContext =

(parent as any)[MONKEY_PATCH_KEY_NAME] as ElementContext | LViewData | null;

if (parentContext) {

const lViewData =

Array.isArray(parentContext) ? (parentContext as LViewData) : parentContext.lViewData;

const index = findMatchingElement(element, lViewData);

if (index >= 0) {

context = {index, native: element, lViewData};

attachLViewDataToNode(element, context);

break;

}

}

}

}

return (context as ElementContext) || null;

}

mhevery · 2018-08-28T22:59:26Z

-      const wrapper = ngModule.injector.get(WRAP_RENDERER_FACTORY2, (v: RendererFactory2) => v);
-      rendererFactory = wrapper(ngModule.injector.get(RendererFactory2)) as RendererFactory3;
+      const wrapper =
+          ngModule.injector.get(WRAP_RENDERER_FACTORY2, (v: RendererFactory2, i: Injector) => v);


My current thinking is that WRAP_RENDERER_FACTORY2 should be reverted since with element_discovery we don't need it.

What we should do is to change

angular/packages/core/src/debug/debug_node.ts

Lines 167 to 169 in 29761ea

export function getDebugNode(nativeNode: any): DebugNode|null {

return _nativeNodeToDebugNode.get(nativeNode) || null;

}

to go through ng_ivy_switch.ts so that in case of Ivy it returns DebugNode implementation which knows how to use

angular/packages/core/src/render3/element_discovery.ts

Lines 39 to 66 in 29761ea

export function getElementContext(element: RElement): ElementContext|null {

let context = (element as any)[MONKEY_PATCH_KEY_NAME] as ElementContext | LViewData | null;

if (context) {

if (Array.isArray(context)) {

const lViewData = context as LViewData;

const index = findMatchingElement(element, lViewData);

context = {index, native: element, lViewData};

attachLViewDataToNode(element, context);

}

} else {

let parent = element as any;

while (parent = parent.parentNode) {

const parentContext =

(parent as any)[MONKEY_PATCH_KEY_NAME] as ElementContext | LViewData | null;

if (parentContext) {

const lViewData =

Array.isArray(parentContext) ? (parentContext as LViewData) : parentContext.lViewData;

const index = findMatchingElement(element, lViewData);

if (index >= 0) {

context = {index, native: element, lViewData};

attachLViewDataToNode(element, context);

break;

}

}

}

}

return (context as ElementContext) || null;

}

@mhevery I fully agree with the general idea of using element discovery to "enrich" native node with debug context for TestBed.

In the details, though, I don't think we can simply plug into

angular/packages/core/src/debug/debug_node.ts

Lines 167 to 169 in 29761ea

export function getDebugNode(nativeNode: any): DebugNode|null {

return _nativeNodeToDebugNode.get(nativeNode) || null;

}

since:

at this point we don't know the type of native node (element / text / comment) and we need to know this in order to instantiate DebugNode or DebugElement

we don't have enough context to constructs any of those classes (ex. listeners, properties etc.)

My thinking is that we will have to call indexDebugNode from ivy instructions - unless we want to do deeper breaking changes to the TestBed.

The challenge to call indexDebugNode would be to do this in the tree-shakable way (do so only in the debug mode?).

Let's discuss more today.

pkozlowski-opensource · 2018-10-02T14:15:42Z

Closing in favour of #26211

angular-automatic-lock-bot · 2019-09-14T02:23:23Z

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 Aug 27, 2018

pkozlowski-opensource added comp: ivy action: review The PR is still awaiting reviews from at least one requested reviewer target: major This PR is targeted for the next major release labels Aug 27, 2018

pkozlowski-opensource force-pushed the ivy_test_bed_module_injector branch from f5d6b68 to 90f9ec0 Compare August 27, 2018 14:24

fix(ivy): expose module injector ivy's TestBed impl

90f9ec0

pkozlowski-opensource commented Aug 27, 2018

View reviewed changes

pkozlowski-opensource added the state: WIP label Aug 27, 2018

mhevery reviewed Aug 28, 2018

View reviewed changes

pkozlowski-opensource closed this Oct 2, 2018

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

Navigation Menu

Repository navigation

fix(ivy): expose module injector ivy's TestBed impl - #25685

fix(ivy): expose module injector ivy's TestBed impl#25685
pkozlowski-opensource wants to merge 1 commit into
angular:masterfrom
pkozlowski-opensource:ivy_test_bed_module_injector

pkozlowski-opensource commented Aug 27, 2018

Uh oh!

mary-poppins commented Aug 27, 2018

Uh oh!

mary-poppins commented Aug 27, 2018

Uh oh!

pkozlowski-opensource Aug 27, 2018

Uh oh!

mhevery Aug 28, 2018

Uh oh!

mhevery Aug 28, 2018

Uh oh!

pkozlowski-opensource Aug 29, 2018

Uh oh!

pkozlowski-opensource commented Oct 2, 2018

Uh oh!

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

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	export function getElementContext(element: RElement): ElementContext\|null {
	let context = (element as any)[MONKEY_PATCH_KEY_NAME] as ElementContext \| LViewData \| null;
	if (context) {
	if (Array.isArray(context)) {
	const lViewData = context as LViewData;
	const index = findMatchingElement(element, lViewData);
	context = {index, native: element, lViewData};
	attachLViewDataToNode(element, context);
	}
	} else {
	let parent = element as any;
	while (parent = parent.parentNode) {
	const parentContext =
	(parent as any)[MONKEY_PATCH_KEY_NAME] as ElementContext \| LViewData \| null;
	if (parentContext) {
	const lViewData =
	Array.isArray(parentContext) ? (parentContext as LViewData) : parentContext.lViewData;
	const index = findMatchingElement(element, lViewData);
	if (index >= 0) {
	context = {index, native: element, lViewData};
	attachLViewDataToNode(element, context);
	break;
	}
	}
	}
	}
	return (context as ElementContext) \|\| null;
	}

	export function getDebugNode(nativeNode: any): DebugNode\|null {
	return _nativeNodeToDebugNode.get(nativeNode) \|\| null;
	}

Conversation

pkozlowski-opensource commented Aug 27, 2018

Uh oh!

mary-poppins commented Aug 27, 2018

Uh oh!

mary-poppins commented Aug 27, 2018

Uh oh!

pkozlowski-opensource Aug 27, 2018

Choose a reason for hiding this comment

Uh oh!

mhevery Aug 28, 2018

Choose a reason for hiding this comment

Uh oh!

mhevery Aug 28, 2018

Choose a reason for hiding this comment

Uh oh!

pkozlowski-opensource Aug 29, 2018

Choose a reason for hiding this comment

Uh oh!

pkozlowski-opensource commented Oct 2, 2018

Uh oh!

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

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants