TypeScript definitions for 2.0.10

[icylace]

A new and improved TypeScript definitions PR !

[zaceno]

type StateWithEffects<S, D = any> = [State<S>, ...(Effect<S, D> | RunnerDescriptor<S, D>)[]]

appears to permit the following usage

import {delay} from @hyperapp/time
const MyAction = state => [state, delay]

But as far as I know hyperapp does not support this, and will encounter a runtime error.

[zaceno]

The following tests represent valid uses of hyperapp, but fail with the current types.

// -------------------------------------
// app with initial effect

import { delay } from '../../packages/time/index.js'
const IncrementFoo: Action<Test> = state => ({ ...state, foo: state.foo + 1 })

// $ExpectType void
app<Test>({
  init: [{ foo: 2 }, delay(2000, { foo: 3 })],
  view: state => h('main', {}, []),
  node: document.body,
})

// $ExpectType void
app<Test>({
  init: [{ foo: 2 }, delay(2000, IncrementFoo)],
  view: state => h('main', {}, []),
  node: document.body,
})

// $ExpectType void
app<Test>({
  init: (state: Test) => [{ foo: 2 }, delay(200, IncrementFoo)],
  view: state => h('main', {}, []),
  node: document.body,
})

As far as I can tell, the reason they fail is because delay(200, {foo: 3}) and delay(200, IncementFoo) are not assignable to RunnerDescriptor<Test> but I could be mistaken.

The error messages are:

> [email protected] dts /Users/zache/Code/hyperapp
> dtslint types

Error: /Users/zache/Code/hyperapp/types/test/app.test.ts:126:12
ERROR: 126:12  expect  [email protected] compile error: 
Type '{ foo: number; }' is not assignable to type 'Action<Test, any>'.
  Object literal may only specify known properties, and 'foo' does not exist in type 'Action<Test, any>'.
ERROR: 133:12  expect  [email protected] compile error: 
Type '{ foo: number; }' is not assignable to type 'Action<Test, any>'.
  Object literal may only specify known properties, and 'foo' does not exist in type 'Action<Test, any>'.
ERROR: 140:3   expect  [email protected] compile error: 
Type '(state: Test) => [{ foo: number; }, (((dispatch: any, props: any) => NodeJS.Timeout) | { delay: any; action: any; })[]]' is not assignable to type 'StateFormat<Test> | Action<Test, any>'.
  Type '(state: Test) => [{ foo: number; }, (((dispatch: any, props: any) => NodeJS.Timeout) | { delay: any; action: any; })[]]' is not assignable to type 'ActionTransform<Test, any>'.
    Type '[{ foo: number; }, (((dispatch: any, props: any) => Timeout) | { delay: any; action: any; })[]]' is not assignable to type 'StateFormat<Test> | Action<Test, any>'.
      Type '[{ foo: number; }, (((dispatch: any, props: any) => Timeout) | { delay: any; action: any; })[]]' is not assignable to type 'ActionDescriptor<Test, any>'.
        Type '{ foo: number; }' is not assignable to type 'Action<Test, any>'.
          Type '{ foo: number; }' is not assignable to type 'ActionDescriptor<Test, any>'.

    at /Users/zache/Code/hyperapp/node_modules/dtslint/bin/index.js:206:19
    at Generator.next (<anonymous>)
    at fulfilled (/Users/zache/Code/hyperapp/node_modules/dtslint/bin/index.js:6:58)

[icylace]

@zaceno Nice work! I'll look at these later.

[icylace]

@asabatba Your subscriptions example from #969 (comment) seems to expose a bug in my types. An important find indeed! I'll see what I can do.

[icylace]

@zaceno

import {delay} from @hyperapp/time
const MyAction = state => [state, delay]

But as far as I know hyperapp does not support this, and will encounter a runtime error.

Well,

const MyAction = state => [state, delay]

is, in Hyperapp's eyes, equivalent to

const MyAction = state => [(state) => state, delay]

which is covered by the type definitions

type Action<S, P = any> = ActionTransform<S, P> | ActionDescriptor<S, P>
type ActionTransform<S, P = any> = (state: State<S>, props?: Payload<P>) => StateFormat<S> | Action<S>

type StateFormat<S> = State<S> | StateWithEffects<S>
type StateWithEffects<S, D = any> = [State<S>, ...(Effect<S, D> | RunnerDescriptor<S, D>)[]]

The key is ActionTransform.

[zaceno]

@icylace You're mistaken I'm afraid.

state => [state, delay]

and

state => [(state) => state, delay]

are not equivalent to hyperapp. The former causes a runtime error while the latter does not, as evidenced by this live example

[icylace]

@zaceno I love test cases !

[zaceno]

@zaceno I love test cases !

Sorry I think I misunderstood you before. Like you were approving of my flems.io example.😅 I now think you meant: "please give me a test case that I can make work, or reject as being an invalid test case" ? If so, here's a failing test case. I believe the types should be altered so that it does not fail:

import { Dispatch, Payload, Effect, Action } from "hyperapp"

type Test = { x: number; y: number }

const runJustEcho = (
  _dispatch: Dispatch<Test>,
  data?: Payload<string>
): void => {
  console.log(data)
}

const justEcho: Effect<Test, string> = (x) => [runJustEcho, x]

/*
  The following action definition is not valid 
  hyperapp. We would expect a type error.

  However, the types as defined allow this
  action definition. Hence this test
  case fails
*/
// $ExpectError
const IncrXAndEcho: Action<Test> = (state: Test) => [
  { ...state, x: state.x + 1 },
  justEcho
]

---

UPDATE:

Changing:

type StateWithEffects<S, D = any> = [State<S>, ...(Effect<S, D> | RunnerDescriptor<S, D>)[]]

to:

 type StateWithEffects<S, D = any> = [State<S>, ...RunnerDescriptor<S, D>[]]

in the definition file fixes it. And breaks no other tests.

[zaceno]

@icylace Here's another test case that I would not expect to fail but does:

// $ExpectType VDOM<unknown>
h("input", {oninput: (state, event) => event.target.value})

Error output:

ERROR: 228:40  expect                  [email protected] compile error: 
Object is possibly 'undefined'.
ERROR: 228:40  expect                  [email protected] compile error: 
Object is possibly 'null'.
ERROR: 228:53  expect                  [email protected] compile error: 
Property 'value' does not exist on type 'EventTarget'.

I'm afraid I'm not good enough with TS yet to know wether the types should be improved, loosened or if the test itself needs to have some type annotations to pass.

EDIT: This is a valid failure. My test should not pass without additional validation that the event target is not null, and casting the event.target to the expected type. Ignore this comment.

[zaceno]

Further findings about #1016 (comment) :

It is because RunnerDescriptor is typed as a tuple:

 type RunnerDescriptor<S, D> = [Runner<S, D>, Payload<D>]

However, the type-checker is unable to infer that delay(2000, { foo: 3 }) matches RunnerDescriptor, as TS interprets the return value [fn, {foo: number}] at best as (Runner | Payload)[] – that is to say: an array of any length where each of the items could be either Runner or Payload type. – which of course doesn't match.

The following is how the test could be made to work:

import { h, app, Action, RunnerDescriptor } from "hyperapp"
import { delay } from "../../packages/time/index.js"

type Test = { foo: number; bar?: number }

const IncrementFoo: Action<Test> = state => ({ ...state, foo: state.foo + 1 })

// $ExpectType void
app<Test>({
    init: [{ foo: 2 }, delay(200, IncrementFoo) as RunnerDescriptor<Test, any>], // <--- note "as RunnerD..."
    view: state => h("main", {}, []),
    node: document.body,
})

The same problem, and same approach would go for any action/event handler where an action with payload is needed: It would have to be cast to ActionDescriptor using as.

Seems like it could end up getting annoying... especially when using plain javascript as it requires using /** @type {import('hyperapp').ActionDescriptor} */([MyAction, 'foo']) (I think. Have not yet verified this). Wonder if there's a way to cast types by force inside the declaration file?

————-

Edit: you would only need to cast to a tuple in those cases where the function you are importing doesn’t have types. I assume that once this PR is merged we’ll go on to add types to all the @hyperapp/xyz libraries.

So in practice it’s only a problem for javascripters who are making custom effects AND want to use typechecking. They will have to write some type annotations I JSDoc-comments to silence the type checker sometimes. Maybe not such a big deal - I think I could live with it anyway.

[jorgebucaran]

I want to wait for this before going official, but just how close are we to merging here? Left you some comments, @icylace.

[jorgebucaran]

I'd like to remove all non-essential type tests, and put just the bare minimum in a single file.

[icylace]

I'd like to remove all non-essential type tests, and put just the bare minimum in a single file.

I would agree that any redundant tests should be removed but what is a "non-essential" test? Why would you assume the bare minimum fits appropriately in a single file?

[jorgebucaran]

I just one to merge index.d.ts, so perhaps the comments could include some examples? 😄

[jorgebucaran]

Let's just merge index.d.ts and take it from there. @icylace Can you update the PR? 🙏

[icylace]

I'm making updates soon which includes bug fixes and some renaming.

[icylace]

Massive changes were made !!!

Here's my changelog:

• I moved the types test suite into their own dedicated repo! For those interested, they're now available at: https://github.com/icylace/hyperapp-types-tests
  • This was done to help @jorgebucaran focus his long-term resources.
• EventActions now recognizes when custom payloads are being used with event handlers.
  • However, it's not perfect. The types test repo contains the relevant test cases, graciously provided by @zaceno.
• Updated App in the following ways:
  • Dispatch is returned instead of void.
  • init: was made optional.
  • Renamed middleware: to dispatch:.
  • Refactored/expanded App to keep treating app({}) as an error while recognizing other uses of app.
    • Certain obscure edge cases are currently not handled correctly. See the types test repo for details.
• In discussions about the recently merged docs PR @zaceno suggested that VNodes were redundant and ultimately I agree, so I replaced VNode with the new MaybeVDOM. Also, I removed talk of VNodes from the docs.
• Inlined MaybeNode in the only placed it was used and then removed it because it seemed unnecessary.
• Inlined StateFormat where it's used in the type definitions and then removed it.
  • I originally created StateFormat to handle situations where an action may or may not invoke effects depending on what happens within its processing. However, I later realized StateWithEffects can cover this use case pretty well.
• The original Effect got dropped from StateWithEffects as per @zaceno's suggestion.
• Renamed the props parameter of ActionTransform to payload.
• Renamed the props parameter of Transform to payload.
• Renamed the props parameter of Runner to payload.
• Renamed Effect to EffectCreator.
• Renamed RunnerDescriptor to Effect.
• Renamed Runner to Effecter as suggested by @skanne .
• Inlined Subscription into Subscriptions.
• Removed the original Subscription.
• Renamed SubscriberDescriptor to Subscription.
• The docs were updated to reflect the new terminology.
• Some comments are either new, removed, reordered, or slightly updated.
• Test cases were updated accordingly.

For now, I've decided to leave in certain "superfluous" but useful types, namely EffectCreator and Transform. I believe their inclusion in the core types is worthy of debate but for the moment I think the focus should be on having the types as a whole in a releaseable state.

[zaceno]

Wow @icylace, massive work! Look forward to digging in to the news!

[jorgebucaran]

Thank you, @icylace! 💯