From c86c23748e32aea561061e94e017489b65c7c296 Mon Sep 17 00:00:00 2001 From: Reyo Date: Sat, 22 Aug 2026 16:22:52 +0530 Subject: [PATCH] fix(swap-animation): guard against undefined elements when sorting into empty containers --- .../fix-swap-animation-empty-container.md | 5 + src/Plugins/SwapAnimation/SwapAnimation.ts | 18 +- .../SwapAnimation/tests/SwapAnimation.test.ts | 249 ++++++++++++++++++ 3 files changed, 270 insertions(+), 2 deletions(-) create mode 100644 .changeset/fix-swap-animation-empty-container.md create mode 100644 src/Plugins/SwapAnimation/tests/SwapAnimation.test.ts diff --git a/.changeset/fix-swap-animation-empty-container.md b/.changeset/fix-swap-animation-empty-container.md new file mode 100644 index 00000000..e4c37a6c --- /dev/null +++ b/.changeset/fix-swap-animation-empty-container.md @@ -0,0 +1,5 @@ +--- +'@shopify/draggable': patch +--- + +Fix unhandled TypeError in SwapAnimation when sorting into an empty container or when over element is undefined. diff --git a/src/Plugins/SwapAnimation/SwapAnimation.ts b/src/Plugins/SwapAnimation/SwapAnimation.ts index 87acf298..7f5f1d6e 100644 --- a/src/Plugins/SwapAnimation/SwapAnimation.ts +++ b/src/Plugins/SwapAnimation/SwapAnimation.ts @@ -88,8 +88,16 @@ export default class SwapAnimation extends AbstractPlugin { */ @AutoBind onSortableSorted({oldIndex, newIndex, dragEvent}: FixMeAny) { + if (!dragEvent) { + return; + } + const {source, over} = dragEvent; + if (!source || !over) { + return; + } + if (this.lastAnimationFrame) { cancelAnimationFrame(this.lastAnimationFrame); } @@ -120,6 +128,10 @@ function animate( to: HTMLElement, {duration, easingFunction, horizontal}: Options, ) { + if (!from || !to || !isHTMLElement(from) || !isHTMLElement(to)) { + return; + } + for (const element of [from, to]) { element.style.pointerEvents = 'none'; } @@ -161,6 +173,8 @@ function resetElementOnTransitionEnd(event: Event) { ); } -function isHTMLElement(eventTarget: EventTarget): eventTarget is HTMLElement { - return Boolean('style' in eventTarget); +function isHTMLElement( + eventTarget: EventTarget | null | undefined, +): eventTarget is HTMLElement { + return Boolean(eventTarget && 'style' in eventTarget); } diff --git a/src/Plugins/SwapAnimation/tests/SwapAnimation.test.ts b/src/Plugins/SwapAnimation/tests/SwapAnimation.test.ts new file mode 100644 index 00000000..a6e22902 --- /dev/null +++ b/src/Plugins/SwapAnimation/tests/SwapAnimation.test.ts @@ -0,0 +1,249 @@ +import {createSandbox, waitForRequestAnimationFrame, DRAG_DELAY} from 'helper'; +import {FixMeAny} from 'shared/types'; + +/* eslint-disable @typescript-eslint/ban-ts-comment */ +// @ts-ignore +import Sortable from '../../../Sortable'; +/* eslint-enable @typescript-eslint/ban-ts-comment */ +import SwapAnimation, {defaultOptions} from '../SwapAnimation'; + +const sampleMarkup = ` + + +`; + +describe('SwapAnimation', () => { + let sandbox: HTMLElement; + let containers: HTMLElement[]; + let sortable: FixMeAny; + let firstItem: HTMLElement; + let secondItem: HTMLElement; + let emptyContainer: HTMLElement; + + beforeEach(() => { + sandbox = createSandbox(sampleMarkup); + containers = Array.from(sandbox.querySelectorAll('.Container')); + firstItem = sandbox.querySelector('.Item--1') as HTMLElement; + secondItem = sandbox.querySelector('.Item--2') as HTMLElement; + emptyContainer = sandbox.querySelector('.Container--empty') as HTMLElement; + + sortable = new Sortable(containers, { + draggable: '.Item', + delay: DRAG_DELAY, + plugins: [SwapAnimation], + }); + }); + + afterEach(() => { + sortable.destroy(); + sandbox.remove(); + }); + + it('initializes with default options', () => { + const plugin = sortable.plugins.find( + (pluginInstance: FixMeAny) => pluginInstance instanceof SwapAnimation, + ); + expect(plugin).toBeDefined(); + expect(plugin.options).toMatchObject(defaultOptions); + }); + + it('accepts custom options passed through Sortable', () => { + sortable.destroy(); + + sortable = new Sortable(containers, { + draggable: '.Item', + delay: DRAG_DELAY, + plugins: [SwapAnimation], + swapAnimation: { + duration: 300, + easingFunction: 'linear', + horizontal: true, + }, + }); + + const plugin = sortable.plugins.find( + (pluginInstance: FixMeAny) => pluginInstance instanceof SwapAnimation, + ); + expect(plugin.options).toMatchObject({ + duration: 300, + easingFunction: 'linear', + horizontal: true, + }); + }); + + it('attaches and detaches listeners on Sortable', () => { + const plugin = sortable.plugins.find( + (pluginInstance: FixMeAny) => pluginInstance instanceof SwapAnimation, + ); + const onSortableSortedSpy = jest.spyOn(plugin, 'onSortableSorted'); + + plugin.detach(); + sortable.trigger({ + type: 'sortable:sorted', + oldIndex: 0, + newIndex: 1, + dragEvent: {source: firstItem, over: secondItem}, + }); + expect(onSortableSortedSpy).not.toHaveBeenCalled(); + + plugin.attach(); + sortable.trigger({ + type: 'sortable:sorted', + oldIndex: 0, + newIndex: 1, + dragEvent: {source: firstItem, over: secondItem}, + }); + expect(onSortableSortedSpy).toHaveBeenCalled(); + }); + + it('does not throw when dragging into an empty container where over is undefined', () => { + expect(() => { + // Simulate sorting into an empty container where over element is undefined + sortable.trigger({ + type: 'sortable:sorted', + oldIndex: 0, + newIndex: 0, + dragEvent: { + source: firstItem, + over: undefined, + overContainer: emptyContainer, + }, + }); + + waitForRequestAnimationFrame(); + }).not.toThrow(); + }); + + it('does not throw when dragEvent or source/over is missing or invalid', () => { + const plugin = sortable.plugins.find( + (pluginInstance: FixMeAny) => pluginInstance instanceof SwapAnimation, + ); + + expect(() => { + plugin.onSortableSorted({ + oldIndex: 0, + newIndex: 1, + dragEvent: undefined, + }); + + waitForRequestAnimationFrame(); + }).not.toThrow(); + + expect(() => { + plugin.onSortableSorted({ + oldIndex: 0, + newIndex: 1, + dragEvent: { + source: undefined, + over: secondItem, + }, + }); + + waitForRequestAnimationFrame(); + }).not.toThrow(); + + expect(() => { + plugin.onSortableSorted({ + oldIndex: 0, + newIndex: 1, + dragEvent: { + source: firstItem, + over: undefined, + }, + }); + + waitForRequestAnimationFrame(); + }).not.toThrow(); + }); + + it('animates elements vertically by default during sortable:sorted', () => { + sortable.trigger({ + type: 'sortable:sorted', + oldIndex: 0, + newIndex: 1, + dragEvent: { + source: firstItem, + over: secondItem, + }, + }); + + // 1st frame: triggers animate() -> sets pointerEvents='none' + waitForRequestAnimationFrame(); + + expect(firstItem.style.pointerEvents).toBe('none'); + expect(secondItem.style.pointerEvents).toBe('none'); + + // 2nd frame: triggers transition setup + waitForRequestAnimationFrame(); + + expect(firstItem.style.transition).toContain('transform'); + expect(secondItem.style.transition).toContain('transform'); + }); + + it('animates elements horizontally when horizontal option is true', () => { + sortable.destroy(); + + sortable = new Sortable(containers, { + draggable: '.Item', + delay: DRAG_DELAY, + plugins: [SwapAnimation], + swapAnimation: { + horizontal: true, + }, + }); + + sortable.trigger({ + type: 'sortable:sorted', + oldIndex: 1, + newIndex: 0, + dragEvent: { + source: secondItem, + over: firstItem, + }, + }); + + waitForRequestAnimationFrame(); + + expect(firstItem.style.pointerEvents).toBe('none'); + expect(secondItem.style.pointerEvents).toBe('none'); + + waitForRequestAnimationFrame(); + + expect(firstItem.style.transition).toContain('transform'); + expect(secondItem.style.transition).toContain('transform'); + }); + + it('cleans up styles when transitionend fires', () => { + sortable.trigger({ + type: 'sortable:sorted', + oldIndex: 0, + newIndex: 1, + dragEvent: { + source: firstItem, + over: secondItem, + }, + }); + + waitForRequestAnimationFrame(); + waitForRequestAnimationFrame(); + + // Trigger transitionend on the animated elements + const transitionEndEvent = new Event('transitionend', {bubbles: true}); + firstItem.dispatchEvent(transitionEndEvent); + secondItem.dispatchEvent(transitionEndEvent); + + expect(firstItem.style.transition).toBe(''); + expect(firstItem.style.pointerEvents).toBe(''); + expect(secondItem.style.transition).toBe(''); + expect(secondItem.style.pointerEvents).toBe(''); + + // Trigger transitionend with non-element or null target without errors + const customEvent = new CustomEvent('transitionend'); + Object.defineProperty(customEvent, 'target', {value: null}); + window.dispatchEvent(customEvent); + }); +});