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

Problems with Ref type declaration make explicit typing impossible #1209

Closed
TedDriggs opened this issue Sep 8, 2017 · 4 comments
Closed

Comments

@TedDriggs
Copy link
Contributor

Observed Behaviour

import createElement from 'inferno-create-element';
import { InfernoChildren } from 'inferno';
import Component from 'inferno-component';

export class Mine extends Component<{}, void> {
    render(): InfernoChildren {
        return <div></div>;
    }
}

export class Outer extends Component<{}, void> {
    private r: Mine | null;

    render(): InfernoChildren {
        return <Mine ref={(n: Mine | null | undefined) => (this.r = n!;)} />
    }
}

This fails to typecheck:

file: 'file:///Users/___/ts/record-editor/src/repro.tsx'
severity: 'Error'
message: 'Type '{ ref: (n: Maybe<Mine>) => Mine; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Mine> & Props'.
  Type '{ ref: (n: Maybe<Mine>) => Mine; }' is not assignable to type 'Props'.
    Types of property 'ref' are incompatible.
      Type '(n: Maybe<Mine>) => Mine' is not assignable to type 'Ref | undefined'.
        Type '(n: Maybe<Mine>) => Mine' is not assignable to type 'Ref'.
          Types of parameters 'n' and 'node' are incompatible.
            Type 'Element | null | undefined' is not assignable to type 'Maybe<Mine>'.
              Type 'Element' is not assignable to type 'Maybe<Mine>'.
                Type 'Element' is not assignable to type 'Mine'.
                  Property 'render' is missing in type 'Element'.'
at: '17,22'
source: 'ts'

Following the Ref type declaration in inferno/src/core/VNodes.ts, we see:

export type Ref = (node?: Element | null) => void;

This has three problems:

  1. For a component T, the parameter should be (node?: T | null). The same is technically true for HTML elements, as it will be passing the element's concrete instance to the callback in all cases.
  2. The return type is void, when any would be more appropriate. Note that the React typings already handle this correctly (DefinitelyTyped).
  3. The sole argument is declared as optional and nullable, which forces callers to handle null and undefined cases. This doesn't seem to match with real-world behavior.

Expected Current Behaviour

Inferno should have the correct type information exposed for the ref property. This is the largest hurdle I've encountered while attempting to resolve #686; most of the HTML typings can be used as-is from the React typings on DefinitelyTyped, and the code needed to use VNode etc. isn't that much.

I believe the correct type for a ref attribute would be:

export type Ref<T> = (instance: T | null) => any;

Inferno Metadata

macOS 10.12, inferno 3.9.0

@Havunen
Copy link
Member

Havunen commented Sep 9, 2017

@TedDriggs If you can figure out how to make it work, PR would be nice :)

@Havunen
Copy link
Member

Havunen commented Sep 17, 2017

@TedDriggs Any update on this? One option is that we just remove Ref type?

@rendall
Copy link

rendall commented Sep 26, 2017

@Havunen @TedDriggs This gist can give you a good head start. It's just the React typedefs modified for Inferno. It seems to resolve errors with TSX and the basic Inferno lib, although not for inferno-router (and others, probably).

https://gist.github.com/rendall/cdd23c962c88fac3dbd9322cc2b09d58

@Havunen
Copy link
Member

Havunen commented Mar 17, 2018

V5 is released. This is fixed there

@Havunen Havunen closed this as completed Mar 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants