Skip to content

Commit

Permalink
fix(ivy): avoid DOM element assertions if procedural renderer is used (
Browse files Browse the repository at this point in the history
…angular#33156)

Prior to this commit, Ivy runtime asserted that a given element is an instance of a DOM node. These asserts may not be correct in case custom renderer is used, which operates objects with a shape different than DOM nodes. This commit updates the code to avoid the mentioned checks in case procedural renderer is used.

PR Close angular#33156
  • Loading branch information
AndrewKushnir authored and mhevery committed Oct 15, 2019
1 parent ec6a9f2 commit 11e04b1
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 6 deletions.
2 changes: 1 addition & 1 deletion packages/core/src/render3/instructions/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1463,8 +1463,8 @@ const LContainerArray: any = ((typeof ngDevMode === 'undefined' || ngDevMode) &&
export function createLContainer(
hostNative: RElement | RComment | LView, currentView: LView, native: RComment, tNode: TNode,
isForViewContainerRef?: boolean): LContainer {
ngDevMode && assertDomNode(native);
ngDevMode && assertLView(currentView);
ngDevMode && !isProceduralRenderer(currentView[RENDERER]) && assertDomNode(native);
// https://jsperf.com/array-literal-vs-new-array-really
const lContainer: LContainer = new (ngDevMode ? LContainerArray : Array)(
hostNative, // host native
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/render3/node_manipulation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ function applyToElementOrContainer(
lNodeToHandle = lNodeToHandle[HOST] !;
}
const rNode: RNode = unwrapRNode(lNodeToHandle);
ngDevMode && assertDomNode(rNode);
ngDevMode && !isProceduralRenderer(renderer) && assertDomNode(rNode);

if (action === WalkTNodeTreeAction.Create && parent !== null) {
if (beforeNode == null) {
Expand Down
8 changes: 4 additions & 4 deletions packages/core/src/render3/util/view_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import {assertTNodeForLView} from '../assert';
import {LContainer, TYPE} from '../interfaces/container';
import {LContext, MONKEY_PATCH_KEY_NAME} from '../interfaces/context';
import {TNode} from '../interfaces/node';
import {RNode} from '../interfaces/renderer';
import {RNode, isProceduralRenderer} from '../interfaces/renderer';
import {isLContainer, isLView} from '../interfaces/type_checks';
import {FLAGS, HEADER_OFFSET, HOST, LView, LViewFlags, PARENT, PREORDER_HOOK_FLAGS, TData, TVIEW} from '../interfaces/view';
import {FLAGS, HEADER_OFFSET, HOST, LView, LViewFlags, PARENT, PREORDER_HOOK_FLAGS, RENDERER, TData, TVIEW} from '../interfaces/view';



Expand Down Expand Up @@ -93,7 +93,7 @@ export function getNativeByTNode(tNode: TNode, lView: LView): RNode {
ngDevMode && assertTNodeForLView(tNode, lView);
ngDevMode && assertDataInRange(lView, tNode.index);
const node: RNode = unwrapRNode(lView[tNode.index]);
ngDevMode && assertDomNode(node);
ngDevMode && !isProceduralRenderer(lView[RENDERER]) && assertDomNode(node);
return node;
}

Expand All @@ -110,7 +110,7 @@ export function getNativeByTNodeOrNull(tNode: TNode, lView: LView): RNode|null {
if (index !== -1) {
ngDevMode && assertTNodeForLView(tNode, lView);
const node: RNode|null = unwrapRNode(lView[index]);
ngDevMode && node !== null && assertDomNode(node);
ngDevMode && node !== null && !isProceduralRenderer(lView[RENDERER]) && assertDomNode(node);
return node;
}
return null;
Expand Down
46 changes: 46 additions & 0 deletions packages/core/test/acceptance/renderer_factory_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,3 +198,49 @@ function getAnimationRendererFactory2(document: any): RendererFactory2 {
document.body, new MockAnimationDriver(), new ɵNoopAnimationStyleNormalizer()),
fakeNgZone);
}

describe('custom renderer', () => {
@Component({
selector: 'some-component',
template: `<div><span></span></div>`,
})
class SomeComponent {
}

/**
* Creates a patched renderer factory that creates elements with a shape different than DOM node
*/
function createPatchedRendererFactory(document: any) {
let rendererFactory = getRendererFactory2(document);
const origCreateRenderer = rendererFactory.createRenderer;
rendererFactory.createRenderer = function(element: any, type: RendererType2|null) {
const renderer = origCreateRenderer.call(this, element, type);
renderer.appendChild = () => {};
renderer.createElement = (name: string) => ({
name,
el: document.createElement(name),
});
return renderer;
};

return rendererFactory;
}

beforeEach(() => {
TestBed.configureTestingModule({
declarations: [SomeComponent],
providers: [{
provide: RendererFactory2,
useFactory: (document: any) => createPatchedRendererFactory(document),
deps: [DOCUMENT]
}]
});
});

it('should not trigger errors', () => {
expect(() => {
const fixture = TestBed.createComponent(SomeComponent);
fixture.detectChanges();
}).not.toThrow();
});
});

0 comments on commit 11e04b1

Please sign in to comment.