chore: code review comments

Signed-off-by: blam <ben@blam.sh>
This commit is contained in:
blam
2024-08-14 15:37:09 +02:00
parent 9eec064aaf
commit f1e9c08f97
9 changed files with 95 additions and 37 deletions
@@ -77,7 +77,7 @@ describe('NavItemBlueprint', () => {
const tester = createExtensionTester(extension);
expect(tester.data(NavItemBlueprint.dataRefs.target)).toEqual({
expect(tester.get(NavItemBlueprint.dataRefs.target)).toEqual({
title: 'TEST',
icon: MockIcon,
routeRef: mockRouteRef,
@@ -97,7 +97,7 @@ describe('NavItemBlueprint', () => {
config: { title: 'OVERRIDDEN' },
});
expect(tester.data(NavItemBlueprint.dataRefs.target)).toEqual({
expect(tester.get(NavItemBlueprint.dataRefs.target)).toEqual({
title: 'OVERRIDDEN',
icon: MockIcon,
routeRef: mockRouteRef,
@@ -64,7 +64,7 @@ describe('NavLogoBlueprint', () => {
const tester = createExtensionTester(extension);
expect(tester.data(NavLogoBlueprint.dataRefs.logoElements)).toEqual({
expect(tester.get(NavLogoBlueprint.dataRefs.logoElements)).toEqual({
logoFull,
logoIcon,
});
@@ -101,7 +101,7 @@ describe('PageBlueprint', () => {
// TODO(blam): test for the routePath output doesn't work, due to the the way the test harness works
// expect(tester.data(coreExtensionData.routePath)).toBe('/test');
expect(tester.data(coreExtensionData.routeRef)).toBe(mockRouteRef);
expect(tester.get(coreExtensionData.routeRef)).toBe(mockRouteRef);
const { getByTestId } = tester.render();
@@ -147,7 +147,7 @@ describe('PageBlueprint', () => {
CardBlueprint.make({ name: 'card', params: {} }),
);
const { getByTestId, getByText } = renderInTestApp(tester.element());
const { getByTestId, getByText } = renderInTestApp(tester.reactElement());
await waitFor(() => expect(getByTestId('card')).toBeInTheDocument());
await waitFor(() =>
@@ -62,7 +62,7 @@ describe('SignInPageBlueprint', () => {
const tester = createExtensionTester(extension);
const Element = tester.data(SignInPageBlueprint.dataRefs.component)!;
const Element = tester.get(SignInPageBlueprint.dataRefs.component)!;
expect(Element).toBeDefined();
@@ -56,7 +56,7 @@ describe('ThemeBlueprint', () => {
const extension = ThemeBlueprint.make({ params: { theme } });
expect(
createExtensionTester(extension).data(ThemeBlueprint.dataRefs.theme),
createExtensionTester(extension).get(ThemeBlueprint.dataRefs.theme),
).toEqual(theme);
});
});
@@ -76,7 +76,7 @@ describe('TranslationBlueprint', () => {
});
expect(
createExtensionTester(extension).data(
createExtensionTester(extension).get(
TranslationBlueprint.dataRefs.translation,
),
).toBe(messages);
@@ -688,7 +688,7 @@ describe('createExtension', () => {
const tester = createExtensionTester(overridden);
expect(tester.data(numberDataRef)).toBe(43);
expect(tester.get(numberDataRef)).toBe(43);
});
it('should work functionally with overrides', () => {
@@ -722,14 +722,14 @@ describe('createExtension', () => {
},
});
expect(createExtensionTester(overriden).data(stringDataRef)).toBe(
expect(createExtensionTester(overriden).get(stringDataRef)).toBe(
'foo-boom-override-hello',
);
expect(
createExtensionTester(overriden, {
config: { foo: 'hello', bar: 'world' },
}).data(stringDataRef),
}).get(stringDataRef),
).toBe('foo-hello-override-world');
});
@@ -809,7 +809,7 @@ describe('createExtension', () => {
.add(singleExt)
.add(multi1Ext)
.add(multi2Ext)
.data(outputRef),
.get(outputRef),
).toEqual({
opt: 'orig-opt',
single: 'orig-single',
@@ -836,7 +836,7 @@ describe('createExtension', () => {
.add(singleExt)
.add(multi1Ext)
.add(multi2Ext)
.data(outputRef),
.get(outputRef),
).toEqual({
opt: 'opt',
single: 'single',
@@ -862,7 +862,7 @@ describe('createExtension', () => {
.add(singleExt)
.add(multi1Ext)
.add(multi2Ext)
.data(outputRef),
.get(outputRef),
).toEqual({
opt: 'none',
single: 'single',
@@ -885,7 +885,7 @@ describe('createExtension', () => {
.add(singleExt)
.add(multi1Ext)
.add(multi2Ext)
.data(outputRef),
.get(outputRef),
).toEqual({
opt: 'orig-opt',
single: 'orig-single',
@@ -912,7 +912,7 @@ describe('createExtension', () => {
.add(singleExt)
.add(multi1Ext)
.add(multi2Ext)
.data(outputRef),
.get(outputRef),
).toEqual({
opt: 'orig-opt',
single: 'orig-single',
@@ -939,7 +939,7 @@ describe('createExtension', () => {
.add(singleExt)
.add(multi1Ext)
.add(multi2Ext)
.data(outputRef),
.get(outputRef),
).toEqual({
opt: 'orig-opt',
single: 'orig-single',
@@ -966,7 +966,7 @@ describe('createExtension', () => {
.add(singleExt)
.add(multi1Ext)
.add(multi2Ext)
.data(outputRef),
.get(outputRef),
).toEqual({
opt: 'orig-opt',
single: 'orig-single',
@@ -997,7 +997,7 @@ describe('createExtension', () => {
.add(singleExt)
.add(multi1Ext)
.add(multi2Ext)
.data(outputRef),
.get(outputRef),
).toEqual({
opt: 'none',
single: 'override-orig-single',
@@ -1023,7 +1023,7 @@ describe('createExtension', () => {
.add(singleExt)
.add(multi1Ext)
.add(multi2Ext)
.data(outputRef),
.get(outputRef),
).toThrowErrorMatchingInlineSnapshot(
`"Failed to instantiate extension 'subject', override data provided for input 'multi' must match the length of the original inputs"`,
);
@@ -1046,7 +1046,7 @@ describe('createExtension', () => {
.add(singleExt)
.add(multi1Ext)
.add(multi2Ext)
.data(outputRef),
.get(outputRef),
).toThrowErrorMatchingInlineSnapshot(
`"Failed to instantiate extension 'subject', override data for input 'multi' may not mix forwarded inputs with data overrides"`,
);
@@ -1069,7 +1069,7 @@ describe('createExtension', () => {
.add(singleExt)
.add(multi1Ext)
.add(multi2Ext)
.data(outputRef),
.get(outputRef),
).toThrowErrorMatchingInlineSnapshot(
`"Failed to instantiate extension 'subject', missing required extension data value(s) 'test1'"`,
);
@@ -1098,7 +1098,7 @@ describe('createExtension', () => {
.add(singleExt)
.add(multi1Ext)
.add(multi2Ext)
.data(outputRef),
.get(outputRef),
).toThrowErrorMatchingInlineSnapshot(
`"Failed to instantiate extension 'subject', extension data 'test2' was provided but not declared"`,
);
@@ -246,7 +246,7 @@ describe('createExtensionTester', () => {
const tester = createExtensionTester(extension);
expect(tester.data(stringDataRef)).toBe('test-text');
expect(tester.get(stringDataRef)).toBe('test-text');
});
it('should throw an error if trying to access an instance not provided to the tester', () => {
@@ -328,8 +328,8 @@ describe('createExtensionTester', () => {
const tester = createExtensionTester(extension).add(extension2);
expect(tester.query(extension).data(stringDataRef)).toBe('nest-test-text');
expect(tester.query(extension2).data(stringDataRef)).toBe('test-text');
expect(tester.query(extension).get(stringDataRef)).toBe('nest-test-text');
expect(tester.query(extension2).get(stringDataRef)).toBe('test-text');
// @ts-expect-error
expect(tester.query(extension).input('input').data(stringDataRef)).toBe(
'nest-test-text',
@@ -362,6 +362,6 @@ describe('createExtensionTester', () => {
inputs: { input: 'test-text' },
});
expect(tester.query(extension).data(stringDataRef)).toBe('nest-test-text');
expect(tester.query(extension).get(stringDataRef)).toBe('nest-test-text');
});
});
@@ -15,7 +15,7 @@
*/
import React from 'react';
import { MemoryRouter } from 'react-router-dom';
import { MemoryRouter, Link } from 'react-router-dom';
import { RenderResult, render } from '@testing-library/react';
import { createSpecializedApp } from '@backstage/frontend-app-api';
import {
@@ -24,10 +24,15 @@ import {
Extension,
ExtensionDataRef,
ExtensionDefinition,
IconComponent,
RouteRef,
coreExtensionData,
createExtension,
createExtensionInput,
createExtensionOverrides,
createNavItemExtension,
createRouterExtension,
useRouteRef,
} from '@backstage/frontend-plugin-api';
import { Config, ConfigReader } from '@backstage/config';
import { JsonArray, JsonObject, JsonValue } from '@backstage/types';
@@ -43,7 +48,57 @@ import { resolveAppNodeSpecs } from '../../../frontend-app-api/src/tree/resolveA
import { instantiateAppNodeTree } from '../../../frontend-app-api/src/tree/instantiateAppNodeTree';
// eslint-disable-next-line @backstage/no-relative-monorepo-imports
import { readAppExtensionsConfig } from '../../../frontend-app-api/src/tree/readAppExtensionsConfig';
import { TestAppNavExtension } from './renderInTestApp';
const NavItem = (props: {
routeRef: RouteRef<undefined>;
title: string;
icon: IconComponent;
}) => {
const { routeRef, title, icon: Icon } = props;
const link = useRouteRef(routeRef);
if (!link) {
return null;
}
return (
<li>
<Link to={link()}>
<Icon /> {title}
</Link>
</li>
);
};
const TestAppNavExtension = createExtension({
namespace: 'app',
name: 'nav',
attachTo: { id: 'app/layout', input: 'nav' },
inputs: {
items: createExtensionInput({
target: createNavItemExtension.targetDataRef,
}),
},
output: {
element: coreExtensionData.reactElement,
},
factory({ inputs }) {
return {
element: (
<nav>
<ul>
{inputs.items.map((item, index) => (
<NavItem
key={index}
icon={item.output.target.icon}
title={item.output.target.title}
routeRef={item.output.target.routeRef}
/>
))}
</ul>
</nav>
),
};
},
});
/** @public */
export class ExtensionQuery {
@@ -69,7 +124,7 @@ export class ExtensionQuery {
return instance;
}
data<T>(ref: ExtensionDataRef<T>): T | undefined {
get<T>(ref: ExtensionDataRef<T>): T | undefined {
return this.instance.getData(ref);
}
}
@@ -164,10 +219,10 @@ export class ExtensionTester {
return this;
}
data<T>(ref: ExtensionDataRef<T>): T | undefined {
get<T>(ref: ExtensionDataRef<T>): T | undefined {
const tree = this.#resolveTree();
return new ExtensionQuery(tree.root).data(ref);
return new ExtensionQuery(tree.root).get(ref);
}
query(id: string | ExtensionDefinition<any, any>): ExtensionQuery {
@@ -190,16 +245,16 @@ export class ExtensionTester {
return new ExtensionQuery(node);
}
element(): JSX.Element {
reactElement(): JSX.Element {
const tree = this.#resolveTree();
const element = new ExtensionQuery(tree.root).data(
const element = new ExtensionQuery(tree.root).get(
coreExtensionData.reactElement,
);
if (!element) {
throw new Error(
'No element found. Make sure the extension has a `coreExtensionData.reactElement` output, or use the `.get(myComponentDataRef)` method to get the component',
'No element found. Make sure the extension has a `coreExtensionData.reactElement` output, or use the `.get(...)` to access output data directly instead',
);
}
@@ -207,33 +262,36 @@ export class ExtensionTester {
}
/**
* @deprecated Switch to using `renderInTestApp` directly and using `.element()` or `.get(myComponentDataRef)` to get the component you would like to wrap up
* @deprecated Switch to using `renderInTestApp` directly and using `.reactElement()` or `.get(...)` to get the component you w
*/
render(options?: { config?: JsonObject }): RenderResult {
const { config = {} } = options ?? {};
const [subject] = this.#extensions;
if (!subject) {
throw new Error(
'No subject found. At least one extension should be added to the tester.',
);
}
const app = createSpecializedApp({
features: [
createExtensionOverrides({
extensions: [
...this.#extensions.map(extension => extension.definition),
TestAppNavExtension,
createRouterExtension({
namespace: 'test',
Component: ({ children }) => (
<MemoryRouter>{children}</MemoryRouter>
),
}),
TestAppNavExtension,
],
}),
],
config: this.#getConfig(config),
});
return render(app.createRoot());
}