Merge pull request #26142 from backstage/techdocs-shadowrootelements-update
fix:(techdocs): Unnecessary updates in `useShadowRootElements`
This commit is contained in:
@@ -0,0 +1,5 @@
|
||||
---
|
||||
'@backstage/plugin-techdocs-react': patch
|
||||
---
|
||||
|
||||
Fixed issue in useShadowRootElements which could lead to unlimited render loops
|
||||
@@ -16,10 +16,22 @@
|
||||
|
||||
import { TechDocsAddonTester } from '@backstage/plugin-techdocs-addons-test-utils';
|
||||
import React from 'react';
|
||||
import { fireEvent, waitFor, act } from '@testing-library/react';
|
||||
import { act, fireEvent, waitFor } from '@testing-library/react';
|
||||
import { TextSize } from '../plugin';
|
||||
import { useShadowRootElements } from '@backstage/plugin-techdocs-react';
|
||||
|
||||
jest.mock('@backstage/plugin-techdocs-react', () => ({
|
||||
...jest.requireActual('@backstage/plugin-techdocs-react'),
|
||||
useShadowRootElements: jest.fn(),
|
||||
}));
|
||||
|
||||
describe('TextSize', () => {
|
||||
const useShadowRootElementsMock = useShadowRootElements as jest.Mock;
|
||||
|
||||
beforeEach(() => {
|
||||
useShadowRootElementsMock.mockReturnValue([]);
|
||||
});
|
||||
|
||||
it('renders without exploding', async () => {
|
||||
const { getByText } = await TechDocsAddonTester.buildAddonsInTechDocs([
|
||||
<TextSize />,
|
||||
@@ -36,6 +48,9 @@ describe('TextSize', () => {
|
||||
.withDom(<body>TEST_CONTENT</body>)
|
||||
.renderWithEffects();
|
||||
|
||||
const content = getByText('TEST_CONTENT');
|
||||
useShadowRootElementsMock.mockReturnValue([content]);
|
||||
|
||||
fireEvent.click(getByTitle('Settings'));
|
||||
|
||||
await waitFor(() => {
|
||||
@@ -60,7 +75,9 @@ describe('TextSize', () => {
|
||||
|
||||
let style = window.getComputedStyle(getByText('TEST_CONTENT'));
|
||||
|
||||
expect(style.getPropertyValue('--md-typeset-font-size')).toBe('18.4px');
|
||||
await waitFor(() => {
|
||||
expect(style.getPropertyValue('--md-typeset-font-size')).toBe('18.4px');
|
||||
});
|
||||
|
||||
fireEvent.keyDown(slider, {
|
||||
key: 'ArrowLeft',
|
||||
@@ -88,6 +105,9 @@ describe('TextSize', () => {
|
||||
.withDom(<body>TEST_CONTENT</body>)
|
||||
.renderWithEffects();
|
||||
|
||||
const content = getByText('TEST_CONTENT');
|
||||
useShadowRootElementsMock.mockReturnValue([content]);
|
||||
|
||||
fireEvent.click(getByTitle('Settings'));
|
||||
|
||||
await waitFor(() => {
|
||||
|
||||
@@ -19,7 +19,7 @@ import {
|
||||
useShadowRootElements,
|
||||
useShadowRootSelection,
|
||||
} from './hooks';
|
||||
import { renderHook } from '@testing-library/react';
|
||||
import { act, renderHook } from '@testing-library/react';
|
||||
import { fireEvent, waitFor } from '@testing-library/react';
|
||||
|
||||
const fireSelectionChangeEvent = (window: Window) => {
|
||||
@@ -34,7 +34,7 @@ const getSelection = jest.fn();
|
||||
const mockShadowRoot = () => {
|
||||
const div = document.createElement('div');
|
||||
const shadowRoot = div.attachShadow({ mode: 'open' });
|
||||
shadowRoot.innerHTML = '<h1>Shadow DOM Mock</h1>';
|
||||
shadowRoot.innerHTML = '<div><h1>Shadow DOM Mock</h1></div>';
|
||||
(shadowRoot as ShadowRoot & Pick<Document, 'getSelection'>).getSelection =
|
||||
getSelection;
|
||||
return shadowRoot;
|
||||
@@ -85,6 +85,55 @@ describe('hooks', () => {
|
||||
|
||||
expect(result.current).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('should update elements if shadow root changes', async () => {
|
||||
const { result, rerender } = renderHook(() =>
|
||||
useShadowRootElements(['h1']),
|
||||
);
|
||||
|
||||
act(() => {
|
||||
shadowRoot.innerHTML = '<div><h1>Updated Shadow DOM Mock</h1></div>';
|
||||
rerender();
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current[0].textContent).toBe('Updated Shadow DOM Mock');
|
||||
});
|
||||
});
|
||||
|
||||
describe('mutation observer', () => {
|
||||
const observer: jest.Mocked<MutationObserver> = {
|
||||
observe: jest.fn(),
|
||||
disconnect: jest.fn(),
|
||||
takeRecords: jest.fn(),
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
jest
|
||||
.spyOn(window, 'MutationObserver')
|
||||
.mockImplementation(() => observer);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
it('should observe shadow root changes', async () => {
|
||||
renderHook(() => useShadowRootElements(['h1']));
|
||||
expect(observer.observe).toHaveBeenCalledWith(shadowRoot, {
|
||||
childList: true,
|
||||
attributes: true,
|
||||
characterData: true,
|
||||
subtree: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('should disconnect observer on unmount', async () => {
|
||||
const { unmount } = renderHook(() => useShadowRootElements(['h1']));
|
||||
unmount();
|
||||
expect(observer.disconnect).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('useShadowRootSelection', () => {
|
||||
|
||||
@@ -39,13 +39,13 @@ export const useShadowRootElements = <
|
||||
selectors: string[],
|
||||
): TReturnedElement[] => {
|
||||
const shadowRoot = useShadowRoot();
|
||||
const [render, rerender] = useState(false);
|
||||
const [root, setRootNode] = useState(shadowRoot?.firstChild);
|
||||
|
||||
useEffect(() => {
|
||||
let observer: MutationObserver;
|
||||
if (shadowRoot) {
|
||||
observer = new MutationObserver(() => {
|
||||
rerender(!render);
|
||||
setRootNode(shadowRoot?.firstChild);
|
||||
});
|
||||
observer.observe(shadowRoot, {
|
||||
attributes: true,
|
||||
@@ -55,12 +55,12 @@ export const useShadowRootElements = <
|
||||
});
|
||||
}
|
||||
return () => observer?.disconnect();
|
||||
}, [shadowRoot, render, rerender]);
|
||||
}, [shadowRoot]);
|
||||
|
||||
if (!shadowRoot) return [];
|
||||
if (!root || !(root instanceof HTMLElement)) return [];
|
||||
|
||||
return selectors
|
||||
.map(selector => shadowRoot.querySelectorAll<TReturnedElement>(selector))
|
||||
.map(selector => root.querySelectorAll<TReturnedElement>(selector))
|
||||
.filter(nodeList => nodeList.length)
|
||||
.map(nodeList => Array.from(nodeList))
|
||||
.flat();
|
||||
|
||||
+32
-22
@@ -20,6 +20,7 @@ import { CompoundEntityRef } from '@backstage/catalog-model';
|
||||
import {
|
||||
techdocsApiRef,
|
||||
TechDocsReaderPageProvider,
|
||||
useShadowRootElements,
|
||||
} from '@backstage/plugin-techdocs-react';
|
||||
import { renderInTestApp, TestApiProvider } from '@backstage/test-utils';
|
||||
|
||||
@@ -36,6 +37,7 @@ jest.mock('../useReaderState', () => ({
|
||||
jest.mock('@backstage/plugin-techdocs-react', () => ({
|
||||
...jest.requireActual('@backstage/plugin-techdocs-react'),
|
||||
useShadowDomStylesLoading: jest.fn().mockReturnValue(false),
|
||||
useShadowRootElements: jest.fn(),
|
||||
}));
|
||||
|
||||
import { TechDocsReaderPageContent } from './TechDocsReaderPageContent';
|
||||
@@ -88,6 +90,12 @@ const Wrapper = ({
|
||||
);
|
||||
|
||||
describe('<TechDocsReaderPageContent />', () => {
|
||||
const useShadowRootElementsMock = useShadowRootElements as jest.Mock;
|
||||
|
||||
beforeEach(() => {
|
||||
useShadowRootElementsMock.mockReturnValue([]);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
@@ -154,6 +162,29 @@ describe('<TechDocsReaderPageContent />', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('should scroll to header if hash is not present in url', async () => {
|
||||
jest.spyOn(document, 'querySelector');
|
||||
|
||||
getEntityMetadata.mockResolvedValue(mockEntityMetadata);
|
||||
getTechDocsMetadata.mockResolvedValue(mockTechDocsMetadata);
|
||||
useTechDocsReaderDom.mockReturnValue(document.createElement('html'));
|
||||
useReaderState.mockReturnValue({ state: 'cached' });
|
||||
|
||||
const rendered = await renderInTestApp(
|
||||
<Wrapper>
|
||||
<TechDocsReaderPageContent withSearch={false} />
|
||||
</Wrapper>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(
|
||||
rendered.getByTestId('techdocs-native-shadowroot'),
|
||||
).toBeInTheDocument();
|
||||
|
||||
expect(document.querySelector).toHaveBeenCalledWith('header');
|
||||
});
|
||||
});
|
||||
|
||||
it('should scroll to hash if hash is present in url', async () => {
|
||||
jest.spyOn(document, 'querySelector');
|
||||
|
||||
@@ -165,6 +196,7 @@ describe('<TechDocsReaderPageContent />', () => {
|
||||
const mockTechDocsPage = document.createElement('html');
|
||||
mockTechDocsPage.appendChild(h2);
|
||||
|
||||
useShadowRootElementsMock.mockReturnValue([h2]);
|
||||
getEntityMetadata.mockResolvedValue(mockEntityMetadata);
|
||||
getTechDocsMetadata.mockResolvedValue(mockTechDocsMetadata);
|
||||
useTechDocsReaderDom.mockReturnValue(mockTechDocsPage);
|
||||
@@ -188,26 +220,4 @@ describe('<TechDocsReaderPageContent />', () => {
|
||||
|
||||
window.location.hash = '';
|
||||
});
|
||||
|
||||
it('should scroll to header if hash is not present in url', async () => {
|
||||
jest.spyOn(document, 'querySelector');
|
||||
|
||||
getEntityMetadata.mockResolvedValue(mockEntityMetadata);
|
||||
getTechDocsMetadata.mockResolvedValue(mockTechDocsMetadata);
|
||||
useTechDocsReaderDom.mockReturnValue(document.createElement('html'));
|
||||
useReaderState.mockReturnValue({ state: 'cached' });
|
||||
|
||||
const rendered = await renderInTestApp(
|
||||
<Wrapper>
|
||||
<TechDocsReaderPageContent withSearch={false} />
|
||||
</Wrapper>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(
|
||||
rendered.getByTestId('techdocs-native-shadowroot'),
|
||||
).toBeInTheDocument();
|
||||
expect(document.querySelector).toHaveBeenCalledWith('header');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user