Skip to content
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.

Sign up for GitHub

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

Improve TypeScript typing for Component.helpers #158

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Improve TypeScript typing for Component.helpers #158

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 5 additions & 4 deletions lib/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export interface PanelLifecycleContext {
unbindFromComponent?(component: Component<any>): void;
}

export interface ConfigOptions<StateT, AppStateT = unknown, ContextRegistryT = unknown> {
export interface ConfigOptions<StateT, AppStateT = unknown, ContextRegistryT = unknown, HelpersT extends PanelHelpers = unknown> {
/** Function transforming state object to virtual dom tree */
template(scope?: StateT): VNode;

Expand All @@ -94,7 +94,7 @@ export interface ConfigOptions<StateT, AppStateT = unknown, ContextRegistryT = u
appState?: AppStateT;

/** Properties and functions injected automatically into template state object */
helpers?: PanelHelpers;
helpers?: HelpersT;

/** Extra rendering/lifecycle callbacks */
hooks?: PanelHooks<StateT>;
Expand Down Expand Up @@ -153,7 +153,8 @@ export class Component<
AttrsT = AnyAttrs,
AppStateT = unknown,
AppT = unknown,
ContextRegistryT = unknown
ContextRegistryT = unknown,
HelpersT extends PanelHelpers = unknown
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to take advantage of this, you'll need to include a component-specific helpers type to fill in the generic for each individual component, right? Given that we're moving away from using helpers in general (it was mainly there for Jade/Pug ergonomics, and is mostly just extra noise in already-noisy TSX), I wonder if it's worth adding these helpers type defs everywhere. The better move may be to spend that effort moving things out of helpers into main component methods.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you'll need to include a component-specific helpers type to fill in the generic for each individual component, right?

You don't need to fill in the generic to get the code navigation working. I'm not sure if it's a TypeScript thing or something PyCharm-specific that's making this work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, if it doesn't need anything except this unused generic, I'm fine with adding it, at least until helpers goes away. Just get the build passing and we can merge.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we just type it with &?

get config(): ReturnType<Component['config']> & Helpers

another generic on the class is pretty messy indeed.

> extends WebComponent {
/** The first Panel Component ancestor in the DOM tree; null if this component is the root */
$panelParent: Component<unknown>;
Expand Down Expand Up @@ -198,7 +199,7 @@ export class Component<
applyStaticStyle(styleSheetText: null | string, options?: {ignoreCache: boolean}): void;

/** Defines standard component configuration */
get config(): ConfigOptions<StateT, AppStateT, ContextRegistryT>;
get config(): ConfigOptions<StateT, AppStateT, ContextRegistryT, HelpersT>;

/**
* Template helper functions defined in config object, and exposed to template code as $helpers.
Expand Down