paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 26 Packages Projects Releases Wiki Activity

Merge remote-tracking branch 'upstream/master' into feat/techdocs-async-transformer

Browse Source
This commit is contained in:
Dominik Henneke
2021-07-21 14:00:11 +02:00
parent 445440a408 17b6919001
commit af46ffabfd
4 changed files with 211 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/techdocs-fifty-cameras-listen.md
+7
View File
@@ -0,0 +1,7 @@
---
'@backstage/plugin-techdocs': patch
---
Only update the `path` when the content is updated.
If content and path are updated independently, the frontend rendering is triggered twice on each navigation: Once for the `path` change (with the old content) and once for the new content.
This might result in a flickering rendering that is caused by the async frontend preprocessing, and the fact that replacing the shadow dom content is expensive.
plugins/techdocs/src/reader/components/Reader.tsx
+6 -2
View File
@@ -61,18 +61,18 @@ const useStyles = makeStyles<BackstageTheme>(() => ({
export const Reader = ({ entityId, onReady }: Props) => {
const { kind, namespace, name } = entityId;
const { '*': path } = useParams();
const theme = useTheme<BackstageTheme>();
const classes = useStyles();
const {
state,
path,
contentReload,
content: rawPage,
contentErrorMessage,
syncErrorMessage,
buildLog,
} = useReaderState(kind, namespace, name, path);
} = useReaderState(kind, namespace, name, useParams()['*']);
const techdocsStorageApi = useApi(techdocsStorageApiRef);
const [sidebars, setSidebars] = useState<HTMLElement[]>();
@@ -322,6 +322,10 @@ export const Reader = ({ entityId, onReady }: Props) => {
useEffect(() => {
if (!rawPage || !shadowDomRef.current) {
// clear the shadow dom if no content is available
if (shadowDomRef.current?.shadowRoot) {
shadowDomRef.current.shadowRoot.innerHTML = '';
}
return () => {};
}
if (onReady) {
plugins/techdocs/src/reader/components/useReaderState.test.tsx
+173 -26
View File
@@ -86,7 +86,7 @@ describe('useReaderState', () => {
};
it('should return a copy of the state', () => {
expect(reducer(oldState, { type: 'navigate', path: '/' })).toEqual({
expect(reducer(oldState, { type: 'content', path: '/' })).toEqual({
activeSyncState: 'CHECKING',
contentLoading: false,
path: '/',
@@ -102,13 +102,13 @@ describe('useReaderState', () => {
});
it.each`
type | oldActiveSyncState | newActiveSyncState
${'content'} | ${'BUILD_READY'} | ${'UP_TO_DATE'}
${'content'} | ${'BUILD_READY_RELOAD'} | ${'UP_TO_DATE'}
${'navigate'} | ${'BUILD_READY'} | ${'UP_TO_DATE'}
${'navigate'} | ${'BUILD_READY_RELOAD'} | ${'UP_TO_DATE'}
${'sync'} | ${'BUILD_READY'} | ${undefined}
${'sync'} | ${'BUILD_READY_RELOAD'} | ${undefined}
type | oldActiveSyncState | newActiveSyncState
${'contentLoading'} | ${'BUILD_READY'} | ${'UP_TO_DATE'}
${'contentLoading'} | ${'BUILD_READY_RELOAD'} | ${'UP_TO_DATE'}
${'content'} | ${'BUILD_READY'} | ${'UP_TO_DATE'}
${'content'} | ${'BUILD_READY_RELOAD'} | ${'UP_TO_DATE'}
${'sync'} | ${'BUILD_READY'} | ${undefined /* undefined, because we don't set an input */}
${'sync'} | ${'BUILD_READY_RELOAD'} | ${undefined /* undefined, because we don't set an input */}
`(
'should, when type=$type and activeSyncState=$oldActiveSyncState, set activeSyncState=$newActiveSyncState',
({ type, oldActiveSyncState, newActiveSyncState }) => {
@@ -124,18 +124,45 @@ describe('useReaderState', () => {
},
);
describe('"content" action', () => {
describe('"contentLoading" action', () => {
it('should set loading', () => {
expect(
reducer(oldState, {
type: 'contentLoading',
}),
).toEqual({
...oldState,
contentLoading: true,
});
});
it('should keep content', () => {
expect(
reducer(
{
...oldState,
content: 'some-old-content',
},
{
type: 'contentLoading',
},
),
).toEqual({
...oldState,
contentLoading: true,
content: 'some-old-content',
});
});
it('should reset errors', () => {
expect(
reducer(
{
...oldState,
contentError: new Error(),
},
{
type: 'content',
contentLoading: true,
type: 'contentLoading',
},
),
).toEqual({
@@ -143,7 +170,9 @@ describe('useReaderState', () => {
contentLoading: true,
});
});
});
describe('"content" action', () => {
it('should set content', () => {
expect(
reducer(
@@ -164,6 +193,27 @@ describe('useReaderState', () => {
});
});
it('should set content and update path', () => {
expect(
reducer(
{
...oldState,
contentLoading: true,
},
{
type: 'content',
content: 'asdf',
path: '/new-path',
},
),
).toEqual({
...oldState,
contentLoading: false,
content: 'asdf',
path: '/new-path',
});
});
it('should set error', () => {
expect(
reducer(
@@ -185,20 +235,6 @@ describe('useReaderState', () => {
});
});
describe('"navigate" action', () => {
it('should work', () => {
expect(
reducer(oldState, {
type: 'navigate',
path: '/',
}),
).toEqual({
...oldState,
path: '/',
});
});
});
describe('"sync" action', () => {
it('should update state', () => {
expect(
@@ -256,6 +292,7 @@ describe('useReaderState', () => {
expect(result.current).toEqual({
state: 'CHECKING',
path: '/example',
content: undefined,
contentErrorMessage: undefined,
syncErrorMessage: undefined,
@@ -267,6 +304,7 @@ describe('useReaderState', () => {
expect(result.current).toEqual({
state: 'CONTENT_FRESH',
path: '/example',
content: 'my content',
contentErrorMessage: undefined,
syncErrorMessage: undefined,
@@ -313,6 +351,7 @@ describe('useReaderState', () => {
expect(result.current).toEqual({
state: 'CHECKING',
path: '/example',
content: undefined,
contentErrorMessage: undefined,
syncErrorMessage: undefined,
@@ -324,6 +363,7 @@ describe('useReaderState', () => {
expect(result.current).toEqual({
state: 'INITIAL_BUILD',
path: '/example',
content: undefined,
contentErrorMessage: 'NotFoundError: Page Not Found',
syncErrorMessage: undefined,
@@ -335,6 +375,7 @@ describe('useReaderState', () => {
expect(result.current).toEqual({
state: 'CHECKING',
path: '/example',
content: undefined,
contentErrorMessage: undefined,
syncErrorMessage: undefined,
@@ -346,6 +387,7 @@ describe('useReaderState', () => {
expect(result.current).toEqual({
state: 'CONTENT_FRESH',
path: '/example',
content: 'my content',
contentErrorMessage: undefined,
syncErrorMessage: undefined,
@@ -394,6 +436,7 @@ describe('useReaderState', () => {
expect(result.current).toEqual({
state: 'CHECKING',
path: '/example',
content: undefined,
contentErrorMessage: undefined,
syncErrorMessage: undefined,
@@ -405,6 +448,7 @@ describe('useReaderState', () => {
await waitForValueToChange(() => result.current.state);
expect(result.current).toEqual({
state: 'CONTENT_FRESH',
path: '/example',
content: 'my content',
contentErrorMessage: undefined,
syncErrorMessage: undefined,
@@ -416,6 +460,7 @@ describe('useReaderState', () => {
await waitForValueToChange(() => result.current.state);
expect(result.current).toEqual({
state: 'CONTENT_STALE_REFRESHING',
path: '/example',
content: 'my content',
contentErrorMessage: undefined,
syncErrorMessage: undefined,
@@ -427,6 +472,7 @@ describe('useReaderState', () => {
await waitForValueToChange(() => result.current.state);
expect(result.current).toEqual({
state: 'CONTENT_STALE_READY',
path: '/example',
content: 'my content',
contentErrorMessage: undefined,
syncErrorMessage: undefined,
@@ -441,7 +487,8 @@ describe('useReaderState', () => {
await waitForValueToChange(() => result.current.state);
expect(result.current).toEqual({
state: 'CHECKING',
content: undefined,
path: '/example',
content: 'my content',
contentErrorMessage: undefined,
syncErrorMessage: undefined,
buildLog: [],
@@ -452,6 +499,7 @@ describe('useReaderState', () => {
await waitForValueToChange(() => result.current.state);
expect(result.current).toEqual({
state: 'CONTENT_FRESH',
path: '/example',
content: 'my new content',
contentErrorMessage: undefined,
syncErrorMessage: undefined,
@@ -475,6 +523,103 @@ describe('useReaderState', () => {
});
});
it('should handle navigation', async () => {
techdocsStorageApi.getEntityDocs
.mockResolvedValueOnce('my content')
.mockImplementationOnce(async () => {
await new Promise(resolve => setTimeout(resolve, 1100));
return 'my new content';
})
.mockRejectedValueOnce(new NotFoundError('Some error description'));
techdocsStorageApi.syncEntityDocs.mockResolvedValue('cached');
await act(async () => {
const { result, waitForValueToChange, rerender } = await renderHook(
({ path }: { path: string }) =>
useReaderState('Component', 'default', 'backstage', path),
{ initialProps: { path: '/example' }, wrapper: Wrapper as any },
);
expect(result.current).toEqual({
state: 'CHECKING',
path: '/example',
content: undefined,
contentErrorMessage: undefined,
syncErrorMessage: undefined,
buildLog: [],
contentReload: expect.any(Function),
});
// show the content
await waitForValueToChange(() => result.current.state);
expect(result.current).toEqual({
state: 'CONTENT_FRESH',
path: '/example',
content: 'my content',
contentErrorMessage: undefined,
syncErrorMessage: undefined,
buildLog: [],
contentReload: expect.any(Function),
});
// navigate
rerender({ path: '/new' });
await waitForValueToChange(() => result.current.state);
expect(result.current).toEqual({
state: 'CHECKING',
path: '/example',
content: 'my content',
contentErrorMessage: undefined,
syncErrorMessage: undefined,
buildLog: [],
contentReload: expect.any(Function),
});
await waitForValueToChange(() => result.current.state);
expect(result.current).toEqual({
state: 'CONTENT_FRESH',
path: '/new',
content: 'my new content',
contentErrorMessage: undefined,
syncErrorMessage: undefined,
buildLog: [],
contentReload: expect.any(Function),
});
// navigate
rerender({ path: '/missing' });
await waitForValueToChange(() => result.current.state);
expect(result.current).toEqual({
state: 'CONTENT_NOT_FOUND',
path: '/missing',
content: undefined,
contentErrorMessage: 'NotFoundError: Some error description',
syncErrorMessage: undefined,
buildLog: [],
contentReload: expect.any(Function),
});
expect(techdocsStorageApi.getEntityDocs).toBeCalledWith(
{ kind: 'Component', namespace: 'default', name: 'backstage' },
'/example',
);
expect(techdocsStorageApi.getEntityDocs).toBeCalledWith(
{ kind: 'Component', namespace: 'default', name: 'backstage' },
'/new',
);
expect(techdocsStorageApi.syncEntityDocs).toBeCalledWith(
{
kind: 'Component',
namespace: 'default',
name: 'backstage',
},
expect.any(Function),
);
});
});
it('should handle content error', async () => {
techdocsStorageApi.getEntityDocs.mockRejectedValue(
new NotFoundError('Some error description'),
@@ -489,6 +634,7 @@ describe('useReaderState', () => {
expect(result.current).toEqual({
state: 'CHECKING',
path: '/example',
content: undefined,
contentErrorMessage: undefined,
syncErrorMessage: undefined,
@@ -500,6 +646,7 @@ describe('useReaderState', () => {
await waitForValueToChange(() => result.current.state);
expect(result.current).toEqual({
state: 'CONTENT_NOT_FOUND',
path: '/example',
content: undefined,
contentErrorMessage: 'NotFoundError: Some error description',
syncErrorMessage: undefined,
plugins/techdocs/src/reader/components/useReaderState.ts
+25 -19
View File
@@ -15,7 +15,7 @@
*/
import { useApi } from '@backstage/core-plugin-api';
import { useEffect, useMemo, useReducer, useRef } from 'react';
import { useMemo, useReducer, useRef } from 'react';
import { useAsync, useAsyncRetry } from 'react-use';
import { techdocsStorageApiRef } from '../../api';
@@ -131,13 +131,13 @@ type ReducerActions =
state: SyncStates;
syncError?: Error;
}
| { type: 'contentLoading' }
| {
type: 'content';
path?: string;
content?: string;
contentLoading?: true;
contentError?: Error;
}
| { type: 'navigate'; path: string }
| { type: 'buildLog'; log: string };
type ReducerState = {
@@ -186,14 +186,22 @@ export function reducer(
newState.syncError = action.syncError;
break;
case 'content':
newState.content = action.content;
newState.contentLoading = action.contentLoading ?? false;
newState.contentError = action.contentError;
case 'contentLoading':
newState.contentLoading = true;
// only reset errors but keep the old content until it is replaced by the 'content' action
newState.contentError = undefined;
break;
case 'navigate':
newState.path = action.path;
case 'content':
// only override the path if it is part of the action
if (typeof action.path === 'string') {
newState.path = action.path;
}
newState.contentLoading = false;
newState.content = action.content;
newState.contentError = action.contentError;
break;
case 'buildLog':
@@ -204,10 +212,10 @@ export function reducer(
throw new Error();
}
// a navigation or a content update loads fresh content so the build is updated to being up-to-date
// a content update loads fresh content so the build is updated to being up-to-date
if (
['BUILD_READY', 'BUILD_READY_RELOAD'].includes(newState.activeSyncState) &&
['content', 'navigate'].includes(action.type)
['contentLoading', 'content'].includes(action.type)
) {
newState.activeSyncState = 'UP_TO_DATE';
newState.buildLog = [];
@@ -223,6 +231,7 @@ export function useReaderState(
path: string,
): {
state: ContentStateTypes;
path: string;
contentReload: () => void;
content?: string;
contentErrorMessage?: string;
@@ -238,14 +247,9 @@ export function useReaderState(
const techdocsStorageApi = useApi(techdocsStorageApiRef);
// convert all path changes into actions
useEffect(() => {
dispatch({ type: 'navigate', path });
}, [path]);
// try to load the content. the function will fire events and we don't care for the return values
const { retry: contentReload } = useAsyncRetry(async () => {
dispatch({ type: 'content', contentLoading: true });
dispatch({ type: 'contentLoading' });
try {
const entityDocs = await techdocsStorageApi.getEntityDocs(
@@ -253,11 +257,12 @@ export function useReaderState(
path,
);
dispatch({ type: 'content', content: entityDocs });
// update content and path at the same time
dispatch({ type: 'content', content: entityDocs, path });
return entityDocs;
} catch (e) {
dispatch({ type: 'content', contentError: e });
dispatch({ type: 'content', contentError: e, path });
}
return undefined;
@@ -335,6 +340,7 @@ export function useReaderState(
return {
state: displayState,
contentReload,
path: state.path,
content: state.content,
contentErrorMessage: state.contentError?.toString(),
syncErrorMessage: state.syncError?.toString(),