From 3ce42f39ab89a61f6b1ae9b5afa497811380a701 Mon Sep 17 00:00:00 2001 From: Sebastian Ware Date: Sat, 22 Oct 2022 16:31:48 +0200 Subject: [PATCH 1/6] Improved specificity in types and removed an inconsistent passing of props to method on Class Component (componentWillMove) --- .../src/AnimatedAllComponent.ts | 8 +++--- .../src/AnimatedComponent.ts | 2 +- .../src/AnimatedMoveComponent.ts | 4 +-- packages/inferno-animation/src/animations.ts | 16 ++++++------ packages/inferno-animation/src/utils.ts | 26 +++++++++---------- 5 files changed, 28 insertions(+), 28 deletions(-) diff --git a/packages/inferno-animation/src/AnimatedAllComponent.ts b/packages/inferno-animation/src/AnimatedAllComponent.ts index 6fefae778..adc8d1199 100644 --- a/packages/inferno-animation/src/AnimatedAllComponent.ts +++ b/packages/inferno-animation/src/AnimatedAllComponent.ts @@ -7,15 +7,15 @@ type AnimationProp = { }; export abstract class AnimatedAllComponent

extends Component { - 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); } } diff --git a/packages/inferno-animation/src/AnimatedComponent.ts b/packages/inferno-animation/src/AnimatedComponent.ts index 894272dfd..41064951c 100644 --- a/packages/inferno-animation/src/AnimatedComponent.ts +++ b/packages/inferno-animation/src/AnimatedComponent.ts @@ -11,7 +11,7 @@ export abstract class AnimatedComponent

extends Component extends Component { - 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); } } diff --git a/packages/inferno-animation/src/animations.ts b/packages/inferno-animation/src/animations.ts index a26e8c568..6b9711919 100644 --- a/packages/inferno-animation/src/animations.ts +++ b/packages/inferno-animation/src/animations.ts @@ -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'); @@ -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 @@ -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); @@ -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 @@ -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); @@ -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, @@ -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; } } @@ -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); diff --git a/packages/inferno-animation/src/utils.ts b/packages/inferno-animation/src/utils.ts index fd1ab85e4..617960e4c 100644 --- a/packages/inferno-animation/src/utils.ts +++ b/packages/inferno-animation/src/utils.ts @@ -15,7 +15,7 @@ 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++) { @@ -23,7 +23,7 @@ export function addClassName(node: HTMLElement, className: string) { } } -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++) { @@ -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 { @@ -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) { @@ -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 @@ -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'; @@ -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 = ''; } @@ -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]; /** From 92b7b71dc8851b3c6c9cb110fd33d0b055372d01 Mon Sep 17 00:00:00 2001 From: Sebastian Ware Date: Sat, 22 Oct 2022 16:33:26 +0200 Subject: [PATCH 2/6] Added missing argument props --- packages/inferno/src/core/types.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/inferno/src/core/types.ts b/packages/inferno/src/core/types.ts index 9f7555f49..5e3cacb38 100644 --- a/packages/inferno/src/core/types.ts +++ b/packages/inferno/src/core/types.ts @@ -142,9 +142,10 @@ export interface Refs

{ onComponentWillUnmount?(domNode: Element, nextProps: Readonly

): void; - onComponentDidAppear?(domNode: Element): void; + onComponentDidAppear?(domNode: Element, props: Readonly

): void; + + onComponentWillDisappear?(domNode: Element, props: Readonly

, callback: Function): void; - onComponentWillDisappear?(domNode: Element, callback: Function): void; } export interface Props { From eadb41fdaa301a8ea849abee5669c68083e08b18 Mon Sep 17 00:00:00 2001 From: Sebastian Ware Date: Sat, 22 Oct 2022 16:38:40 +0200 Subject: [PATCH 3/6] Added type for missing hook componentWillMove --- packages/inferno/src/core/types.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/inferno/src/core/types.ts b/packages/inferno/src/core/types.ts index 5e3cacb38..e6da41217 100644 --- a/packages/inferno/src/core/types.ts +++ b/packages/inferno/src/core/types.ts @@ -45,6 +45,8 @@ export interface IComponent { 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; @@ -146,6 +148,7 @@ export interface Refs

{ onComponentWillDisappear?(domNode: Element, props: Readonly

, callback: Function): void; + onComponentWillMove?(parentVNode: VNode, parentDOM: Element, dom: Element, props: Readonly

): void; } export interface Props { From 4715e5feb926efc410994dace1746d022a80ac22 Mon Sep 17 00:00:00 2001 From: Sebastian Ware Date: Sat, 22 Oct 2022 16:41:23 +0200 Subject: [PATCH 4/6] 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. --- packages/inferno/src/DOM/utils/common.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/inferno/src/DOM/utils/common.ts b/packages/inferno/src/DOM/utils/common.ts index 9fef6e48f..fb74542bc 100644 --- a/packages/inferno/src/DOM/utils/common.ts +++ b/packages/inferno/src/DOM/utils/common.ts @@ -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); } @@ -206,7 +206,6 @@ export function moveVNodeDOM(parentVNode, vNode, parentDOM, nextNode, animations if (flags & VNodeFlags.ComponentClass) { refOrInstance = vNode.children; - instanceProps = vNode.props; vNode = children.$LI; } else if (flags & VNodeFlags.ComponentFunction) { refOrInstance = vNode.ref; From 4390203ed6c6942ce397c66a0368cb0579d27189 Mon Sep 17 00:00:00 2001 From: Sebastian Ware Date: Sun, 30 Oct 2022 11:02:36 +0100 Subject: [PATCH 5/6] Fix review comment "Props should have children property defined in its type" --- packages/inferno/src/core/types.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/inferno/src/core/types.ts b/packages/inferno/src/core/types.ts index e6da41217..b6c2960e7 100644 --- a/packages/inferno/src/core/types.ts +++ b/packages/inferno/src/core/types.ts @@ -132,23 +132,23 @@ export interface ForwardRef extends Inferno.StatelessComponent

{ } export interface Refs

{ - onComponentDidMount?: (domNode: Element | null, nextProps: Readonly

) => void; + onComponentDidMount?: (domNode: Element | null, nextProps: Readonly<{ children?: Inferno.InfernoNode } & P>) => void; - onComponentWillMount?(): void; + onComponentWillMount?(props: Readonly<{ children?: Inferno.InfernoNode } & P>): void; - onComponentShouldUpdate?(lastProps: Readonly

, nextProps: Readonly

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

, nextProps: Readonly

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

, nextProps: Readonly

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

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

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

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

): void; + onComponentWillMove?(parentVNode: VNode, parentDOM: Element, dom: Element, props: Readonly<{ children?: Inferno.InfernoNode } & P>): void; } export interface Props { From 502afcb02bc9c8bb3b6c025ff1b3ec6d6497e419 Mon Sep 17 00:00:00 2001 From: Sebastian Ware Date: Sun, 30 Oct 2022 11:13:39 +0100 Subject: [PATCH 6/6] Defer breaking change, enough that it is hinted in types --- packages/inferno/src/DOM/utils/common.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/inferno/src/DOM/utils/common.ts b/packages/inferno/src/DOM/utils/common.ts index fb74542bc..ca1c3058f 100644 --- a/packages/inferno/src/DOM/utils/common.ts +++ b/packages/inferno/src/DOM/utils/common.ts @@ -206,6 +206,8 @@ 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) { refOrInstance = vNode.ref;