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.

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

Conversation

mduan
Copy link

@mduan mduan commented Jun 21, 2022

Before this change (in PyCharm) navigating to Component.helpers.someMethod's definition would always take me to

[helper: string]: any;
. With these typing changes, it'll now navigate to the definition in Component.config.helpers.someMethod.

@@ -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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants