-
Notifications
You must be signed in to change notification settings - Fork 781
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
Usage with TypeScript #1048
Comments
@stewartrule Hyperapp's type definitions are brand new and haven't been real-world-tested by enough people in enough scenarios yet, to really give you any good answers. Your input here is much appreciated and will help us polish things. Sorry that's the best answer I can give for now. I can only chime in that I too have had a lot of issues with the generic type parameters to generic functions not being inferred correctly (becoming @icylace might have more concrete help to give. I know he's working on a usage guide for the types but it is also still a WIP |
@zaceno Thanks for your answer. I'm starting to think wrapping might be the only solution here since an import of export const useSelector: TypedUseSelectorHook<State> = useReduxSelector; maybe hyperapp could expose something similar so you could do: export const h: TypedH<State> = originalH; |
Interesting idea! TBH I'm kind of thinking the state inference down the entire vdom might not be worth it. If we got rid of it, it would open the possibility for mistakes of the kind where actions in different parts of the view are defined for different types of state. But I feel like that is going to be a pretty unlikely mistake, so the cost is not very high. The benefit would be that we don't get errors just because the inference failed for some reason. |
@stewartrule Thank you for your input! I'll take a look at this when I have time later. @zaceno It is worth it. "errors just because the inference failed for some reason" is important to me and my efforts to use Hyperapp in an enterprise setting. I'm sure I'm not the only one. Besides, casting to |
@stewartrule Yeah, TypeScript's type inference is weird sometimes. I'm still investigating this but another option you can do is: import { app, h, text, View } from "hyperapp";
import { ShapeType, state, State } from "./state";
const Canvas = (state: State) => {
return h("div", {}, state.shapes.map(toShape));
};
const toShape = (shape: ShapeType) => {
return h<State>("h3", {}, [text(shape.type)]);
};
app<State>({
init: state,
view: Canvas,
node: document.getElementById("app")!,
}); Notice the |
@stewartrule I've never used React stuff, so thank you for mentioning Please verify the following works on your end: Add this new type definition to interface TypedH<S> {
<_, T extends string = string>(
tag: T extends "" ? never : T,
props: PropList<S>,
children?: MaybeVDOM<S> | readonly MaybeVDOM<S>[]
): VDOM<S>
} and now hopefully this works for you: import { app, h as ha, text, View } from "hyperapp";
import { ShapeType, state, State } from "./state";
const h: TypedH<State> = ha;
const Canvas = (state: State) => {
return h("div", {}, state.shapes.map(toShape));
};
const toShape = (shape: ShapeType) => {
return h("h3", {}, [text(shape.type)]);
};
app<State>({
init: state,
view: Canvas,
node: document.getElementById("app")!,
}); Let us know how it goes ! Edit: To be clear, I'm planning on turning this solution into a PR. |
@icylace yep, that seems to work. I do think you can get rid of the |
@stewartrule That should prevent some gotcha errors from happening. When I have time later I'll explain. Edit: On second thought, maybe those errors would be beneficial.... hmm... will discuss later ! |
TL;DR -- Let's discuss const Canvas = (state: State) => {
return h("div", {}, state.shapes.map(toShape));
};
const toShape = (shape: ShapeType) => {
return h<State>("h3", {}, [text(shape.type)]);
}; versus: const h: TypedH<State> = ha;
const Canvas = (state: State) => {
return h("div", {}, state.shapes.map(toShape));
};
const toShape = (shape: ShapeType) => {
return h("h3", {}, [text(shape.type)]);
}; Both of them work. In the former, one Now, imagine a user who cares about this trade-off and is trying to decide one way or the other about what's best for their app as they work on it. They may experiment with including or removing So, if const h: TypedH<State> = ha;
const Canvas = (state: State) => {
return h("div", {}, state.shapes.map(toShape));
};
const toShape = (shape: ShapeType) => {
return h<State>("h3", {}, [text(shape.type)]);
}; And if
After weighing these options I've opted for correctness over convenience in this instance. However, simply removing the What if your state type was a string literal that exactly matched the tag name of the h<"h3">("h3", {}, [text(shape.type)]) Yeah, I know the odds of that are pretty unlikely but it's easily preventable while achieving the goal of increased correctness by replacing the So, I'm going with that. |
@stewartrule The latest Hyperapp dot release, 2.0.15, contains |
@stewartrule I played around a little with your original example and this seems to work nicely (without the need for typing every import {h, text, app, VDOM} from 'hyperapp'
type State = {shapes: ShapeType[]}
type ShapeType = {type: string}
const Canvas = (state: State):VDOM<State> => h("div", {}, state.shapes.map(x => toShape(x)))
const toShape = <S>(shape: ShapeType):VDOM<S> => h("h3", {}, [text(shape.type)])
app({
init: {shapes: []},
view: Canvas,
node: document.getElementById("app")!,
}); Two things make this work (as far as I can tell):
As regards that last point, I think it is because typescript is expecting a function with the full mapper signature (with the second and third arguments typed as well), and when it gets a function that is explicitly typed differently, it throws off the inference somehow. |
I hope this is the right place to ask/give feedback about the hyperapp type definitions. The type definition for Effect currently is type Effect<S, P = any> = readonly [
effecter: (dispatch: Dispatch<S>, payload: P) => void | Promise<void>,
payload: P
] While the documentation for Effect defines an Effect as |
@tillarnold Writing effects, or more specifically effecters, without a payload parameter should already be valid with the current definitions. That may sound strange. To find out why TypeScript does this, check this out: https://github.com/Microsoft/TypeScript/wiki/FAQ#why-are-functions-with-fewer-parameters-assignable-to-functions-that-take-more-parameters |
I'm not entirely sure but I think my previous comment was a bit impresise and I think the problem is not with the type Effect<S, P = any> = readonly [
effecter: (dispatch: Dispatch<S>, payload: P) => void | Promise<void>,
payload?: P
] I propose this because with the current definition I get a typescript error with an Action like export const FooBar = (state: State): Dispatchable<State> => [
{ ...state, abc: false },
[
(dispatch) => {
doExternalThing();
},
],
]; |
@tillarnold I would open a new issue for this topic since it is technically a different question than the OP of this issue. But anyway, I think you're right. "Inline effects" like this import {Action} from 'hyperapp'
type State = {abc: boolean, def: number}
const FooBar : Action<State, any> = state => [
{ ...state, abc: false },
[() => console.log('Hello!')],
] ...while perhaps not exactly "best practice" are technically allowed in plain javascript, and often useful in library code or or in early stages of development. As such the types should allow it, but as you point out they dont (see also playground), |
Hi, I'm running into some issues when using
hyperapp
(^2.0.13
) withtypescript
(^4.2.3
).I remember trying things about about 6 months ago and had similar experiences but thought maybe the definitions were not really ready yet.
I'm not sure if there are any guidelines into usage with ts, but I kind of feel the generics aren't always passed down properly. I wrote down some issues below.
No problems yet.
Introducing
.map(toShape)
suddenly causes a conflict.Removing
View<State>
and just using(state: State)
moves the error up to theapp
call.Passing
State
to every call toh
seems to work, but I really don't want to do that.The only thing that seems to work ok is wrapping the
h
call itself and just hardcodeS
toState
but since I just started looking into this I'm not sure if this won't cause other issues later on.Any ideas on how to use hyperapp properly when using typescript?
The text was updated successfully, but these errors were encountered: