Skip to content

refactor: clean up removeChild usages #23592

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

Merged
merged 1 commit into from
Oct 6, 2021
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
10 changes: 2 additions & 8 deletions src/cdk-experimental/column-resize/resizable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,8 @@ export abstract class Resizable<HandleComponent extends ResizeOverlayHandle>
ngOnDestroy(): void {
this.destroyed.next();
this.destroyed.complete();

if (this.inlineHandle) {
this.elementRef.nativeElement!.removeChild(this.inlineHandle);
}

if (this.overlayRef) {
this.overlayRef.dispose();
}
this.inlineHandle?.remove();
this.overlayRef?.dispose();
}

protected abstract getInlineHandleCssClassName(): string;
Expand Down
7 changes: 2 additions & 5 deletions src/cdk-experimental/column-resize/resize-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,8 @@ export class CdkFlexTableResizeStrategy extends ResizeStrategy implements OnDest
}

ngOnDestroy(): void {
// TODO: Use remove() once we're off IE11.
if (this._styleElement && this._styleElement.parentNode) {
this._styleElement.parentNode.removeChild(this._styleElement);
this._styleElement = undefined;
}
this._styleElement?.remove();
this._styleElement = undefined;
}

private _getPropertyValue(cssFriendlyColumnName: string, key: string): string|undefined {
Expand Down
12 changes: 6 additions & 6 deletions src/cdk-experimental/dialog/dialog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -877,7 +877,7 @@ describe('Dialog', () => {
describe('focus management', () => {
// When testing focus, all of the elements must be in the DOM.
beforeEach(() => document.body.appendChild(overlayContainerElement));
afterEach(() => document.body.removeChild(overlayContainerElement));
afterEach(() => overlayContainerElement.remove());

it('should focus the first tabbable element of the dialog on open (the default)',
fakeAsync(() => {
Expand Down Expand Up @@ -971,7 +971,7 @@ describe('Dialog', () => {
.withContext('Expected that the trigger was refocused after the dialog is closed.')
.toBe('dialog-trigger');

document.body.removeChild(button);
button.remove();
}));

it('should re-focus trigger element inside the shadow DOM when dialog closes', fakeAsync(() => {
Expand Down Expand Up @@ -1036,8 +1036,8 @@ describe('Dialog', () => {
expect(document.activeElement!.id)
.withContext('Expected focus to stay on the alternate button.').toBe('other-button');

body.removeChild(button);
body.removeChild(otherButton);
button.remove();
otherButton.remove();
}));

it('should allow the consumer to shift focus in afterClosed', fakeAsync(() => {
Expand Down Expand Up @@ -1069,8 +1069,8 @@ describe('Dialog', () => {
.withContext('Expected that the trigger was refocused after the dialog is closed.')
.toBe('input-to-be-focused');

document.body.removeChild(button);
document.body.removeChild(input);
button.remove();
input.remove();
flush();
}));

Expand Down
2 changes: 1 addition & 1 deletion src/cdk-experimental/listbox/listbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ export class CdkOption<T = unknown> implements ListKeyManagerOption, Highlightab
private _removeIcons(element: Element) {
// TODO: make this a configurable function that can removed any desired type of node.
for (const icon of Array.from(element.querySelectorAll('mat-icon, .material-icons'))) {
icon.parentNode?.removeChild(icon);
icon.remove();
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/cdk-experimental/menu/menu-item.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import {Toggler, MENU_AIM, MenuAim} from './menu-aim';
/** Removes all icons from within the given element. */
function removeIcons(element: Element) {
for (const icon of Array.from(element.querySelectorAll('mat-icon, .material-icons'))) {
icon.parentNode?.removeChild(icon);
icon.remove();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,8 @@ export class CdkTableScrollContainer implements StickyPositioningListener,
}

ngOnDestroy(): void {
// TODO: Use remove() once we're off IE11.
if (this._styleElement?.parentNode) {
this._styleElement.parentNode.removeChild(this._styleElement);
this._styleElement = undefined;
}
this._styleElement?.remove();
this._styleElement = undefined;
}

stickyColumnsUpdated({sizes}: StickyUpdate): void {
Expand Down
5 changes: 1 addition & 4 deletions src/cdk/a11y/aria-describer/aria-describer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,7 @@ describe('AriaDescriber', () => {

// Use `querySelectorAll` with an attribute since `getElementById` will stop at the first match.
expect(document.querySelectorAll(`[id='${MESSAGES_CONTAINER_ID}']`).length).toBe(1);

if (extraContainer.parentNode) {
extraContainer.parentNode.removeChild(extraContainer);
}
extraContainer.remove();
});

it('should not describe messages that match up with the aria-label of the element', () => {
Expand Down
13 changes: 4 additions & 9 deletions src/cdk/a11y/aria-describer/aria-describer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,7 @@ export class AriaDescriber implements OnDestroy {
/** Deletes the message element from the global messages container. */
private _deleteMessageElement(key: string|Element) {
const registeredMessage = messageRegistry.get(key);
const messageElement = registeredMessage && registeredMessage.messageElement;
if (messagesContainer && messageElement) {
messagesContainer.removeChild(messageElement);
}
registeredMessage?.messageElement?.remove();
messageRegistry.delete(key);
}

Expand All @@ -172,9 +169,7 @@ export class AriaDescriber implements OnDestroy {
// already a container on the page, but we don't have a reference to it. Clear the
// old container so we don't get duplicates. Doing this, instead of emptying the previous
// container, should be slightly faster.
if (preExistingContainer && preExistingContainer.parentNode) {
preExistingContainer.parentNode.removeChild(preExistingContainer);
}
preExistingContainer?.remove();

messagesContainer = this._document.createElement('div');
messagesContainer.id = MESSAGES_CONTAINER_ID;
Expand All @@ -193,8 +188,8 @@ export class AriaDescriber implements OnDestroy {

/** Deletes the global messages container. */
private _deleteMessagesContainer() {
if (messagesContainer && messagesContainer.parentNode) {
messagesContainer.parentNode.removeChild(messagesContainer);
if (messagesContainer) {
messagesContainer.remove();
messagesContainer = null;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/cdk/a11y/aria-describer/aria-reference.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ describe('AriaReference', () => {
});

afterEach(() => {
document.body.removeChild(testElement!);
testElement?.remove();
});

it('should be able to append/remove aria reference IDs', () => {
Expand Down
10 changes: 2 additions & 8 deletions src/cdk/a11y/focus-trap/focus-trap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,18 +76,12 @@ export class FocusTrap {

if (startAnchor) {
startAnchor.removeEventListener('focus', this.startAnchorListener);

if (startAnchor.parentNode) {
startAnchor.parentNode.removeChild(startAnchor);
}
startAnchor.remove();
}

if (endAnchor) {
endAnchor.removeEventListener('focus', this.endAnchorListener);

if (endAnchor.parentNode) {
endAnchor.parentNode.removeChild(endAnchor);
}
endAnchor.remove();
}

this._startAnchor = this._endAnchor = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export class HighContrastModeDetector {
documentWindow.getComputedStyle(testElement) : null;
const computedColor =
(computedStyle && computedStyle.backgroundColor || '').replace(/ /g, '');
this._document.body.removeChild(testElement);
testElement.remove();

switch (computedColor) {
case 'rgb(0,0,0)': return HighContrastMode.WHITE_ON_BLACK;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe('InteractivityChecker', () => {
}));

afterEach(() => {
document.body.removeChild(testContainerElement);
testContainerElement.remove();
testContainerElement.innerHTML = '';
});

Expand Down Expand Up @@ -529,8 +529,7 @@ describe('InteractivityChecker', () => {
tmpRoot.innerHTML = template;

const element = tmpRoot.firstElementChild!;

tmpRoot.removeChild(element);
element.remove();

if (append) {
appendElements([element]);
Expand Down
5 changes: 1 addition & 4 deletions src/cdk/a11y/live-announcer/live-announcer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,7 @@ describe('LiveAnnouncer', () => {
expect(document.body.querySelectorAll('.cdk-live-announcer-element').length)
.withContext('Expected only one live announcer element in the DOM.')
.toBe(1);

if (extraElement.parentNode) {
extraElement.parentNode.removeChild(extraElement);
}
extraElement.remove();
}));

it('should clear any previous timers when a new one is started', fakeAsync(() => {
Expand Down
9 changes: 3 additions & 6 deletions src/cdk/a11y/live-announcer/live-announcer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,8 @@ export class LiveAnnouncer implements OnDestroy {

ngOnDestroy() {
clearTimeout(this._previousTimeout);

if (this._liveElement && this._liveElement.parentNode) {
this._liveElement.parentNode.removeChild(this._liveElement);
this._liveElement = null!;
}
this._liveElement?.remove();
this._liveElement = null!;
}

private _createLiveElement(): HTMLElement {
Expand All @@ -156,7 +153,7 @@ export class LiveAnnouncer implements OnDestroy {

// Remove any old containers. This can happen when coming in from a server-side-rendered page.
for (let i = 0; i < previousElements.length; i++) {
previousElements[i].parentNode!.removeChild(previousElements[i]);
previousElements[i].remove();
}

liveEl.classList.add(elementClass);
Expand Down
7 changes: 2 additions & 5 deletions src/cdk/clipboard/clipboard.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@ describe('Clipboard', () => {
});

afterEach(() => {
if (focusedInput.parentNode) {
focusedInput.parentNode.removeChild(focusedInput);
}
focusedInput.remove();
});

describe('#beginCopy', () => {
Expand Down Expand Up @@ -79,8 +77,7 @@ describe('Clipboard', () => {

clipboard.copy(COPY_CONTENT);
expect(document.activeElement).toBe(svg);

svg.parentNode!.removeChild(svg);
svg.remove();
});

describe('when execCommand fails', () => {
Expand Down
5 changes: 1 addition & 4 deletions src/cdk/clipboard/pending-copy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,7 @@ export class PendingCopy {
const textarea = this._textarea;

if (textarea) {
if (textarea.parentNode) {
textarea.parentNode.removeChild(textarea);
}

textarea.remove();
this._textarea = undefined;
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/cdk/drag-drop/directives/drag.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2336,7 +2336,7 @@ expect(item.style.top)
const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement;

expect(preview.parentNode).toBe(fakeDocument.fullscreenElement);
fakeDocument.fullscreenElement.parentNode!.removeChild(fakeDocument.fullscreenElement);
fakeDocument.fullscreenElement.remove();
}));

it('should be able to constrain the preview position', fakeAsync(() => {
Expand Down Expand Up @@ -5831,7 +5831,7 @@ expect(sourceCanvas.getContext('2d')!
targetRect.left + 1, targetRect.top + 1);

// Remove the extra node after the element was dropped, but before the animation is over.
extraSibling.parentNode!.removeChild(extraSibling);
extraSibling.remove();

expect(() => {
flush();
Expand Down Expand Up @@ -7286,7 +7286,7 @@ function makeScrollable(

return () => {
scrollTo(0, 0);
veryTallElement.parentNode!.removeChild(veryTallElement);
veryTallElement.remove();
};
}

Expand Down
34 changes: 6 additions & 28 deletions src/cdk/drag-drop/drag-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -485,10 +485,10 @@ export class DragRef<T = any> {
if (this.isDragging()) {
// Since we move out the element to the end of the body while it's being
// dragged, we have to make sure that it's removed if it gets destroyed.
removeNode(this._rootElement);
this._rootElement?.remove();
}

removeNode(this._anchor);
this._anchor?.remove();
this._destroyPreview();
this._destroyPlaceholder();
this._dragDropRegistry.removeDragItem(this);
Expand Down Expand Up @@ -606,27 +606,15 @@ export class DragRef<T = any> {

/** Destroys the preview element and its ViewRef. */
private _destroyPreview() {
if (this._preview) {
removeNode(this._preview);
}

if (this._previewRef) {
this._previewRef.destroy();
}

this._preview?.remove();
this._previewRef?.destroy();
this._preview = this._previewRef = null!;
}

/** Destroys the placeholder element and its ViewRef. */
private _destroyPlaceholder() {
if (this._placeholder) {
removeNode(this._placeholder);
}

if (this._placeholderRef) {
this._placeholderRef.destroy();
}

this._placeholder?.remove();
this._placeholderRef?.destroy();
this._placeholder = this._placeholderRef = null!;
}

Expand Down Expand Up @@ -1481,16 +1469,6 @@ function clamp(value: number, min: number, max: number) {
return Math.max(min, Math.min(max, value));
}

/**
* Helper to remove a node from the DOM and to do all the necessary null checks.
* @param node Node to be removed.
*/
function removeNode(node: Node | null) {
if (node && node.parentNode) {
node.parentNode.removeChild(node);
}
}

/** Determines whether an event is a touch event. */
function isTouchEvent(event: MouseEvent | TouchEvent): event is TouchEvent {
// This function is called for every pixel that the user has dragged so we need it to be
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ describe('OverlayKeyboardDispatcher', () => {
dispatchKeyboardEvent(button, 'keydown', ESCAPE);

expect(spy).not.toHaveBeenCalled();

button.parentNode!.removeChild(button);
button.remove();
});

it('should complete the keydown stream on dispose', () => {
Expand Down
Loading