hyperapp-router icon indicating copy to clipboard operation
hyperapp-router copied to clipboard

WIP: Only way found to fix nasty erros on TS build.

Open tachyon-ops opened this issue 5 years ago • 7 comments

  • Reason for export function Link(props: LinkProps): VNode<LinkProps> | any; change request:
    TS2605: JSX element type 'void | VNode<LinkProps>' is not a constructor function for JSX elements.
  Type 'void' is not assignable to type 'Element | null'.
  • Reason for export function Route<P>( props: RouteProps<P>): VNode<RenderProps<P>> | any; change request:
    TS2605: JSX element type 'VNode<RenderProps<{}>>' is not a constructor function for JSX elements.
  Type 'VNode<RenderProps<{}>>' is missing the following properties from type 'Element': type, props

Let me know if you guys find exactly which type we should declare.

tachyon-ops avatar Dec 02 '18 16:12 tachyon-ops

Codecov Report

Merging #92 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #92   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           6      6           
  Lines          68     68           
  Branches       11     11           
=====================================
  Hits           68     68

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 97b0a07...c59997c. Read the comment docs.

codecov[bot] avatar Dec 02 '18 16:12 codecov[bot]

The first warning seem explicitly enough for me. I would move on VNode<LinkProps> | null. A VNode must not be undefined but can be null. For the second, I guess Hyperapp.VNode should extends JSX.Element.

Plus, returns seem incorrect, as since #59 helpers do not return VNode, they return Lazy Components.

Swizz avatar Dec 03 '18 10:12 Swizz

@Swizz

declare global {
  namespace JSX {
    interface Element extends VNode<any> {}
    interface IntrinsicElements {
      [elemName: string]: any
    }
  }
}

Tested null for the first and it didn't work as well... @Swizz have you tested your suggestions?

tachyon-ops avatar Dec 03 '18 15:12 tachyon-ops

Untested. I knew that DT use JSX.Element | null on some places : https://github.com/DefinitelyTyped/DefinitelyTyped/blob/8ff7e8f29e059f3799c19e0bafb7ac010786ca50/types/react/v15/index.d.ts#L314

I think it is more related to Element and not to the null as any match with Element | null.

So I would try to extend Hyperapp.VNode from JSX.Element.

Swizz avatar Dec 03 '18 15:12 Swizz

But this means an update on the hyperapp dependency. What's your take on this @jorgebucaran ?

tachyon-ops avatar Dec 03 '18 15:12 tachyon-ops

@nmpribeiro Maybe someone with more TS experience can comment.

jorgebucaran avatar Dec 03 '18 16:12 jorgebucaran

At least someone able to understand the return types of these JSX.Element and the VNode. h function should only expect JSX.Element right (at least only VNode)?. I will look at that again when possible.

tachyon-ops avatar Dec 04 '18 13:12 tachyon-ops