From ef12fa689ac9a9e009c98b905ee57ff21e0843e5 Mon Sep 17 00:00:00 2001 From: Norman Fomferra Date: Thu, 20 Mar 2025 13:21:49 +0100 Subject: [PATCH 1/3] stopping here --- chartlets.js/CHANGES.md | 6 ++--- .../src/actions/configureFramework.test.tsx | 21 ++++++++++----- .../lib/src/components/Children.test.tsx | 2 +- .../packages/lib/src/components/Children.tsx | 8 ++++-- .../lib/src/components/Component.test.tsx | 2 +- .../packages/lib/src/components/Component.tsx | 9 ++++--- .../packages/lib/src/components/registry.ts | 27 +++++++++---------- .../packages/lib/src/types/state/plugin.ts | 5 +++- 8 files changed, 47 insertions(+), 33 deletions(-) diff --git a/chartlets.js/CHANGES.md b/chartlets.js/CHANGES.md index 50958765..1a0fb70f 100644 --- a/chartlets.js/CHANGES.md +++ b/chartlets.js/CHANGES.md @@ -3,12 +3,12 @@ * 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. Implementing `ComponentProps` is now optional. (#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) diff --git a/chartlets.js/packages/lib/src/actions/configureFramework.test.tsx b/chartlets.js/packages/lib/src/actions/configureFramework.test.tsx index 1dbb6e14..91b5d115 100644 --- a/chartlets.js/packages/lib/src/actions/configureFramework.test.tsx +++ b/chartlets.js/packages/lib/src/actions/configureFramework.test.tsx @@ -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 ComponentRegistration, registry } from "@/components/registry"; import type { HostStore } from "@/types/state/host"; import type { Plugin } from "@/types/state/plugin"; -import type { ComponentProps } from "@/components/Component"; -function getComponents(): [string, ComponentType][] { - interface DivProps extends ComponentProps { +function getComponents(): [string, ComponentRegistration][] { + interface DivProps { text: string; } const Div: FC = ({ text }) =>
{text}
; return [ - ["A", Div as FC], - ["B", Div as FC], + ["A", Div], + ["B", Div], ]; } @@ -61,6 +60,14 @@ describe("configureFramework", () => { }); expect(registry.types.length).toBe(2); }); + + it("should allow adding plain components", () => { + expect(registry.types.length).toBe(0); + configureFramework({ + plugins: [{ components: getComponents() }], + }); + expect(registry.types.length).toBe(2); + }); }); describe("resolvePlugin", () => { diff --git a/chartlets.js/packages/lib/src/components/Children.test.tsx b/chartlets.js/packages/lib/src/components/Children.test.tsx index d494cc14..b1777dcb 100644 --- a/chartlets.js/packages/lib/src/components/Children.test.tsx +++ b/chartlets.js/packages/lib/src/components/Children.test.tsx @@ -37,7 +37,7 @@ describe("Children", () => { text: string; } const Div: FC = ({ text }) =>
{text}
; - registry.register("Div", Div as FC); + registry.register("Div", Div); const divProps = { type: "Div", text: "Hello", diff --git a/chartlets.js/packages/lib/src/components/Children.tsx b/chartlets.js/packages/lib/src/components/Children.tsx index d85cc9bd..aa81e197 100644 --- a/chartlets.js/packages/lib/src/components/Children.tsx +++ b/chartlets.js/packages/lib/src/components/Children.tsx @@ -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"; @@ -7,7 +8,10 @@ export interface ChildrenProps { onChange: ComponentChangeHandler; } -export function Children({ nodes, onChange }: ChildrenProps) { +export const Children: FC = ({ + nodes, + onChange, +}: ChildrenProps) => { if (!nodes || nodes.length === 0) { return null; } @@ -27,4 +31,4 @@ export function Children({ nodes, onChange }: ChildrenProps) { })} ); -} +}; diff --git a/chartlets.js/packages/lib/src/components/Component.test.tsx b/chartlets.js/packages/lib/src/components/Component.test.tsx index cbb1d9df..9b2a4145 100644 --- a/chartlets.js/packages/lib/src/components/Component.test.tsx +++ b/chartlets.js/packages/lib/src/components/Component.test.tsx @@ -31,7 +31,7 @@ describe("Component", () => { text: string; } const Div: FC = ({ text }) =>
{text}
; - registry.register("Div", Div as FC); + registry.register("Div", Div); const divProps = { type: "Div", id: "root", diff --git a/chartlets.js/packages/lib/src/components/Component.tsx b/chartlets.js/packages/lib/src/components/Component.tsx index 238e199d..c5f0956a 100644 --- a/chartlets.js/packages/lib/src/components/Component.tsx +++ b/chartlets.js/packages/lib/src/components/Component.tsx @@ -1,4 +1,6 @@ +import { type FC } from "react"; import { type ComponentChangeHandler } from "@/types/state/event"; +import { isString } from "@/utils/isString"; import { registry } from "@/components/registry"; export interface ComponentProps { @@ -6,9 +8,8 @@ export interface ComponentProps { onChange: ComponentChangeHandler; } -export function Component(props: ComponentProps) { - const { type: componentType } = props; - const ActualComponent = registry.lookup(componentType); +export const Component: FC = (props: ComponentProps) => { + const ActualComponent = isString(props.type) && registry.lookup(props.type); if (typeof ActualComponent === "function") { return ; } else { @@ -20,4 +21,4 @@ export function Component(props: ComponentProps) { // ); return null; } -} +}; diff --git a/chartlets.js/packages/lib/src/components/registry.ts b/chartlets.js/packages/lib/src/components/registry.ts index 9cfc4b95..5d96bd80 100644 --- a/chartlets.js/packages/lib/src/components/registry.ts +++ b/chartlets.js/packages/lib/src/components/registry.ts @@ -2,6 +2,10 @@ import type { ComponentType } from "react"; import type { ComponentProps } from "@/components/Component"; +export type ComponentRegistration = + | ComponentType> + | ComponentType; + /** * A registry for Chartlets components. */ @@ -9,8 +13,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` 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. @@ -23,14 +29,14 @@ export interface Registry { * @param type The Chartlets component's unique type name. * @param component A functional React component. */ - register(type: string, component: ComponentType): () => void; + register(type: string, component: ComponentRegistration): () => void; /** * Lookup the component of the provided type. * * @param type The Chartlets component's type name. */ - lookup(type: string): ComponentType | undefined; + lookup(type: string): ComponentRegistration | undefined; /** * Clears the registry. @@ -46,9 +52,9 @@ export interface Registry { // export for testing only export class RegistryImpl implements Registry { - private components = new Map>(); + private components = new Map(); - register(type: string, component: ComponentType): () => void { + register(type: string, component: ComponentRegistration): () => void { const oldComponent = this.components.get(type); this.components.set(type, component); return () => { @@ -60,7 +66,7 @@ export class RegistryImpl implements Registry { }; } - lookup(type: string): ComponentType | undefined { + lookup(type: string): ComponentRegistration | undefined { return this.components.get(type); } @@ -77,12 +83,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(); diff --git a/chartlets.js/packages/lib/src/types/state/plugin.ts b/chartlets.js/packages/lib/src/types/state/plugin.ts index 967ab4c6..965f9a49 100644 --- a/chartlets.js/packages/lib/src/types/state/plugin.ts +++ b/chartlets.js/packages/lib/src/types/state/plugin.ts @@ -5,7 +5,10 @@ import type { ComponentProps } from "@/components/Component"; * A component registration - a pair comprising the component type name * and the React component. */ -export type ComponentRegistration = [string, ComponentType]; +export type ComponentRegistration = [ + string, + ComponentType | ComponentType, +]; /** * A framework plugin. From c19ec3b7cf5d568b0ea10facd5807dca0933838d Mon Sep 17 00:00:00 2001 From: Norman Fomferra Date: Thu, 20 Mar 2025 13:30:43 +0100 Subject: [PATCH 2/3] update --- chartlets.js/CHANGES.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/chartlets.js/CHANGES.md b/chartlets.js/CHANGES.md index 1a0fb70f..d212f442 100644 --- a/chartlets.js/CHANGES.md +++ b/chartlets.js/CHANGES.md @@ -4,7 +4,8 @@ of multiple elements. The `default` mode is supported at the moment. * Relaxed requirements for adding new components to Chartlets.js via - plugins. Implementing `ComponentProps` is now optional. (#115) + plugins. We no longer require registered components + to implement `ComponentType`. `ComponentProps`. (#115) * Static information about callbacks retrieved from API is not cached reducing unnecessary processing and improving performance. (#113) From e23f4ce130938c959d5145dd3f84477861a218e8 Mon Sep 17 00:00:00 2001 From: Norman Fomferra Date: Thu, 20 Mar 2025 15:55:37 +0100 Subject: [PATCH 3/3] cleanup, but no fix :( --- .../src/actions/configureFramework.test.tsx | 6 ++--- .../lib/src/components/Children.test.tsx | 3 +-- .../lib/src/components/Component.test.tsx | 4 ++-- .../packages/lib/src/components/Component.tsx | 7 +----- .../packages/lib/src/components/registry.ts | 18 +++++---------- chartlets.js/packages/lib/src/index.ts | 4 ++-- .../packages/lib/src/types/state/plugin.ts | 23 +++++++++++++++---- 7 files changed, 33 insertions(+), 32 deletions(-) diff --git a/chartlets.js/packages/lib/src/actions/configureFramework.test.tsx b/chartlets.js/packages/lib/src/actions/configureFramework.test.tsx index 91b5d115..07b0581b 100644 --- a/chartlets.js/packages/lib/src/actions/configureFramework.test.tsx +++ b/chartlets.js/packages/lib/src/actions/configureFramework.test.tsx @@ -2,11 +2,11 @@ import type { FC } from "react"; import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { configureFramework, resolvePlugin } from "./configureFramework"; import { store } from "@/store"; -import { type ComponentRegistration, registry } from "@/components/registry"; +import { registry } from "@/components/registry"; import type { HostStore } from "@/types/state/host"; -import type { Plugin } from "@/types/state/plugin"; +import type { RegistrableComponent, Plugin } from "@/types/state/plugin"; -function getComponents(): [string, ComponentRegistration][] { +function getComponents(): [string, RegistrableComponent][] { interface DivProps { text: string; } diff --git a/chartlets.js/packages/lib/src/components/Children.test.tsx b/chartlets.js/packages/lib/src/components/Children.test.tsx index b1777dcb..6ca79ff7 100644 --- a/chartlets.js/packages/lib/src/components/Children.test.tsx +++ b/chartlets.js/packages/lib/src/components/Children.test.tsx @@ -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", () => { @@ -33,7 +32,7 @@ describe("Children", () => { }); it("should render all child types", () => { - interface DivProps extends ComponentProps { + interface DivProps { text: string; } const Div: FC = ({ text }) =>
{text}
; diff --git a/chartlets.js/packages/lib/src/components/Component.test.tsx b/chartlets.js/packages/lib/src/components/Component.test.tsx index 9b2a4145..29ea4554 100644 --- a/chartlets.js/packages/lib/src/components/Component.test.tsx +++ b/chartlets.js/packages/lib/src/components/Component.test.tsx @@ -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"; @@ -27,7 +27,7 @@ describe("Component", () => { }); it("should render a known component", () => { - interface DivProps extends ComponentProps { + interface DivProps { text: string; } const Div: FC = ({ text }) =>
{text}
; diff --git a/chartlets.js/packages/lib/src/components/Component.tsx b/chartlets.js/packages/lib/src/components/Component.tsx index c5f0956a..caa39f03 100644 --- a/chartlets.js/packages/lib/src/components/Component.tsx +++ b/chartlets.js/packages/lib/src/components/Component.tsx @@ -1,12 +1,7 @@ import { type FC } from "react"; -import { type ComponentChangeHandler } from "@/types/state/event"; import { isString } from "@/utils/isString"; import { registry } from "@/components/registry"; - -export interface ComponentProps { - type: string; - onChange: ComponentChangeHandler; -} +import type { ComponentProps } from "@/types/state/plugin"; export const Component: FC = (props: ComponentProps) => { const ActualComponent = isString(props.type) && registry.lookup(props.type); diff --git a/chartlets.js/packages/lib/src/components/registry.ts b/chartlets.js/packages/lib/src/components/registry.ts index 5d96bd80..ce163cad 100644 --- a/chartlets.js/packages/lib/src/components/registry.ts +++ b/chartlets.js/packages/lib/src/components/registry.ts @@ -1,10 +1,4 @@ -import type { ComponentType } from "react"; - -import type { ComponentProps } from "@/components/Component"; - -export type ComponentRegistration = - | ComponentType> - | ComponentType; +import type { RegistrableComponent } from "@/types/state/plugin"; /** * A registry for Chartlets components. @@ -29,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: ComponentRegistration): () => 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): ComponentRegistration | undefined; + lookup(type: string): RegistrableComponent | undefined; /** * Clears the registry. @@ -52,9 +46,9 @@ export interface Registry { // export for testing only export class RegistryImpl implements Registry { - private components = new Map(); + private components = new Map(); - register(type: string, component: ComponentRegistration): () => void { + register(type: string, component: RegistrableComponent): () => void { const oldComponent = this.components.get(type); this.components.set(type, component); return () => { @@ -66,7 +60,7 @@ export class RegistryImpl implements Registry { }; } - lookup(type: string): ComponentRegistration | undefined { + lookup(type: string): RegistrableComponent | undefined { return this.components.get(type); } diff --git a/chartlets.js/packages/lib/src/index.ts b/chartlets.js/packages/lib/src/index.ts index 6271e50a..cd40e3a4 100644 --- a/chartlets.js/packages/lib/src/index.ts +++ b/chartlets.js/packages/lib/src/index.ts @@ -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 @@ -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"; diff --git a/chartlets.js/packages/lib/src/types/state/plugin.ts b/chartlets.js/packages/lib/src/types/state/plugin.ts index 965f9a49..2985afa6 100644 --- a/chartlets.js/packages/lib/src/types/state/plugin.ts +++ b/chartlets.js/packages/lib/src/types/state/plugin.ts @@ -1,14 +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 + | ComponentType; /** * A component registration - a pair comprising the component type name * and the React component. */ -export type ComponentRegistration = [ - string, - ComponentType | ComponentType, -]; +export type ComponentRegistration = [string, RegistrableComponent]; /** * A framework plugin.