Skip to content
Open
Show file tree
Hide file tree
Changes from 10 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
1 change: 1 addition & 0 deletions packages/eui/changelogs/upcoming/9511.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Replaced `EuiObserver` abstract base class with a `useObserver` hook
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
* Side Public License, v 1.
*/

import { ReactNode, useEffect } from 'react';
import { ReactNode, useCallback, useEffect, useRef, FunctionComponent } from 'react';
Comment thread
ragini-pandey marked this conversation as resolved.
Outdated

import { EuiObserver } from '../observer';
import { useObserver } from '../observer';

export interface EuiMutationObserverProps {
/**
Expand All @@ -19,24 +19,37 @@ export interface EuiMutationObserverProps {
observerOptions?: MutationObserverInit;
}

export class EuiMutationObserver extends EuiObserver<EuiMutationObserverProps> {
name = 'EuiMutationObserver';
export const EuiMutationObserver: FunctionComponent<EuiMutationObserverProps> = ({
children,
onMutation,
observerOptions,
}) => {
// Store onMutation and observerOptions in refs so the observer callback
// and setup always use the latest prop values without needing to
// re-subscribe (which would cause the ref callback to cycle)
const onMutationRef = useRef<MutationCallback>(onMutation);
onMutationRef.current = onMutation;

// the `onMutation` prop may change while the observer is bound, abstracting
// it out into a separate function means the current `onMutation` value is used
onMutation: MutationCallback = (records, observer) => {
this.props.onMutation(records, observer);
};
const observerOptionsRef = useRef(observerOptions);
observerOptionsRef.current = observerOptions;

beginObserve = () => {
const childNode = this.childNode!;
this.observer = makeMutationObserver(
childNode,
this.props.observerOptions,
this.onMutation
);
};
}
const mutationCallback: MutationCallback = useCallback(
(records, observer) => {
onMutationRef.current(records, observer);
},
[]
);

const beginObserve = useCallback(
(node: Element) =>
makeMutationObserver(node, observerOptionsRef.current, mutationCallback),
[mutationCallback]
);

const updateChildNode = useObserver(beginObserve, 'EuiMutationObserver');

return children(updateChildNode);
};

const makeMutationObserver = (
node: Element,
Expand Down
87 changes: 48 additions & 39 deletions packages/eui/src/components/observer/observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,59 +6,68 @@
* Side Public License, v 1.
*/

import { Component, ReactNode } from 'react';

interface BaseProps {
/**
* ReactNode to render as this component's content
*/
children: (ref: any) => ReactNode;
}
import { useCallback, useEffect, useRef } from 'react';

export interface Observer {
disconnect: () => void;
observe: (element: Element, options?: { [key: string]: any }) => void;
}

export class EuiObserver<Props extends BaseProps> extends Component<Props> {
protected name: string = 'EuiObserver';
protected childNode: null | Element = null;
protected observer: null | Observer = null;
/**
* A shared custom hook that provides a pattern for observing DOM nodes
* via browser observer APIs. Used by `EuiResizeObserver` and `EuiMutationObserver`.
*
* @param beginObserve - A callback that receives the target DOM element and should
* create and return the observer instance (e.g., `ResizeObserver`).
* @param componentName - Optional name used in error messages when no ref is
* attached on mount, mirroring the guard previously in `EuiObserver`.
*/
export const useObserver = (
beginObserve: (node: Element) => Observer | undefined,
componentName: string = 'useObserver'
) => {
const childNodeRef = useRef<Element | null>(null);
Comment thread
ragini-pandey marked this conversation as resolved.
const observerRef = useRef<Observer | null>(null);

componentDidMount() {
if (this.childNode == null) {
throw new Error(`${this.name} did not receive a ref`);
}
}
// Store beginObserve in a ref so the ref callback doesn't cycle
const beginObserveRef = useRef(beginObserve);
beginObserveRef.current = beginObserve;

componentWillUnmount() {
if (this.observer != null) {
this.observer.disconnect();
// Store componentName in a ref so the mount-only effect can access the
// latest value without needing it as a dependency.
const componentNameRef = useRef(componentName);
componentNameRef.current = componentName;

// Guard: throw if the ref callback was never called (no element attached),
// mirroring the check previously in EuiObserver.componentDidMount.
// Also cleans up the observer on unmount.
// Empty deps: run only on mount/unmount — componentName is only used for the
// error message and changing it must not disconnect/re-connect the observer.
useEffect(() => {
if (childNodeRef.current == null) {
throw new Error(`${componentNameRef.current} did not receive a ref`);
}
}
return () => {
observerRef.current?.disconnect();
};
}, []);
Comment thread
ragini-pandey marked this conversation as resolved.

updateChildNode = (ref: Element) => {
if (this.childNode === ref) return; // node hasn't changed
const updateChildNode = useCallback((ref: Element | null) => {
if (childNodeRef.current === ref) return; // node hasn't changed

// if there's an existing observer disconnect it
if (this.observer != null) {
this.observer.disconnect();
this.observer = null;
if (observerRef.current != null) {
observerRef.current.disconnect();
observerRef.current = null;
}

this.childNode = ref;
childNodeRef.current = ref;

if (this.childNode != null) {
this.beginObserve();
if (childNodeRef.current != null) {
observerRef.current =
beginObserveRef.current(childNodeRef.current) ?? null;
}
};

beginObserve: () => void = () => {
throw new Error('EuiObserver has no default observation method');
};
}, []);

render() {
const props: BaseProps = this.props;
return props.children(this.updateChildNode);
}
}
return updateChildNode;
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,15 @@
* Side Public License, v 1.
*/

import { ReactNode, useCallback, useEffect, useRef, useState } from 'react';
import { EuiObserver } from '../observer';
import {
ReactNode,
useCallback,
useEffect,
useRef,
useState,
FunctionComponent,
} from 'react';
import { useObserver } from '../observer';

export interface EuiResizeObserverProps {
/**
Expand All @@ -20,36 +27,36 @@ export interface EuiResizeObserverProps {
export const hasResizeObserver =
typeof window !== 'undefined' && typeof window.ResizeObserver !== 'undefined';

export class EuiResizeObserver extends EuiObserver<EuiResizeObserverProps> {
name = 'EuiResizeObserver';
export const EuiResizeObserver: FunctionComponent<EuiResizeObserverProps> = ({
children,
onResize,
}) => {
const onResizeRef = useRef(onResize);
onResizeRef.current = onResize;

state = {
height: 0,
width: 0,
};
const sizeRef = useRef({ height: 0, width: 0 });

onResize: ResizeObserverCallback = ([entry]) => {
const resizeCallback: ResizeObserverCallback = useCallback(([entry]) => {
const { inlineSize: width, blockSize: height } = entry.borderBoxSize[0];

// Check for actual resize event
if (this.state.height === height && this.state.width === width) {
if (sizeRef.current.height === height && sizeRef.current.width === width) {
return;
}

this.props.onResize({
height,
width,
});
this.setState({ height, width });
};

beginObserve = () => {
// The superclass checks that childNode is not null before invoking
// beginObserve()
const childNode = this.childNode!;
this.observer = makeResizeObserver(childNode, this.onResize)!;
};
}
sizeRef.current = { height, width };
onResizeRef.current({ height, width });
}, []);

const beginObserve = useCallback(
(node: Element) => makeResizeObserver(node, resizeCallback),
[resizeCallback]
);

const updateChildNode = useObserver(beginObserve, 'EuiResizeObserver');

return children(updateChildNode);
};

const makeResizeObserver = (
node: Element,
Expand Down
2 changes: 1 addition & 1 deletion packages/eui/src/components/popover/popover.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ describe('EuiPopover', () => {
expect(activeAnimationFrames.size).toEqual(1);

unmount();
expect(window.clearTimeout).toHaveBeenCalledTimes(9);
expect(window.clearTimeout).toHaveBeenCalledTimes(8);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it expected that clearTimeout is called 8 times and not 9?

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.

Yes, this is expected and intentional. The count dropped from 9 → 8 because the old EuiObserver class disconnected the observer twice on unmount:

  1. In componentWillUnmount — explicitly called this.observer.disconnect()
  2. Via the ref callback — React calls the ref with null on unmount, which also triggered updateChildNode(null)this.observer.disconnect()

Each disconnect() call invokes clearTimeout inside the MutationObserver polyfill (used in the test environment), so there were 2 disconnects = 2 extra clearTimeout calls, giving a total of 9.

The new useObserver hook disconnects only once — the updateChildNode ref callback handles disconnection when called with null on unmount. The useEffect cleanup also calls observerRef.current?.disconnect(), but by that point observerRef.current is already null (nulled out by the ref callback), making it a no-op. So there is now 1 observer disconnect = 1 clearTimeout, giving a total of 8.

This is the correct behavior — cleaning up once is better than twice, and the count change reflects that improvement.

expect(cafSpy).toHaveBeenCalledTimes(1);
expect(activeAnimationFrames.size).toEqual(0);
Comment thread
ragini-pandey marked this conversation as resolved.

Expand Down
Loading