Skip to content

Commit

Permalink
Improve types for animation hooks etc. (#1612)
Browse files Browse the repository at this point in the history 
* Improved specificity in types and removed an inconsistent passing of props to method on Class Component (componentWillMove)

* Added missing argument props

* Added type for missing hook componentWillMove

* BREAKING CHANGE: Don't set and pass props to Class Component for componentWillMove. It should be referenced using this.props. This was implemented inconsistently with the other Class Component animation hooks and I think it is better to fix it now than have it linger. Worth a bump in minor version.

* Fix review comment "Props should have children property defined in its type"

* Defer breaking change, enough that it is hinted in types
  • Loading branch information
@jhsware
jhsware authored Oct 31, 2022
1 parent 688728e commit 25b6ad1
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 38 deletions.
8 changes: 4 additions & 4 deletions packages/inferno-animation/src/AnimatedAllComponent.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ type AnimationProp = {
};

export abstract class AnimatedAllComponent<P = {}, S = {}> extends Component<AnimationProp & P, S> {
public componentDidAppear(dom: HTMLElement) {
public componentDidAppear(dom: HTMLElement | SVGElement) {
componentDidAppear(dom, this.props);
}

public componentWillDisappear(dom: HTMLElement, callback: Function) {
public componentWillDisappear(dom: HTMLElement | SVGElement, callback: Function) {
componentWillDisappear(dom, this.props, callback);
}

public componentWillMove(parentVNode, parent: HTMLElement, dom: HTMLElement, props: any) {
componentWillMove(parentVNode, parent, dom, props);
public componentWillMove(parentVNode, parent: HTMLElement | SVGElement, dom: HTMLElement | SVGElement) {
componentWillMove(parentVNode, parent, dom, this.props);
}
}
2 changes: 1 addition & 1 deletion packages/inferno-animation/src/AnimatedComponent.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export abstract class AnimatedComponent<P = {}, S = {}> extends Component<Animat
componentDidAppear(dom, this.props);
}

public componentWillDisappear(dom: HTMLElement, callback: Function) {
public componentWillDisappear(dom: HTMLElement | SVGElement, callback: Function) {
componentWillDisappear(dom, this.props, callback);
}
}
4 changes: 2 additions & 2 deletions packages/inferno-animation/src/AnimatedMoveComponent.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ type AnimationProp = {
};

export abstract class AnimatedMoveComponent<P = {}, S = {}> extends Component<AnimationProp & P, S> {
public componentWillMove(parentVNode, parent: HTMLElement, dom: HTMLElement, props: any) {
componentWillMove(parentVNode, parent, dom, props);
public componentWillMove(parentVNode, parent: HTMLElement | SVGElement, dom: HTMLElement | SVGElement) {
componentWillMove(parentVNode, parent, dom, this.props);
}
}
16 changes: 8 additions & 8 deletions packages/inferno-animation/src/animations.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function getAnimationClass(animationProp: AnimationClass | string | undefined |
return animCls;
}

export function componentDidAppear(dom: HTMLElement, props) {
export function componentDidAppear(dom: HTMLElement | SVGElement, props) {
// Get dimensions and unpack class names
const cls = getAnimationClass(props.animation, '-enter');

Expand All @@ -66,7 +66,7 @@ function _getDidAppearTransitionCallback(dom, cls) {
};
}

function _didAppear(phase: AnimationPhase, dom: HTMLElement, cls: AnimationClass, dimensions, display: string, sourceState: GlobalAnimationState | null) {
function _didAppear(phase: AnimationPhase, dom: HTMLElement | SVGElement, cls: AnimationClass, dimensions, display: string, sourceState: GlobalAnimationState | null) {
switch (phase) {
case AnimationPhase.INITIALIZE:
// Needs to be done in a single pass to avoid reflows
Expand Down Expand Up @@ -123,7 +123,7 @@ function _didAppear(phase: AnimationPhase, dom: HTMLElement, cls: AnimationClass
}
}

export function componentWillDisappear(dom: HTMLElement, props, callback: Function) {
export function componentWillDisappear(dom: HTMLElement | SVGElement , props, callback: Function) {
// Get dimensions and unpack class names
const cls = getAnimationClass(props.animation, '-leave');
const dimensions = getDimensions(dom);
Expand All @@ -134,7 +134,7 @@ export function componentWillDisappear(dom: HTMLElement, props, callback: Functi
}
}

function _willDisappear(phase: AnimationPhase, dom: HTMLElement, callback: Function, cls: AnimationClass, dimensions) {
function _willDisappear(phase: AnimationPhase, dom: HTMLElement | SVGElement, callback: Function, cls: AnimationClass, dimensions) {
switch (phase) {
case AnimationPhase.MEASURE:
// 1. Set animation start state and dimensions
Expand Down Expand Up @@ -165,7 +165,7 @@ function _willDisappear(phase: AnimationPhase, dom: HTMLElement, callback: Funct
}
}

export function componentWillMove(parentVNode, parent: HTMLElement, dom: HTMLElement, props: any) {
export function componentWillMove(parentVNode, parent: HTMLElement | SVGElement, dom: HTMLElement | SVGElement, props: any) {
// tslint:disable-next-line
if (_DBG_MVE_) console.log('Animating move', dom);

Expand All @@ -174,7 +174,7 @@ export function componentWillMove(parentVNode, parent: HTMLElement, dom: HTMLEle
if (!parentVNode.$MV) {
parentVNode.$MV = true;
els = [];
let tmpEl = parent.firstChild as HTMLElement;
let tmpEl = parent.firstChild as HTMLElement | SVGElement;
while (!isNull(tmpEl)) {
els.push({
dx: 0,
Expand All @@ -183,7 +183,7 @@ export function componentWillMove(parentVNode, parent: HTMLElement, dom: HTMLEle
moved: false,
node: tmpEl
});
tmpEl = tmpEl.nextSibling as HTMLElement;
tmpEl = tmpEl.nextSibling as HTMLElement | SVGElement;
}
}

Expand Down Expand Up @@ -282,7 +282,7 @@ function _willMove(phase: AnimationPhase, cls: AnimationClass, animState) {
}
}

function _getWillMoveTransitionCallback(dom: HTMLElement, cls: AnimationClass) {
function _getWillMoveTransitionCallback(dom: HTMLElement | SVGElement, cls: AnimationClass) {
return () => {
// Only remove these if the translate has completed
const cbCount = decrementMoveCbCount(dom);
Expand Down
26 changes: 13 additions & 13 deletions packages/inferno-animation/src/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ function getClassNameList(className: string) {
return className.split(' ').filter(filterEmpty);
}

export function addClassName(node: HTMLElement, className: string) {
export function addClassName(node: HTMLElement | SVGElement, className: string) {
const classNameList = getClassNameList(className);

for (let i = 0; i < classNameList.length; i++) {
node.classList.add(classNameList[i]);
}
}

export function removeClassName(node: HTMLElement, className: string) {
export function removeClassName(node: HTMLElement | SVGElement, className: string) {
const classNameList = getClassNameList(className);

for (let i = 0; i < classNameList.length; i++) {
Expand All @@ -36,7 +36,7 @@ export function forceReflow() {
}

// A quicker version used in pre_initialize
export function resetDisplay(node: HTMLElement, value?: string) {
export function resetDisplay(node: HTMLElement | SVGElement, value?: string) {
if (value !== undefined) {
node.style.setProperty('display', value);
} else {
Expand All @@ -45,7 +45,7 @@ export function resetDisplay(node: HTMLElement, value?: string) {
}
}

export function setDisplay(node: HTMLElement, value?: string) {
export function setDisplay(node: HTMLElement | SVGElement, value?: string) {
const oldVal = node.style.getPropertyValue('display');

if (oldVal !== value) {
Expand All @@ -59,14 +59,14 @@ export function setDisplay(node: HTMLElement, value?: string) {
return oldVal;
}

function _cleanStyle(node: HTMLElement) {
function _cleanStyle(node: HTMLElement | SVGElement) {
if (!node.style) {
// https://developer.mozilla.org/en-US/docs/Web/API/Element/removeAttribute
node.removeAttribute('style');
}
}

export function getDimensions(node: HTMLElement) {
export function getDimensions(node: HTMLElement | SVGElement) {
const tmpDisplay = node.style.getPropertyValue('display');

// The `display: none;` workaround was added to support Bootstrap animations in
Expand Down Expand Up @@ -94,17 +94,17 @@ export function getDimensions(node: HTMLElement) {
};
}

export function getOffsetPosition(node: HTMLElement) {
export function getOffsetPosition(node: HTMLElement | SVGElement) {
const { x, y } = node.getBoundingClientRect();
return { x, y };
}

export function getGeometry(node: HTMLElement) {
export function getGeometry(node: HTMLElement | SVGElement) {
const { x, y, width, height } = node.getBoundingClientRect();
return { x, y, width, height };
}

export function setTransform(node: HTMLElement, x: number, y: number, scaleX: number = 1, scaleY: number = 1) {
export function setTransform(node: HTMLElement | SVGElement, x: number, y: number, scaleX: number = 1, scaleY: number = 1) {
const doScale = scaleX !== 1 || scaleY !== 1;
if (doScale) {
node.style.transformOrigin = '0 0';
Expand All @@ -114,17 +114,17 @@ export function setTransform(node: HTMLElement, x: number, y: number, scaleX: nu
}
}

export function clearTransform(node: HTMLElement) {
export function clearTransform(node: HTMLElement | SVGElement) {
node.style.transform = '';
node.style.transformOrigin = '';
}

export function setDimensions(node: HTMLElement, width: number, height: number) {
export function setDimensions(node: HTMLElement | SVGElement, width: number, height: number) {
node.style.width = width + 'px';
node.style.height = height + 'px';
}

export function clearDimensions(node: HTMLElement) {
export function clearDimensions(node: HTMLElement | SVGElement) {
node.style.width = node.style.height = '';
}

Expand Down Expand Up @@ -216,7 +216,7 @@ function setAnimationTimeout(onTransitionEnd, rootNode, maxDuration) {
* @param nodes a list of nodes that have transitions that are part of this animation
* @param callback callback when all transitions of participating nodes are completed
*/
export function registerTransitionListener(nodes: HTMLElement[], callback: Function) {
export function registerTransitionListener(nodes: (HTMLElement | SVGElement)[], callback: Function) {
const rootNode = nodes[0];

/**
Expand Down
5 changes: 3 additions & 2 deletions packages/inferno/src/DOM/utils/common.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,12 +171,12 @@ export function removeVNodeDOM(vNode: VNode, parentDOM: Element, animations: Ani
}
}

function addMoveAnimationHook(animations: AnimationQueues, parentVNode, refOrInstance, dom: Element, parentDOM: Element, nextNode: Element, flags, props) {
function addMoveAnimationHook(animations: AnimationQueues, parentVNode, refOrInstance, dom: Element, parentDOM: Element, nextNode: Element, flags, props?) {
animations.componentWillMove.push({
dom,
fn: () => {
if (flags & VNodeFlags.ComponentClass) {
refOrInstance.componentWillMove(parentVNode, parentDOM, dom, props);
refOrInstance.componentWillMove(parentVNode, parentDOM, dom);
} else if (flags & VNodeFlags.ComponentFunction) {
refOrInstance.onComponentWillMove(parentVNode, parentDOM, dom, props);
}
Expand Down Expand Up @@ -206,6 +206,7 @@ export function moveVNodeDOM(parentVNode, vNode, parentDOM, nextNode, animations

if (flags & VNodeFlags.ComponentClass) {
refOrInstance = vNode.children;
// TODO: We should probably deprecate this in V9 since it is inconsitent with other class component hooks
instanceProps = vNode.props;
vNode = children.$LI;
} else if (flags & VNodeFlags.ComponentFunction) {
Expand Down
20 changes: 12 additions & 8 deletions packages/inferno/src/core/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ export interface IComponent<P, S> {

componentWillDisappear?(domNode: Element, callback: Function): void;

componentWillMove?(parentVNode: VNode, parentDOM: Element, dom: Element): void;

getChildContext?(): void;

getSnapshotBeforeUpdate?(prevProps: Readonly<{ children?: Inferno.InfernoNode } & P>, prevState: S): any;
Expand Down Expand Up @@ -130,21 +132,23 @@ export interface ForwardRef<P, T> extends Inferno.StatelessComponent<P> {
}

export interface Refs<P> {
onComponentDidMount?: (domNode: Element | null, nextProps: Readonly<P>) => void;
onComponentDidMount?: (domNode: Element | null, nextProps: Readonly<{ children?: Inferno.InfernoNode } & P>) => void;

onComponentWillMount?(props: Readonly<{ children?: Inferno.InfernoNode } & P>): void;

onComponentWillMount?(): void;
onComponentShouldUpdate?(lastProps: Readonly<{ children?: Inferno.InfernoNode } & P>, nextProps: Readonly<{ children?: Inferno.InfernoNode } & P>): boolean;

onComponentShouldUpdate?(lastProps: Readonly<P>, nextProps: Readonly<P>): boolean;
onComponentWillUpdate?(lastProps: Readonly<{ children?: Inferno.InfernoNode } & P>, nextProps: Readonly<{ children?: Inferno.InfernoNode } & P>): void;

onComponentWillUpdate?(lastProps: Readonly<P>, nextProps: Readonly<P>): void;
onComponentDidUpdate?(lastProps: Readonly<{ children?: Inferno.InfernoNode } & P>, nextProps: Readonly<{ children?: Inferno.InfernoNode } & P>): void;

onComponentDidUpdate?(lastProps: Readonly<P>, nextProps: Readonly<P>): void;
onComponentWillUnmount?(domNode: Element, nextProps: Readonly<{ children?: Inferno.InfernoNode } & P>): void;

onComponentWillUnmount?(domNode: Element, nextProps: Readonly<P>): void;
onComponentDidAppear?(domNode: Element, props: Readonly<{ children?: Inferno.InfernoNode } & P>): void;

onComponentDidAppear?(domNode: Element): void;
onComponentWillDisappear?(domNode: Element, props: Readonly<{ children?: Inferno.InfernoNode } & P>, callback: Function): void;

onComponentWillDisappear?(domNode: Element, callback: Function): void;
onComponentWillMove?(parentVNode: VNode, parentDOM: Element, dom: Element, props: Readonly<{ children?: Inferno.InfernoNode } & P>): void;
}

export interface Props<T> {
Expand Down

0 comments on commit 25b6ad1

Please sign in to comment.