Skip to content

Relaxed requirements for adding new components to Chartlets.js #117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions chartlets.js/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
* Add `multiple` property for `Select` component to enable the selection
of multiple elements. The `default` mode is supported at the moment.

* Relaxed requirements for adding new components to Chartlets.js via
plugins. We no longer require registered components
to implement `ComponentType<ComponentProps>`. `ComponentProps`. (#115)

* Static information about callbacks retrieved from API is not cached
reducing unnecessary processing and improving performance. (#113)

* Relaxed requirements for adding new components to Chartlets.js via
plugins. Implementing `ComponentProps` is now optional. (#115)

* Callbacks will now only be invoked when there’s an actual change in state,
reducing unnecessary processing and improving performance. (#112)

Expand Down
21 changes: 14 additions & 7 deletions chartlets.js/packages/lib/src/actions/configureFramework.test.tsx
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
import type { ComponentType, FC } from "react";
import type { FC } from "react";
import { describe, it, expect, beforeEach, afterEach } from "vitest";
import { configureFramework, resolvePlugin } from "./configureFramework";
import { store } from "@/store";
import { registry } from "@/components/registry";
import type { HostStore } from "@/types/state/host";
import type { Plugin } from "@/types/state/plugin";
import type { ComponentProps } from "@/components/Component";
import type { RegistrableComponent, Plugin } from "@/types/state/plugin";

function getComponents(): [string, ComponentType<ComponentProps>][] {
interface DivProps extends ComponentProps {
function getComponents(): [string, RegistrableComponent][] {
interface DivProps {
text: string;
}
const Div: FC<DivProps> = ({ text }) => <div>{text}</div>;
return [
["A", Div as FC<ComponentProps>],
["B", Div as FC<ComponentProps>],
["A", Div],
["B", Div],
];
}

Expand Down Expand Up @@ -61,6 +60,14 @@
});
expect(registry.types.length).toBe(2);
});

it("should allow adding plain components", () => {
expect(registry.types.length).toBe(0);

Check failure on line 65 in chartlets.js/packages/lib/src/actions/configureFramework.test.tsx

View workflow job for this annotation

GitHub Actions / npm-tests-demo

AssertionError: expected 2 to be +0 // Object.is equality - Expected + Received - 0 + 2 ❯ src/actions/configureFramework.test.tsx:65:35

Check failure on line 65 in chartlets.js/packages/lib/src/actions/configureFramework.test.tsx

View workflow job for this annotation

GitHub Actions / npm-tests-lib (18.x)

AssertionError: expected 2 to be +0 // Object.is equality - Expected + Received - 0 + 2 ❯ src/actions/configureFramework.test.tsx:65:35

Check failure on line 65 in chartlets.js/packages/lib/src/actions/configureFramework.test.tsx

View workflow job for this annotation

GitHub Actions / npm-tests-lib (20.x)

AssertionError: expected 2 to be +0 // Object.is equality - Expected + Received - 0 + 2 ❯ src/actions/configureFramework.test.tsx:65:35
configureFramework({
plugins: [{ components: getComponents() }],
});
expect(registry.types.length).toBe(2);
});
});

describe("resolvePlugin", () => {
Expand Down
5 changes: 2 additions & 3 deletions chartlets.js/packages/lib/src/components/Children.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import type { FC } from "react";
import { describe, it, expect, beforeEach, afterEach } from "vitest";
import { render, screen } from "@testing-library/react";
import { registry } from "@/components/registry";
import { type ComponentProps } from "./Component";
import { Children } from "./Children";

describe("Children", () => {
Expand Down Expand Up @@ -33,11 +32,11 @@ describe("Children", () => {
});

it("should render all child types", () => {
interface DivProps extends ComponentProps {
interface DivProps {
text: string;
}
const Div: FC<DivProps> = ({ text }) => <div>{text}</div>;
registry.register("Div", Div as FC<ComponentProps>);
registry.register("Div", Div);
const divProps = {
type: "Div",
text: "Hello",
Expand Down
8 changes: 6 additions & 2 deletions chartlets.js/packages/lib/src/components/Children.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { type FC } from "react";
import { type ComponentChangeHandler } from "@/types/state/event";
import { type ComponentNode, isComponentState } from "@/types/state/component";
import { Component } from "./Component";
Expand All @@ -7,7 +8,10 @@ export interface ChildrenProps {
onChange: ComponentChangeHandler;
}

export function Children({ nodes, onChange }: ChildrenProps) {
export const Children: FC<ChildrenProps> = ({
nodes,
onChange,
}: ChildrenProps) => {
if (!nodes || nodes.length === 0) {
return null;
}
Expand All @@ -27,4 +31,4 @@ export function Children({ nodes, onChange }: ChildrenProps) {
})}
</>
);
}
};
6 changes: 3 additions & 3 deletions chartlets.js/packages/lib/src/components/Component.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, it, expect, beforeEach, afterEach } from "vitest";
import { render, screen } from "@testing-library/react";
import { Component, type ComponentProps } from "./Component";
import { Component } from "./Component";
import { registry } from "@/components/registry";
import type { FC } from "react";

Expand All @@ -27,11 +27,11 @@ describe("Component", () => {
});

it("should render a known component", () => {
interface DivProps extends ComponentProps {
interface DivProps {
text: string;
}
const Div: FC<DivProps> = ({ text }) => <div>{text}</div>;
registry.register("Div", Div as FC<ComponentProps>);
registry.register("Div", Div);
const divProps = {
type: "Div",
id: "root",
Expand Down
16 changes: 6 additions & 10 deletions chartlets.js/packages/lib/src/components/Component.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
import { type ComponentChangeHandler } from "@/types/state/event";
import { type FC } from "react";
import { isString } from "@/utils/isString";
import { registry } from "@/components/registry";
import type { ComponentProps } from "@/types/state/plugin";

export interface ComponentProps {
type: string;
onChange: ComponentChangeHandler;
}

export function Component(props: ComponentProps) {
const { type: componentType } = props;
const ActualComponent = registry.lookup(componentType);
export const Component: FC<ComponentProps> = (props: ComponentProps) => {
const ActualComponent = isString(props.type) && registry.lookup(props.type);
if (typeof ActualComponent === "function") {
return <ActualComponent {...props} />;
} else {
Expand All @@ -20,4 +16,4 @@ export function Component(props: ComponentProps) {
// );
return null;
}
}
};
27 changes: 10 additions & 17 deletions chartlets.js/packages/lib/src/components/registry.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import type { ComponentType } from "react";

import type { ComponentProps } from "@/components/Component";
import type { RegistrableComponent } from "@/types/state/plugin";

/**
* A registry for Chartlets components.
Expand All @@ -9,8 +7,10 @@ export interface Registry {
/**
* Register a React component that renders a Chartlets component.
*
* `component` must be a functional React component with at
* least the following two component props:
* `component` can be any React component. However, if you want to register
* a custom, reactive component, then `component` must be of type
* `ComponentType<ComponentProps>` where a `ComponentProps` has at
* least the following two properties:
*
* - `type: string`: your component's type name.
* This will be the same as the `type` used for registration.
Expand All @@ -23,14 +23,14 @@ export interface Registry {
* @param type The Chartlets component's unique type name.
* @param component A functional React component.
*/
register(type: string, component: ComponentType<ComponentProps>): () => void;
register(type: string, component: RegistrableComponent): () => void;

/**
* Lookup the component of the provided type.
*
* @param type The Chartlets component's type name.
*/
lookup(type: string): ComponentType<ComponentProps> | undefined;
lookup(type: string): RegistrableComponent | undefined;

/**
* Clears the registry.
Expand All @@ -46,9 +46,9 @@ export interface Registry {

// export for testing only
export class RegistryImpl implements Registry {
private components = new Map<string, ComponentType<ComponentProps>>();
private components = new Map<string, RegistrableComponent>();

register(type: string, component: ComponentType<ComponentProps>): () => void {
register(type: string, component: RegistrableComponent): () => void {
const oldComponent = this.components.get(type);
this.components.set(type, component);
return () => {
Expand All @@ -60,7 +60,7 @@ export class RegistryImpl implements Registry {
};
}

lookup(type: string): ComponentType<ComponentProps> | undefined {
lookup(type: string): RegistrableComponent | undefined {
return this.components.get(type);
}

Expand All @@ -77,12 +77,5 @@ export class RegistryImpl implements Registry {
* The Chartlets component registry.
*
* Use `registry.register("C", C)` to register your own component `C`.
*
* `C` must be a functional React component with at least the following
* two properties:
*
* - `type: string`: your component's type name.
* - `onChange: ComponentChangeHandler`: an event handler
* that your component may call to signal change events.
*/
export const registry = new RegistryImpl();
4 changes: 2 additions & 2 deletions chartlets.js/packages/lib/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export { handleComponentChange } from "@/actions/handleComponentChange";
export { updateContributionContainer } from "@/actions/updateContributionContainer";

// React components
export { Component, type ComponentProps } from "@/components/Component";
export { Component } from "@/components/Component";
export { Children, type ChildrenProps } from "@/components/Children";

// React hooks
Expand All @@ -30,5 +30,5 @@ export {

// Application interface
export type { HostStore, MutableHostStore } from "@/types/state/host";
export type { Plugin, PluginLike } from "@/types/state/plugin";
export type { ComponentProps, Plugin, PluginLike } from "@/types/state/plugin";
export type { FrameworkOptions } from "@/types/state/options";
20 changes: 18 additions & 2 deletions chartlets.js/packages/lib/src/types/state/plugin.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,27 @@
import type { ComponentType } from "react";
import type { ComponentProps } from "@/components/Component";

import type { ComponentChangeHandler } from "@/types/state/event";

/**
* Properties that custom components must support.
*/
export interface ComponentProps {
type: string;
onChange: ComponentChangeHandler;
}

/**
* A component type that is eligible for registration.
*/
export type RegistrableComponent =
| ComponentType<object>
| ComponentType<ComponentProps>;

/**
* A component registration - a pair comprising the component type name
* and the React component.
*/
export type ComponentRegistration = [string, ComponentType<ComponentProps>];
export type ComponentRegistration = [string, RegistrableComponent];

/**
* A framework plugin.
Expand Down
Loading