Skip to content
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(cdk/portal): remove ComponentFactoryResolver usages #27427

Merged
merged 1 commit into from
Sep 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 23 additions & 19 deletions src/cdk/portal/dom-portal-outlet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@

import {
ApplicationRef,
ComponentFactoryResolver,
ComponentRef,
EmbeddedViewRef,
Injector,
NgModuleRef,
createComponent,
} from '@angular/core';
import {BasePortalOutlet, ComponentPortal, DomPortal, TemplatePortal} from './portal';

Expand All @@ -36,7 +37,11 @@ export class DomPortalOutlet extends BasePortalOutlet {
constructor(
/** Element into which the content is projected. */
public outletElement: Element,
private _componentFactoryResolver?: ComponentFactoryResolver,
/**
* @deprecated No longer in use. To be removed.
* @breaking-change 18.0.0
*/
_componentFactoryResolver?: any,
private _appRef?: ApplicationRef,
private _defaultInjector?: Injector,

Expand All @@ -51,41 +56,40 @@ export class DomPortalOutlet extends BasePortalOutlet {
}

/**
* Attach the given ComponentPortal to DOM element using the ComponentFactoryResolver.
* Attach the given ComponentPortal to DOM element.
* @param portal Portal to be attached
* @returns Reference to the created component.
*/
attachComponentPortal<T>(portal: ComponentPortal<T>): ComponentRef<T> {
const resolver = (portal.componentFactoryResolver || this._componentFactoryResolver)!;

if ((typeof ngDevMode === 'undefined' || ngDevMode) && !resolver) {
throw Error('Cannot attach component portal to outlet without a ComponentFactoryResolver.');
}

const componentFactory = resolver.resolveComponentFactory(portal.component);
let componentRef: ComponentRef<T>;

// If the portal specifies a ViewContainerRef, we will use that as the attachment point
// for the component (in terms of Angular's component tree, not rendering).
// When the ViewContainerRef is missing, we use the factory to create the component directly
// and then manually attach the view to the application.
if (portal.viewContainerRef) {
componentRef = portal.viewContainerRef.createComponent(
componentFactory,
portal.viewContainerRef.length,
portal.injector || portal.viewContainerRef.injector,
portal.projectableNodes || undefined,
);
const injector = portal.injector || portal.viewContainerRef.injector;
const ngModuleRef = injector.get(NgModuleRef, null, {optional: true}) || undefined;

componentRef = portal.viewContainerRef.createComponent(portal.component, {
index: portal.viewContainerRef.length,
injector,
ngModuleRef,
projectableNodes: portal.projectableNodes || undefined,
});

this.setDisposeFn(() => componentRef.destroy());
} else {
if ((typeof ngDevMode === 'undefined' || ngDevMode) && !this._appRef) {
throw Error('Cannot attach component portal to outlet without an ApplicationRef.');
}

componentRef = componentFactory.create(
portal.injector || this._defaultInjector || Injector.NULL,
);
componentRef = createComponent(portal.component, {
elementInjector: portal.injector || this._defaultInjector || Injector.NULL,
environmentInjector: this._appRef!.injector,
projectableNodes: portal.projectableNodes || undefined,
});

this._appRef!.attachView(componentRef.hostView);
this.setDisposeFn(() => {
// Verify that the ApplicationRef has registered views before trying to detach a host view.
Expand Down
22 changes: 10 additions & 12 deletions src/cdk/portal/portal-directives.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
*/

import {
ComponentFactoryResolver,
ComponentRef,
Directive,
EmbeddedViewRef,
Expand All @@ -20,6 +19,7 @@ import {
ViewContainerRef,
Input,
inject,
NgModuleRef,
} from '@angular/core';
import {DOCUMENT} from '@angular/common';
import {BasePortalOutlet, ComponentPortal, Portal, TemplatePortal, DomPortal} from './portal';
Expand Down Expand Up @@ -79,9 +79,9 @@ export type CdkPortalOutletAttachedRef = ComponentRef<any> | EmbeddedViewRef<any
standalone: true,
})
export class CdkPortalOutlet extends BasePortalOutlet implements OnInit, OnDestroy {
private _componentFactoryResolver = inject(ComponentFactoryResolver);
private _viewContainerRef = inject(ViewContainerRef);
private _moduleRef = inject(NgModuleRef, {optional: true});
private _document = inject(DOCUMENT);
private _viewContainerRef = inject(ViewContainerRef);

/** Whether the portal component is initialized. */
private _isInitialized = false;
Expand Down Expand Up @@ -140,7 +140,7 @@ export class CdkPortalOutlet extends BasePortalOutlet implements OnInit, OnDestr
}

/**
* Attach the given ComponentPortal to this PortalOutlet using the ComponentFactoryResolver.
* Attach the given ComponentPortal to this PortalOutlet.
*
* @param portal Portal to be attached to the portal outlet.
* @returns Reference to the created component.
Expand All @@ -153,14 +153,12 @@ export class CdkPortalOutlet extends BasePortalOutlet implements OnInit, OnDestr
const viewContainerRef =
portal.viewContainerRef != null ? portal.viewContainerRef : this._viewContainerRef;

const resolver = portal.componentFactoryResolver || this._componentFactoryResolver;
const componentFactory = resolver.resolveComponentFactory(portal.component);
const ref = viewContainerRef.createComponent(
componentFactory,
viewContainerRef.length,
portal.injector || viewContainerRef.injector,
portal.projectableNodes || undefined,
);
const ref = viewContainerRef.createComponent(portal.component, {
index: viewContainerRef.length,
injector: portal.injector || viewContainerRef.injector,
projectableNodes: portal.projectableNodes || undefined,
ngModuleRef: this._moduleRef || undefined,
});

// If we're using a view container that's different from the injected one (e.g. when the portal
// specifies its own) we need to move the component into the outlet, otherwise it'll be rendered
Expand Down
41 changes: 1 addition & 40 deletions src/cdk/portal/portal.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,12 @@ import {
AfterViewInit,
ApplicationRef,
Component,
ComponentFactoryResolver,
ComponentRef,
Directive,
ElementRef,
Injector,
QueryList,
TemplateRef,
Type,
ViewChild,
ViewChildren,
ViewContainerRef,
Expand Down Expand Up @@ -38,12 +36,10 @@ describe('Portals', () => {

describe('CdkPortalOutlet', () => {
let fixture: ComponentFixture<PortalTestApp>;
let componentFactoryResolver: ComponentFactoryResolver;

beforeEach(() => {
fixture = TestBed.createComponent(PortalTestApp);
fixture.detectChanges();
componentFactoryResolver = TestBed.inject(ComponentFactoryResolver);
});

it('should load a component into the portal', () => {
Expand Down Expand Up @@ -451,19 +447,6 @@ describe('Portals', () => {
expect(instance.portalOutlet.hasAttached()).toBe(true);
});

it('should use the `ComponentFactoryResolver` from the portal, if available', () => {
const spy = jasmine.createSpy('resolveComponentFactorySpy');
const portal = new ComponentPortal(PizzaMsg, undefined, undefined, {
resolveComponentFactory: <T>(...args: [Type<T>]) => {
spy();
return componentFactoryResolver.resolveComponentFactory(...args);
},
});

fixture.componentInstance.portalOutlet.attachComponentPortal(portal);
expect(spy).toHaveBeenCalled();
});

it('should render inside outlet when component portal specifies view container ref', () => {
const hostContainer = fixture.nativeElement.querySelector('.portal-container');
const portal = new ComponentPortal(PizzaMsg, fixture.componentInstance.alternateContainer);
Expand Down Expand Up @@ -491,7 +474,6 @@ describe('Portals', () => {
});

describe('DomPortalOutlet', () => {
let componentFactoryResolver: ComponentFactoryResolver;
let someViewContainerRef: ViewContainerRef;
let someInjector: Injector;
let someFixture: ComponentFixture<ArbitraryViewContainerRefComponent>;
Expand All @@ -501,18 +483,10 @@ describe('Portals', () => {
let appRef: ApplicationRef;

beforeEach(() => {
componentFactoryResolver = TestBed.inject(ComponentFactoryResolver);
injector = TestBed.inject(Injector);
appRef = TestBed.inject(ApplicationRef);
someDomElement = document.createElement('div');
host = new DomPortalOutlet(
someDomElement,
componentFactoryResolver,
appRef,
injector,
document,
);

host = new DomPortalOutlet(someDomElement, null, appRef, injector, document);
someFixture = TestBed.createComponent(ArbitraryViewContainerRefComponent);
someViewContainerRef = someFixture.componentInstance.viewContainerRef;
someInjector = someFixture.componentInstance.injector;
Expand Down Expand Up @@ -669,19 +643,6 @@ describe('Portals', () => {
expect(spy).toHaveBeenCalled();
});

it('should use the `ComponentFactoryResolver` from the portal, if available', () => {
const spy = jasmine.createSpy('resolveComponentFactorySpy');
const portal = new ComponentPortal(PizzaMsg, undefined, undefined, {
resolveComponentFactory: <T>(...args: [Type<T>]) => {
spy();
return componentFactoryResolver.resolveComponentFactory(...args);
},
});

host.attachComponentPortal(portal);
expect(spy).toHaveBeenCalled();
});

it('should attach and detach a DOM portal', () => {
const fixture = TestBed.createComponent(PortalTestApp);
fixture.detectChanges();
Expand Down
14 changes: 8 additions & 6 deletions src/cdk/portal/portal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
ComponentRef,
EmbeddedViewRef,
Injector,
ComponentFactoryResolver,
} from '@angular/core';
import {
throwNullPortalOutletError,
Expand Down Expand Up @@ -96,10 +95,10 @@ export class ComponentPortal<T> extends Portal<ComponentRef<T>> {
injector?: Injector | null;

/**
* Alternate `ComponentFactoryResolver` to use when resolving the associated component.
* Defaults to using the resolver from the outlet that the portal is attached to.
* @deprecated No longer in use. To be removed.
* @breaking-change 18.0.0
*/
componentFactoryResolver?: ComponentFactoryResolver | null;
componentFactoryResolver?: any;

/**
* List of DOM nodes that should be projected through `<ng-content>` of the attached component.
Expand All @@ -110,14 +109,17 @@ export class ComponentPortal<T> extends Portal<ComponentRef<T>> {
component: ComponentType<T>,
viewContainerRef?: ViewContainerRef | null,
injector?: Injector | null,
componentFactoryResolver?: ComponentFactoryResolver | null,
/**
* @deprecated No longer in use. To be removed.
* @breaking-change 18.0.0
*/
_componentFactoryResolver?: any,
projectableNodes?: Node[][] | null,
) {
super();
this.component = component;
this.viewContainerRef = viewContainerRef;
this.injector = injector;
this.componentFactoryResolver = componentFactoryResolver;
this.projectableNodes = projectableNodes;
}
}
Expand Down
20 changes: 1 addition & 19 deletions src/material/dialog/dialog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ import {SpyLocation} from '@angular/common/testing';
import {
ChangeDetectionStrategy,
Component,
ComponentFactoryResolver,
ComponentRef,
createNgModuleRef,
Directive,
Inject,
Injectable,
Expand All @@ -27,7 +26,6 @@ import {
ViewChild,
ViewContainerRef,
ViewEncapsulation,
createNgModuleRef,
forwardRef,
signal,
} from '@angular/core';
Expand Down Expand Up @@ -119,7 +117,6 @@ describe('MatDialog', () => {

expect(overlayContainerElement.textContent).toContain('Pizza');
expect(dialogRef.componentInstance instanceof PizzaMsg).toBe(true);
expect(dialogRef.componentRef instanceof ComponentRef).toBe(true);
expect(dialogRef.componentInstance.dialogRef).toBe(dialogRef);

viewContainerFixture.detectChanges();
Expand Down Expand Up @@ -746,21 +743,6 @@ describe('MatDialog', () => {
expect(scrollStrategy.enable).toHaveBeenCalled();
}));

it('should be able to pass in an alternate ComponentFactoryResolver', inject(
[ComponentFactoryResolver],
(resolver: ComponentFactoryResolver) => {
spyOn(resolver, 'resolveComponentFactory').and.callThrough();

dialog.open(PizzaMsg, {
viewContainerRef: testViewContainerRef,
componentFactoryResolver: resolver,
});
viewContainerFixture.detectChanges();

expect(resolver.resolveComponentFactory).toHaveBeenCalled();
},
));

describe('passing in data', () => {
it('should be able to pass in data', () => {
let config = {data: {stringParam: 'hello', dateParam: new Date()}};
Expand Down
10 changes: 6 additions & 4 deletions tools/public_api_guard/cdk/portal.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
```ts

import { ApplicationRef } from '@angular/core';
import { ComponentFactoryResolver } from '@angular/core';
import { ComponentRef } from '@angular/core';
import { ElementRef } from '@angular/core';
import { EmbeddedViewRef } from '@angular/core';
Expand Down Expand Up @@ -77,9 +76,11 @@ export type CdkPortalOutletAttachedRef = ComponentRef<any> | EmbeddedViewRef<any

// @public
export class ComponentPortal<T> extends Portal<ComponentRef<T>> {
constructor(component: ComponentType<T>, viewContainerRef?: ViewContainerRef | null, injector?: Injector | null, componentFactoryResolver?: ComponentFactoryResolver | null, projectableNodes?: Node[][] | null);
constructor(component: ComponentType<T>, viewContainerRef?: ViewContainerRef | null, injector?: Injector | null,
_componentFactoryResolver?: any, projectableNodes?: Node[][] | null);
component: ComponentType<T>;
componentFactoryResolver?: ComponentFactoryResolver | null;
// @deprecated (undocumented)
componentFactoryResolver?: any;
injector?: Injector | null;
projectableNodes?: Node[][] | null;
viewContainerRef?: ViewContainerRef | null;
Expand All @@ -104,7 +105,8 @@ export class DomPortalHost extends DomPortalOutlet {
// @public
export class DomPortalOutlet extends BasePortalOutlet {
constructor(
outletElement: Element, _componentFactoryResolver?: ComponentFactoryResolver | undefined, _appRef?: ApplicationRef | undefined, _defaultInjector?: Injector | undefined,
outletElement: Element,
_componentFactoryResolver?: any, _appRef?: ApplicationRef | undefined, _defaultInjector?: Injector | undefined,
_document?: any);
attachComponentPortal<T>(portal: ComponentPortal<T>): ComponentRef<T>;
// @deprecated
Expand Down
Loading