Implement api for extension point typings (#1638)

Currently, the only way to explore available extension points is through our documentation or by browsing the source code. Once you find them, there is no guard rails and the usage is prone to user errors. This new api allows the declaration of extension points as types in code. This way, exposing an extension point is as easy as exporting it from a module. Both the implementation and the developer who uses the extension point work with the same shared type that allows auto-completion and type-checks for safety. This feature is backwards-compatible as the generic methods all have sensible defaults for the type parameters.

Co-authored-by: Sebastian Sdorra <sebastian.sdorra@cloudogu.com>
Co-authored-by: Eduard Heimbuch <eduard.heimbuch@cloudogu.com>
This commit is contained in:
Konstantin Schaper
2021-05-12 16:05:30 +02:00
committed by GitHub
parent b6b304f338
commit 7286a62a80
18 changed files with 5335 additions and 7317 deletions

View File

@@ -107,3 +107,37 @@ binder.bind("repo.avatar", GitAvatar, (props) => props.type === "git");
```javascript ```javascript
<ExtensionPoint name="repo.avatar" props={type: "git"} /> <ExtensionPoint name="repo.avatar" props={type: "git"} />
``` ```
### Typings
Both extension points and extensions can share a common typescript type to define the contract between them.
This includes the `name`, the type of `props` passed to the predicate and what `type` the extensions themselves can be.
Example:
```typescript
type CalculatorExtensionPoint = ExtensionPointDefinition<"extension.calculator", (input: number[]) => number, undefined>;
const sum = (a: number, b: number) => a + b;
binder.bind<CalculatorExtensionPoint>("extension.calculator", (input: number[]) => input.reduce(sum, 0));
const calculator = binder.getExtension<CalculatorExtensionPoint>("extension.calculator");
const result = calculator([1, 2, 3]);
```
In this example, we use the base type `ExtensionPointDefinition<name, type, props>` to declare a new extension point.
As we do not need a predicate, we can define the `props` type parameter as `undefined`. This allows us to skip the `props` parameter in the
`getExtension` method and the `predicate` parameter in the `bind` method.
When using `bind` to define an extension or `getExtension` to retrieve an extension, we can pass the new type as a type parameter.
By doing this, we allow typescript to help us with type-checks and offer us type-completion.
Negative Example:
```typescript
type CalculatorExtensionPoint = ExtensionPointDefinition<"extension.calculator", (input: number[]) => number, undefined>;
const sum = (a: number, b: number) => a + b;
binder.bind<CalculatorExtensionPoint>("extension.calculato", (input: number[]) => input.reduce(sum, 0));
```
This code for example, would lead to a compile time type error because we made a typo in the `name` of the extension when binding it.
If we had used the `bind` method without the type parameter, we would not have gotten an error but run into problems at runtime.

View File

@@ -0,0 +1,2 @@
- type: added
description: Implement api for extension point typings ([#1638](https://github.com/scm-manager/scm-manager/pull/1638))

View File

@@ -14,7 +14,7 @@
"@scm-manager/ui-plugins": "^2.18.1-SNAPSHOT" "@scm-manager/ui-plugins": "^2.18.1-SNAPSHOT"
}, },
"devDependencies": { "devDependencies": {
"@scm-manager/babel-preset": "^2.11.1", "@scm-manager/babel-preset": "^2.12.0",
"@scm-manager/eslint-config": "^2.11.1", "@scm-manager/eslint-config": "^2.11.1",
"@scm-manager/jest-preset": "^2.12.7", "@scm-manager/jest-preset": "^2.12.7",
"@scm-manager/plugin-scripts": "^1.0.1", "@scm-manager/plugin-scripts": "^1.0.1",

View File

@@ -13,7 +13,7 @@
"@scm-manager/ui-plugins": "^2.18.1-SNAPSHOT" "@scm-manager/ui-plugins": "^2.18.1-SNAPSHOT"
}, },
"devDependencies": { "devDependencies": {
"@scm-manager/babel-preset": "^2.11.1", "@scm-manager/babel-preset": "^2.12.0",
"@scm-manager/eslint-config": "^2.11.1", "@scm-manager/eslint-config": "^2.11.1",
"@scm-manager/jest-preset": "^2.12.7", "@scm-manager/jest-preset": "^2.12.7",
"@scm-manager/plugin-scripts": "^1.0.1", "@scm-manager/plugin-scripts": "^1.0.1",

View File

@@ -13,7 +13,7 @@
"@scm-manager/ui-plugins": "^2.18.1-SNAPSHOT" "@scm-manager/ui-plugins": "^2.18.1-SNAPSHOT"
}, },
"devDependencies": { "devDependencies": {
"@scm-manager/babel-preset": "^2.11.1", "@scm-manager/babel-preset": "^2.12.0",
"@scm-manager/eslint-config": "^2.11.1", "@scm-manager/eslint-config": "^2.11.1",
"@scm-manager/jest-preset": "^2.12.7", "@scm-manager/jest-preset": "^2.12.7",
"@scm-manager/plugin-scripts": "^1.0.1", "@scm-manager/plugin-scripts": "^1.0.1",

View File

@@ -13,7 +13,7 @@
"@scm-manager/ui-plugins": "^2.18.1-SNAPSHOT" "@scm-manager/ui-plugins": "^2.18.1-SNAPSHOT"
}, },
"devDependencies": { "devDependencies": {
"@scm-manager/babel-preset": "^2.11.1", "@scm-manager/babel-preset": "^2.12.0",
"@scm-manager/eslint-config": "^2.11.1", "@scm-manager/eslint-config": "^2.11.1",
"@scm-manager/jest-preset": "^2.12.7", "@scm-manager/jest-preset": "^2.12.7",
"@scm-manager/plugin-scripts": "^1.0.1", "@scm-manager/plugin-scripts": "^1.0.1",

View File

@@ -15,7 +15,7 @@
"typecheck": "tsc" "typecheck": "tsc"
}, },
"devDependencies": { "devDependencies": {
"@scm-manager/babel-preset": "^2.11.2", "@scm-manager/babel-preset": "^2.12.0",
"@scm-manager/eslint-config": "^2.10.1", "@scm-manager/eslint-config": "^2.10.1",
"@scm-manager/jest-preset": "^2.12.7", "@scm-manager/jest-preset": "^2.12.7",
"@scm-manager/prettier-config": "^2.10.1", "@scm-manager/prettier-config": "^2.10.1",

View File

@@ -284,7 +284,7 @@ const EXPORT_MEDIA_TYPE = "application/vnd.scmm-repositoryExport+json;v=2";
export const useExportRepository = () => { export const useExportRepository = () => {
const queryClient = useQueryClient(); const queryClient = useQueryClient();
const [intervalId, setIntervalId] = useState<number | undefined>(); const [intervalId, setIntervalId] = useState<ReturnType<typeof setTimeout>>();
useEffect(() => { useEffect(() => {
return () => { return () => {
if (intervalId) { if (intervalId) {

View File

@@ -20,7 +20,7 @@
"update-storyshots": "jest --testPathPattern=\"storyshots.test.ts\" --collectCoverage=false -u" "update-storyshots": "jest --testPathPattern=\"storyshots.test.ts\" --collectCoverage=false -u"
}, },
"devDependencies": { "devDependencies": {
"@scm-manager/babel-preset": "^2.11.2", "@scm-manager/babel-preset": "^2.12.0",
"@scm-manager/eslint-config": "^2.12.0", "@scm-manager/eslint-config": "^2.12.0",
"@scm-manager/jest-preset": "^2.12.7", "@scm-manager/jest-preset": "^2.12.7",
"@scm-manager/prettier-config": "^2.10.1", "@scm-manager/prettier-config": "^2.10.1",

File diff suppressed because it is too large Load Diff

View File

@@ -42,11 +42,14 @@ const FixedHeightInput = styled.input`
const FilterInput: FC<Props> = ({ filter, value, testId, placeholder, autoFocus, className }) => { const FilterInput: FC<Props> = ({ filter, value, testId, placeholder, autoFocus, className }) => {
const [stateValue, setStateValue] = useState(value || ""); const [stateValue, setStateValue] = useState(value || "");
const [timeoutId, setTimeoutId] = useState(0); const [timeoutId, setTimeoutId] = useState<ReturnType<typeof setTimeout>>();
const [t] = useTranslation("commons"); const [t] = useTranslation("commons");
// TODO check dependencies
useEffect(() => { useEffect(() => {
if (timeoutId) {
clearTimeout(timeoutId); clearTimeout(timeoutId);
}
if (!stateValue) { if (!stateValue) {
// no delay if filter input was deleted // no delay if filter input was deleted
filter(stateValue); filter(stateValue);

View File

@@ -13,7 +13,7 @@
"react": "^17.0.1" "react": "^17.0.1"
}, },
"devDependencies": { "devDependencies": {
"@scm-manager/babel-preset": "^2.11.2", "@scm-manager/babel-preset": "^2.12.0",
"@scm-manager/eslint-config": "^2.12.0", "@scm-manager/eslint-config": "^2.12.0",
"@scm-manager/jest-preset": "^2.12.7", "@scm-manager/jest-preset": "^2.12.7",
"@scm-manager/prettier-config": "^2.10.1", "@scm-manager/prettier-config": "^2.10.1",

View File

@@ -22,7 +22,7 @@
* SOFTWARE. * SOFTWARE.
*/ */
import { Binder } from "./binder"; import { Binder, ExtensionPointDefinition, SimpleDynamicExtensionPointDefinition } from "./binder";
describe("binder tests", () => { describe("binder tests", () => {
let binder: Binder; let binder: Binder;
@@ -99,4 +99,48 @@ describe("binder tests", () => {
expect(extensions[0]).toEqual("planetD"); expect(extensions[0]).toEqual("planetD");
expect(extensions[1]).toEqual("planetB"); expect(extensions[1]).toEqual("planetB");
}); });
it("should allow typings for extension points but still be backwards-compatible", () => {
type TestExtensionPointA = ExtensionPointDefinition<"test.extension.a", number, undefined>;
type TestExtensionPointB = ExtensionPointDefinition<"test.extension.b", number, { testProp: boolean[] }>;
binder.bind<TestExtensionPointA>("test.extension.a", 2, () => false);
const binderExtensionA = binder.getExtension<TestExtensionPointA>("test.extension.a");
expect(binderExtensionA).not.toBeNull();
binder.bind<TestExtensionPointB>("test.extension.b", 2);
const binderExtensionsB = binder.getExtensions<TestExtensionPointB>("test.extension.b", {
testProp: [true, false]
});
expect(binderExtensionsB).toHaveLength(1);
binder.bind("test.extension.c", 2, () => false);
const binderExtensionC = binder.getExtension("test.extension.c");
expect(binderExtensionC).not.toBeNull();
});
it("should allow typings for dynamic extension points", () => {
type MarkdownCodeLanguageRendererProps = {
language?: string;
value: string;
};
type MarkdownCodeLanguageRendererExtensionPoint<
S extends string | undefined = undefined
> = SimpleDynamicExtensionPointDefinition<
"markdown-renderer.code.",
(props: any) => any,
MarkdownCodeLanguageRendererProps,
S
>;
type UmlExtensionPoint = MarkdownCodeLanguageRendererExtensionPoint<"uml">;
binder.bind<UmlExtensionPoint>("markdown-renderer.code.uml", props => props.value);
const language = "uml";
const extensionPointName = `markdown-renderer.code.${language}` as const;
const dynamicExtension = binder.getExtension<MarkdownCodeLanguageRendererExtensionPoint>(extensionPointName, {
language: "uml",
value: "const a = 2;"
});
expect(dynamicExtension).not.toBeNull();
});
}); });

View File

@@ -22,14 +22,22 @@
* SOFTWARE. * SOFTWARE.
*/ */
type Predicate = (props: any) => boolean; type Predicate<P extends Record<any, any> = Record<any, any>> = (props: P) => boolean;
type ExtensionRegistration = { type ExtensionRegistration<P, T> = {
predicate: Predicate; predicate: Predicate<P>;
extension: any; extension: T;
extensionName: string; extensionName: string;
}; };
export type ExtensionPointDefinition<N extends string, T, P> = {
name: N;
type: T;
props: P;
};
export type SimpleDynamicExtensionPointDefinition<P extends string, T, Props, S extends string | undefined> = ExtensionPointDefinition<S extends string ? `${P}${S}` : `${P}${string}`, T, Props>;
/** /**
* Binder is responsible for binding plugin extensions to their corresponding extension points. * Binder is responsible for binding plugin extensions to their corresponding extension points.
* The Binder class is mainly exported for testing, plugins should only use the default export. * The Binder class is mainly exported for testing, plugins should only use the default export.
@@ -37,7 +45,7 @@ type ExtensionRegistration = {
export class Binder { export class Binder {
name: string; name: string;
extensionPoints: { extensionPoints: {
[key: string]: Array<ExtensionRegistration>; [key: string]: Array<ExtensionRegistration<unknown, unknown>>;
}; };
constructor(name: string) { constructor(name: string) {
@@ -52,7 +60,22 @@ export class Binder {
* @param extension provided extension * @param extension provided extension
* @param predicate to decide if the extension gets rendered for the given props * @param predicate to decide if the extension gets rendered for the given props
*/ */
bind(extensionPoint: string, extension: any, predicate?: Predicate, extensionName?: string) { bind<E extends ExtensionPointDefinition<string, unknown, undefined>>(
extensionPoint: E["name"],
extension: E["type"]
): void;
bind<E extends ExtensionPointDefinition<string, unknown, any>>(
extensionPoint: E["name"],
extension: E["type"],
predicate?: Predicate<E["props"]>,
extensionName?: string
): void;
bind<E extends ExtensionPointDefinition<string, unknown, any>>(
extensionPoint: E["name"],
extension: E["type"],
predicate?: Predicate<E["props"]>,
extensionName?: string
) {
if (!this.extensionPoints[extensionPoint]) { if (!this.extensionPoints[extensionPoint]) {
this.extensionPoints[extensionPoint] = []; this.extensionPoints[extensionPoint] = [];
} }
@@ -70,7 +93,15 @@ export class Binder {
* @param extensionPoint name of extension point * @param extensionPoint name of extension point
* @param props of the extension point * @param props of the extension point
*/ */
getExtension(extensionPoint: string, props?: object) { getExtension<E extends ExtensionPointDefinition<string, any, undefined>>(extensionPoint: E["name"]): E["type"] | null;
getExtension<E extends ExtensionPointDefinition<string, any, any>>(
extensionPoint: E["name"],
props: E["props"]
): E["type"] | null;
getExtension<E extends ExtensionPointDefinition<any, unknown, any>>(
extensionPoint: E["name"],
props?: E["props"]
): E["type"] | null {
const extensions = this.getExtensions(extensionPoint, props); const extensions = this.getExtensions(extensionPoint, props);
if (extensions.length > 0) { if (extensions.length > 0) {
return extensions[0]; return extensions[0];
@@ -84,10 +115,20 @@ export class Binder {
* @param extensionPoint name of extension point * @param extensionPoint name of extension point
* @param props of the extension point * @param props of the extension point
*/ */
getExtensions(extensionPoint: string, props?: object): Array<any> { getExtensions<E extends ExtensionPointDefinition<string, any, undefined>>(
extensionPoint: E["name"]
): Array<E["type"]>;
getExtensions<E extends ExtensionPointDefinition<string, any, any>>(
extensionPoint: E["name"],
props: E["props"]
): Array<E["type"]>;
getExtensions<E extends ExtensionPointDefinition<string, unknown, any>>(
extensionPoint: E["name"],
props?: E["props"]
): Array<E["type"]> {
let registrations = this.extensionPoints[extensionPoint] || []; let registrations = this.extensionPoints[extensionPoint] || [];
if (props) { if (props) {
registrations = registrations.filter(reg => reg.predicate(props || {})); registrations = registrations.filter(reg => reg.predicate(props));
} }
registrations.sort(this.sortExtensions); registrations.sort(this.sortExtensions);
return registrations.map(reg => reg.extension); return registrations.map(reg => reg.extension);
@@ -96,14 +137,17 @@ export class Binder {
/** /**
* Returns true if at least one extension is bound to the extension point and its props. * Returns true if at least one extension is bound to the extension point and its props.
*/ */
hasExtension(extensionPoint: string, props?: object): boolean { hasExtension<E extends ExtensionPointDefinition<any, unknown, any>>(
return this.getExtensions(extensionPoint, props).length > 0; extensionPoint: E["name"],
props?: E["props"]
): boolean {
return this.getExtensions<E>(extensionPoint, props).length > 0;
} }
/** /**
* Sort extensions in ascending order, starting with entries with specified extensionName. * Sort extensions in ascending order, starting with entries with specified extensionName.
*/ */
sortExtensions = (a: ExtensionRegistration, b: ExtensionRegistration) => { sortExtensions = (a: ExtensionRegistration<unknown, unknown>, b: ExtensionRegistration<unknown, unknown>) => {
const regA = a.extensionName ? a.extensionName.toUpperCase() : ""; const regA = a.extensionName ? a.extensionName.toUpperCase() : "";
const regB = b.extensionName ? b.extensionName.toUpperCase() : ""; const regB = b.extensionName ? b.extensionName.toUpperCase() : "";

View File

@@ -22,6 +22,6 @@
* SOFTWARE. * SOFTWARE.
*/ */
export { default as binder, Binder } from "./binder"; export { default as binder, Binder, ExtensionPointDefinition } from "./binder";
export * from "./useBinder"; export * from "./useBinder";
export { default as ExtensionPoint } from "./ExtensionPoint"; export { default as ExtensionPoint } from "./ExtensionPoint";

View File

@@ -18,7 +18,7 @@
"styled-components": "^5.1.0" "styled-components": "^5.1.0"
}, },
"devDependencies": { "devDependencies": {
"@scm-manager/babel-preset": "^2.11.2", "@scm-manager/babel-preset": "^2.12.0",
"@scm-manager/eslint-config": "^2.12.0", "@scm-manager/eslint-config": "^2.12.0",
"@scm-manager/jest-preset": "^2.12.7", "@scm-manager/jest-preset": "^2.12.7",
"@scm-manager/prettier-config": "^2.10.1", "@scm-manager/prettier-config": "^2.10.1",

View File

@@ -9,8 +9,8 @@
"typecheck": "tsc" "typecheck": "tsc"
}, },
"dependencies": { "dependencies": {
"@wojtekmaj/enzyme-adapter-react-17": "^0.5.0", "@wojtekmaj/enzyme-adapter-react-17": "^0.6.0",
"babel-plugin-istanbul": "^5.2.0", "babel-plugin-istanbul": "^6.0.0",
"enzyme": "^3.10.0", "enzyme": "^3.10.0",
"enzyme-context": "^1.1.2", "enzyme-context": "^1.1.2",
"enzyme-context-react-router-4": "^2.0.0", "enzyme-context-react-router-4": "^2.0.0",

8586
yarn.lock

File diff suppressed because it is too large Load Diff