diff --git a/.changeset/twelve-ducks-swim.md b/.changeset/twelve-ducks-swim.md new file mode 100644 index 0000000000..1e44f09be1 --- /dev/null +++ b/.changeset/twelve-ducks-swim.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog': patch +--- + +Fixed an issue causing `EntitySwitch` to unmount its children once entity refresh was invoked diff --git a/.github/vale/Vocab/Backstage/accept.txt b/.github/vale/Vocab/Backstage/accept.txt index c61c6bdf46..7501010382 100644 --- a/.github/vale/Vocab/Backstage/accept.txt +++ b/.github/vale/Vocab/Backstage/accept.txt @@ -407,6 +407,7 @@ unbreak Unconference unicode unmanaged +unmount unregister unregistering unregistration diff --git a/plugins/catalog/src/components/EntitySwitch/EntitySwitch.test.tsx b/plugins/catalog/src/components/EntitySwitch/EntitySwitch.test.tsx index 13e214d823..c93922e938 100644 --- a/plugins/catalog/src/components/EntitySwitch/EntitySwitch.test.tsx +++ b/plugins/catalog/src/components/EntitySwitch/EntitySwitch.test.tsx @@ -19,8 +19,8 @@ import { AsyncEntityProvider, EntityProvider, } from '@backstage/plugin-catalog-react'; -import { render, screen } from '@testing-library/react'; -import React from 'react'; +import { render, screen, waitFor } from '@testing-library/react'; +import React, { useEffect } from 'react'; import { isKind } from './conditions'; import { EntitySwitch } from './EntitySwitch'; import { featureFlagsApiRef } from '@backstage/core-plugin-api'; @@ -35,6 +35,80 @@ const Wrapper = ({ children }: { children?: React.ReactNode }) => ( ); describe('EntitySwitch', () => { + it('should render only the first match', () => { + const content = ( + + + + + + ); + + render( + + + {content} + + , + ); + + expect(screen.getByText('A')).toBeInTheDocument(); + expect(screen.queryByText('B')).not.toBeInTheDocument(); + expect(screen.queryByText('C')).not.toBeInTheDocument(); + }); + + it('should render the fallback if no cases are matching', () => { + const content = ( + + + + + + ); + + render( + + + {content} + + , + ); + + expect(screen.queryByText('A')).not.toBeInTheDocument(); + expect(screen.queryByText('B')).not.toBeInTheDocument(); + expect(screen.getByText('C')).toBeInTheDocument(); + }); + + it('should render only the first fallback in case no cases are matching', () => { + const content = ( + + + + + + + ); + + render( + + + {content} + + , + ); + + expect(screen.queryByText('A')).not.toBeInTheDocument(); + expect(screen.queryByText('B')).not.toBeInTheDocument(); + expect(screen.getByText('C')).toBeInTheDocument(); + expect(screen.queryByText('D')).not.toBeInTheDocument(); + }); + it('should switch child when entity switches', () => { const content = ( @@ -96,7 +170,7 @@ describe('EntitySwitch', () => { expect(screen.queryByText('A')).not.toBeInTheDocument(); expect(screen.queryByText('B')).not.toBeInTheDocument(); - expect(screen.getByText('C')).toBeInTheDocument(); + expect(screen.queryByText('C')).not.toBeInTheDocument(); }); it('should switch child when filters switch', () => { @@ -209,6 +283,97 @@ describe('EntitySwitch', () => { expect(screen.queryByText('B')).not.toBeInTheDocument(); }); + it('should display the children in case entity is available and loading is true', () => { + const entity = { metadata: { name: 'mock' }, kind: 'component' } as Entity; + + render( + + + + Component

} + /> + System

} /> +
+
+
, + ); + + expect(screen.queryByText('Component')).toBeInTheDocument(); + expect(screen.queryByText('System')).not.toBeInTheDocument(); + }); + + it(`shouldn't unmount the children on entity refresh`, () => { + const entity = { metadata: { name: 'mock' }, kind: 'component' } as Entity; + + let mountsCount = 0; + function Component() { + useEffect(() => { + ++mountsCount; + }, []); + + return

Component

; + } + + const rendered = render( + + + + } + /> + System

} /> +
+
+
, + ); + + expect(screen.queryByText('Component')).toBeInTheDocument(); + expect(screen.queryByText('System')).not.toBeInTheDocument(); + + expect(mountsCount).toBe(1); + + rendered.rerender( + + + + } + /> + System

} /> +
+
+
, + ); + + expect(screen.queryByText('Component')).toBeInTheDocument(); + expect(screen.queryByText('System')).not.toBeInTheDocument(); + + expect(mountsCount).toBe(1); + + rendered.rerender( + + + + } + /> + System

} /> +
+
+
, + ); + + expect(screen.queryByText('Component')).toBeInTheDocument(); + expect(screen.queryByText('System')).not.toBeInTheDocument(); + + expect(mountsCount).toBe(1); + }); + it('should switch with async condition that is true', async () => { const entity = { metadata: { name: 'mock' }, kind: 'component' } as Entity; @@ -292,10 +457,10 @@ describe('EntitySwitch', () => { expect(screen.queryByText('A')).not.toBeInTheDocument(); }); - it('should switch with sync condition that throws', async () => { + it('should switch with async condition that throws', async () => { const entity = { metadata: { name: 'mock' }, kind: 'component' } as Entity; - const shouldRender = () => Promise.reject(); + const shouldRender = jest.fn().mockRejectedValue(undefined); render( @@ -308,7 +473,9 @@ describe('EntitySwitch', () => { , ); - await expect(screen.findByText('C')).resolves.toBeInTheDocument(); + await waitFor(() => expect(shouldRender).toHaveBeenCalled()); + + expect(screen.getByText('C')).toBeInTheDocument(); expect(screen.queryByText('A')).not.toBeInTheDocument(); expect(screen.queryByText('B')).not.toBeInTheDocument(); }); diff --git a/plugins/catalog/src/components/EntitySwitch/EntitySwitch.tsx b/plugins/catalog/src/components/EntitySwitch/EntitySwitch.tsx index 3cd806faa5..b12495052e 100644 --- a/plugins/catalog/src/components/EntitySwitch/EntitySwitch.tsx +++ b/plugins/catalog/src/components/EntitySwitch/EntitySwitch.tsx @@ -64,8 +64,9 @@ export interface EntitySwitchProps { /** @public */ export const EntitySwitch = (props: EntitySwitchProps) => { - const { entity, loading } = useAsyncEntity(); + const { entity } = useAsyncEntity(); const apis = useApiHolder(); + const results = useElementFilter( props.children, collection => @@ -76,23 +77,14 @@ export const EntitySwitch = (props: EntitySwitchProps) => { }) .getElements() .flatMap((element: ReactElement) => { - // Nothing is rendered while loading - if (loading) { + // If the entity is missing or there is an error, render nothing + if (!entity) { return []; } const { if: condition, children: elementsChildren } = element.props as EntitySwitchCase; - // If the entity is missing or there is an error, render the default page - if (!entity) { - return [ - { - if: condition === undefined, - children: elementsChildren, - }, - ]; - } return [ { if: condition?.(entity, { apis }), @@ -100,7 +92,7 @@ export const EntitySwitch = (props: EntitySwitchProps) => { }, ]; }), - [apis, entity, loading], + [apis, entity], ); const hasAsyncCases = results.some(