Improve types for animation hooks etc.

[jhsware]

The typing for animation hooks was incomplete:

• missing argument props in type for function components
• missing componentWillMove hook
• I believe domNode should accept HTMLElement | SVGElement
• we shouldn't pass the argument props to class component componentWillMove hook (it is inconsistent with the other hook, props should be referenced using this.props)

NOTE: The last change is a BREAKING CHANGE, but I think it is better to fix it now than have it linger. Worth a bump in minor version.

While implementing this I also noted the following inconsistency in typing of argument props:

componentWillUpdate?(nextProps: Readonly<{ children?: Inferno.InfernoNode } & P>, nextState: S, context: any): void;

onComponentWillUpdate?(lastProps: Readonly<P>, nextProps: Readonly<P>): void;

Where children? is only added for class component hooks.

Let me know if I should change this.

[coveralls]

Coverage remained the same at 93.27% when pulling 502afcb on jhsware:fix-types-inferno-animation into 688728e on infernojs:master.

[Havunen]

Hmm, doing that breaking change aint good idea IMO. It doesnt benefit much, render method in inferno also has nextProps available so its not a big deal

[Havunen]

Hmm? Why was this line removed?

[jhsware]

We shouldn't pass instance props to a Component Class since we don't do it in the other hooks (correct me if I am wrong), so I wanted to remove this inconsistency because it could cause confusion. See:

  componentDidMount?(): void;

  componentWillMount?(): void;

[jhsware]

Whereas I agree that breaking changes are bad, consistency is worth a lot, I think it is better to clean up the API early. We still have very few downloads of v8 and changing from props to this.props is intuitive. However, just updating the type definition and leaving this row would nudge people in the right direction by only throwing a type error. Then we could remove it in v9 (if we remember).

nextProps is different, because there you access current props with this.props too.

[jhsware]

@Havunen what is your take on props having different types for Class Components and Function Components? Fix or don't fix?

componentWillUpdate?(nextProps: Readonly<{ children?: Inferno.InfernoNode } & P>, nextState: S, context: any): void;

onComponentWillUpdate?(lastProps: Readonly<P>, nextProps: Readonly<P>): void;

[Havunen]

Props should have children property defined in its type

[Havunen]

I think we should add context to onComponentWillUpdate too

[jhsware]

Props should have children property defined in its type

Will fix

I think we should add context to onComponentWillUpdate too

Will fix

I'll need a couple of days until I have completed this.

[jhsware]

@Havunen I have made the changes EXCEPT adding context to the on[xxx]Update hooks. That change broke tests and they are used both in normal and compat mode. In compat mode we don't send context (since React didn't) and I ran out of time figuring out what to change, so I backed out on the context change.

[Havunen]

Great! Can you fix the conflict please :)

[jhsware]

@Havunen PR rebased on master. Didn't run prettier at the end because there were many files affected that I haven't worked on. Better to run prettier separately on master.

[Havunen]

LGTM, thanks for the PR