From e535ea766a257f1bb8635b833c601d64d91deb39 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 17 Oct 2021 13:07:37 +0200 Subject: [PATCH 1/3] core-components: use built-in async react-syntax-highlighter component Signed-off-by: Patrik Oldsberg --- .changeset/brave-spoons-grow.md | 5 ++ .../CodeSnippet/CodeSnippet.test.tsx | 6 ++ .../components/CodeSnippet/CodeSnippet.tsx | 86 ++++++++----------- 3 files changed, 46 insertions(+), 51 deletions(-) create mode 100644 .changeset/brave-spoons-grow.md diff --git a/.changeset/brave-spoons-grow.md b/.changeset/brave-spoons-grow.md new file mode 100644 index 0000000000..d1bd166510 --- /dev/null +++ b/.changeset/brave-spoons-grow.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-components': patch +--- + +Switched to relying on the built-in support for async loading in `react-syntax-highlighter`. This should provide further improvements to async rendering and lazy loading, and avoid test flakiness that was happening because of the significant number or resources being loaded in lazily all at once. diff --git a/packages/core-components/src/components/CodeSnippet/CodeSnippet.test.tsx b/packages/core-components/src/components/CodeSnippet/CodeSnippet.test.tsx index a4ffd3462b..fec5b7ddf7 100644 --- a/packages/core-components/src/components/CodeSnippet/CodeSnippet.test.tsx +++ b/packages/core-components/src/components/CodeSnippet/CodeSnippet.test.tsx @@ -31,6 +31,12 @@ const minProps = { }; describe('', () => { + // react-syntax-highlighter is large and can cause significant slowdowns + // This test makes sure we're loading things in asynchronously and not too broadly. + it('renders quickly', async () => { + await renderInTestApp(); + }, 1000); + it('renders text without exploding', async () => { const { getByText } = await renderInTestApp(); expect(getByText(/"Hello"/)).toBeInTheDocument(); diff --git a/packages/core-components/src/components/CodeSnippet/CodeSnippet.tsx b/packages/core-components/src/components/CodeSnippet/CodeSnippet.tsx index 3aa2f42984..e1c407276b 100644 --- a/packages/core-components/src/components/CodeSnippet/CodeSnippet.tsx +++ b/packages/core-components/src/components/CodeSnippet/CodeSnippet.tsx @@ -14,56 +14,13 @@ * limitations under the License. */ -import React, { lazy, Suspense } from 'react'; +import React from 'react'; import { useTheme } from '@material-ui/core/styles'; import { BackstageTheme } from '@backstage/theme'; import { CopyTextButton } from '../CopyTextButton'; -import { Progress } from '../Progress'; - -const LazySyntaxHighlighter = lazy(async () => { - const [{ default: SyntaxHighlighter }, { docco, dark }] = await Promise.all([ - import('react-syntax-highlighter'), - import('react-syntax-highlighter/dist/cjs/styles/hljs'), - ]); - - function LazyHighlighter(props: CodeSnippetProps) { - const { - text, - language, - showLineNumbers = false, - highlightedNumbers, - customStyle, - } = props; - const theme = useTheme(); - const mode = theme.palette.type === 'dark' ? dark : docco; - const highlightColor = - theme.palette.type === 'dark' ? '#256bf3' : '#e6ffed'; - - return ( - - highlightedNumbers?.includes(lineNumber) - ? { - style: { - backgroundColor: highlightColor, - }, - } - : {} - } - > - {text} - - ); - } - - return { default: LazyHighlighter }; -}); +import { LightAsync } from 'react-syntax-highlighter'; +import dark from 'react-syntax-highlighter/dist/esm/styles/hljs/dark'; +import docco from 'react-syntax-highlighter/dist/esm/styles/hljs/docco'; /** * Properties for {@link CodeSnippet} @@ -112,12 +69,39 @@ export interface CodeSnippetProps { * providing consistent theming and copy code button */ export function CodeSnippet(props: CodeSnippetProps) { - const { text, showCopyCodeButton = false } = props; + const { + text, + language, + showLineNumbers = false, + highlightedNumbers, + customStyle, + showCopyCodeButton = false, + } = props; + const theme = useTheme(); + const mode = theme.palette.type === 'dark' ? dark : docco; + const highlightColor = theme.palette.type === 'dark' ? '#256bf3' : '#e6ffed'; + return (
- }> - - + + highlightedNumbers?.includes(lineNumber) + ? { + style: { + backgroundColor: highlightColor, + }, + } + : {} + } + > + {text} + {showCopyCodeButton && (
From ef3adaf8d635b5ef50e094bf841de83cc40baeb0 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 17 Oct 2021 13:16:29 +0200 Subject: [PATCH 2/3] catalog-import: switch tests back to sync to be sure it works Signed-off-by: Patrik Oldsberg --- .../PreviewCatalogInfoComponent.test.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/plugins/catalog-import/src/components/StepPrepareCreatePullRequest/PreviewCatalogInfoComponent.test.tsx b/plugins/catalog-import/src/components/StepPrepareCreatePullRequest/PreviewCatalogInfoComponent.test.tsx index ea3ea3e96f..fe5e76c968 100644 --- a/plugins/catalog-import/src/components/StepPrepareCreatePullRequest/PreviewCatalogInfoComponent.test.tsx +++ b/plugins/catalog-import/src/components/StepPrepareCreatePullRequest/PreviewCatalogInfoComponent.test.tsx @@ -45,7 +45,7 @@ const entities: Entity[] = [ ]; describe('', () => { - it('renders without exploding', async () => { + it('renders without exploding', () => { render( ', () => { const repositoryUrl = screen.getByText( 'http://my-repository/a/catalog-info.yaml', ); - const kindText = await screen.findByText('Kind_2'); + const kindText = screen.getByText(/Kind_2/); expect(repositoryUrl).toBeInTheDocument(); expect(repositoryUrl).toBeVisible(); expect(kindText).toBeInTheDocument(); expect(kindText).toBeVisible(); }); - it('renders card with custom styles', async () => { + it('renders card with custom styles', () => { const { result } = renderHook(() => useStyles()); render( @@ -77,14 +77,14 @@ describe('', () => { const repositoryUrl = screen.getByText( 'http://my-repository/a/catalog-info.yaml', ); - const kindText = await screen.findByText('Kind_2'); + const kindText = screen.getByText(/Kind_2/); expect(repositoryUrl).toBeInTheDocument(); expect(repositoryUrl).not.toBeVisible(); expect(kindText).toBeInTheDocument(); expect(kindText).not.toBeVisible(); }); - it('renders with custom styles', async () => { + it('renders with custom styles', () => { const { result } = renderHook(() => useStyles()); render( @@ -98,7 +98,7 @@ describe('', () => { const repositoryUrl = screen.getByText( 'http://my-repository/a/catalog-info.yaml', ); - const kindText = await screen.findByText('Kind_2'); + const kindText = screen.getByText(/Kind_2/); expect(repositoryUrl).toBeInTheDocument(); expect(repositoryUrl).toBeVisible(); expect(kindText).toBeInTheDocument(); From 028938b73e130375a3e0ed62e37dda05ffaae7da Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 17 Oct 2021 13:57:55 +0200 Subject: [PATCH 3/3] core-components: fix MardownContent test Signed-off-by: Patrik Oldsberg --- .../MarkdownContent/MarkdownContent.test.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/core-components/src/components/MarkdownContent/MarkdownContent.test.tsx b/packages/core-components/src/components/MarkdownContent/MarkdownContent.test.tsx index d39e92c75e..2e1550078f 100644 --- a/packages/core-components/src/components/MarkdownContent/MarkdownContent.test.tsx +++ b/packages/core-components/src/components/MarkdownContent/MarkdownContent.test.tsx @@ -52,14 +52,14 @@ describe('', () => { it('render MarkdownContent component with CodeSnippet for code blocks', async () => { const rendered = await renderWithEffects( - wrapInTestApp(), + wrapInTestApp( + , + ), ); - const fp1 = rendered.getByText('jest', { selector: 'span' }); + const fp1 = await rendered.findByText('jest(test:', { selector: 'span' }); expect(fp1).toBeInTheDocument(); - expect(fp1.className).toEqual('hljs-function'); - const fp2 = rendered.getByText('(test: string)', { selector: 'span' }); + const fp2 = rendered.getByText('string', { selector: 'span' }); expect(fp2).toBeInTheDocument(); - expect(fp2.className).toEqual('hljs-function'); - expect(rendered.getByText(';', { selector: 'span' })).toBeInTheDocument(); + expect(rendered.getByText(');', { selector: 'span' })).toBeInTheDocument(); }); });