From 49a4b1b7bcc8874ffc51f578943b5c954ddd3f19 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Wed, 5 Aug 2026 20:46:24 +0200 Subject: [PATCH 1/3] Mock Pragmatic lazily in the Stimulus specs The Angular unit-test builder bundles each spec into a __commonJS wrapper, so Vitest's hoist validator walks that wrapper instead of our source and reports top-level vi.mock calls as nested. Only mock, unmock and hoisted are hoist-checked, so vi.doMock sidesteps it. These specs already import the mocked packages and the controller under test lazily in beforeAll, which is the ordering doMock needs. A note above each call records that, since doMock's position above the imports no longer carries the guarantee vi.mock's hoisting gave it. The root spec also claimed it read back the combine, prevent-unhandled and drag-preview options. It never did; they keep the mounted item controller's side effects inert. --- .../dynamic/sortable-lists.controller.spec.ts | 19 ++++++++++--------- .../sortable-lists/item.controller.spec.ts | 10 ++++++---- .../sortable-lists/list.controller.spec.ts | 4 +++- .../scrollable.controller.spec.ts | 2 +- 4 files changed, 20 insertions(+), 15 deletions(-) diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts index 76c264e3aa49..2ae29b583f04 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts @@ -26,31 +26,32 @@ // See COPYRIGHT and LICENSE files for more details. //++ -vi.mock('@atlaskit/pragmatic-drag-and-drop/element/adapter', () => ({ +// vi.doMock is not hoisted above imports, unlike vi.mock, so the subject +// below is imported dynamically further down, after these calls run. +vi.doMock('@atlaskit/pragmatic-drag-and-drop/element/adapter', () => ({ draggable: vi.fn(() => vi.fn()), dropTargetForElements: vi.fn(() => vi.fn()), monitorForElements: vi.fn(() => vi.fn()), })); -vi.mock('@atlaskit/pragmatic-drag-and-drop-auto-scroll/element', () => ({ +vi.doMock('@atlaskit/pragmatic-drag-and-drop-auto-scroll/element', () => ({ autoScrollForElements: vi.fn(() => vi.fn()), })); -// This spec mounts the real item controller, which pulls in these modules. -// Tests share one module registry (the runner does not isolate spec files), -// so importing the real versions here would leak into the item controller -// spec and break its spies. Mock them to keep the shared cache inert. -vi.mock('@atlaskit/pragmatic-drag-and-drop/combine', () => ({ +// This spec mounts the real item controller, which pulls in these three +// modules. Stub them so its Pragmatic side effects stay inert; nothing +// here reads them back. +vi.doMock('@atlaskit/pragmatic-drag-and-drop/combine', () => ({ combine: vi.fn((...cleanups:(() => void)[]) => vi.fn(() => { cleanups.forEach((cleanup) => cleanup()); })), })); -vi.mock('@atlaskit/pragmatic-drag-and-drop/prevent-unhandled', () => ({ +vi.doMock('@atlaskit/pragmatic-drag-and-drop/prevent-unhandled', () => ({ preventUnhandled: { start: vi.fn(), stop: vi.fn() }, })); -vi.mock('@atlaskit/pragmatic-drag-and-drop/element/set-custom-native-drag-preview', () => ({ +vi.doMock('@atlaskit/pragmatic-drag-and-drop/element/set-custom-native-drag-preview', () => ({ setCustomNativeDragPreview: vi.fn(), })); diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.spec.ts index 8bc94713becc..1d7b0832c14f 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.spec.ts @@ -26,26 +26,28 @@ // See COPYRIGHT and LICENSE files for more details. //++ -vi.mock('@atlaskit/pragmatic-drag-and-drop/combine', () => ({ +// vi.doMock is not hoisted above imports, unlike vi.mock, so the subject +// below is imported dynamically further down, after these calls run. +vi.doMock('@atlaskit/pragmatic-drag-and-drop/combine', () => ({ combine: vi.fn((...cleanups:(() => void)[]) => vi.fn(() => { cleanups.forEach((cleanup) => cleanup()); })), })); -vi.mock('@atlaskit/pragmatic-drag-and-drop/element/adapter', () => ({ +vi.doMock('@atlaskit/pragmatic-drag-and-drop/element/adapter', () => ({ draggable: vi.fn(() => vi.fn()), dropTargetForElements: vi.fn(() => vi.fn()), monitorForElements: vi.fn(() => vi.fn()), })); -vi.mock('@atlaskit/pragmatic-drag-and-drop/prevent-unhandled', () => ({ +vi.doMock('@atlaskit/pragmatic-drag-and-drop/prevent-unhandled', () => ({ preventUnhandled: { start: vi.fn(), stop: vi.fn(), }, })); -vi.mock('@atlaskit/pragmatic-drag-and-drop/element/set-custom-native-drag-preview', () => ({ +vi.doMock('@atlaskit/pragmatic-drag-and-drop/element/set-custom-native-drag-preview', () => ({ setCustomNativeDragPreview: vi.fn(), })); diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/list.controller.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/list.controller.spec.ts index 018b9a38d8f4..5a4fed5dcc2e 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/list.controller.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/list.controller.spec.ts @@ -26,7 +26,9 @@ // See COPYRIGHT and LICENSE files for more details. //++ -vi.mock('@atlaskit/pragmatic-drag-and-drop/element/adapter', () => ({ +// vi.doMock is not hoisted above imports, unlike vi.mock, so the subject +// below is imported dynamically further down, after these calls run. +vi.doMock('@atlaskit/pragmatic-drag-and-drop/element/adapter', () => ({ draggable: vi.fn(() => vi.fn()), dropTargetForElements: vi.fn(() => vi.fn()), monitorForElements: vi.fn(() => vi.fn()), diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/scrollable.controller.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/scrollable.controller.spec.ts index 09bc7bcb20d9..11645e71829e 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/scrollable.controller.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/scrollable.controller.spec.ts @@ -31,7 +31,7 @@ import { setupStimulusTest, type StimulusTestContext } from 'core-stimulus/test- import type ScrollableControllerType from './scrollable.controller'; import type { sortableItemData as sortableItemDataFn, SortableListsRoot } from './drag-and-drop'; -vi.mock('@atlaskit/pragmatic-drag-and-drop-auto-scroll/element', () => ({ +vi.doMock('@atlaskit/pragmatic-drag-and-drop-auto-scroll/element', () => ({ autoScrollForElements: vi.fn(() => vi.fn()), })); From 4fffba59cd6dcecf7e3f628388f34c2666175c60 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Wed, 5 Aug 2026 20:46:31 +0200 Subject: [PATCH 2/3] Load the sortable engine after its mock vi.doMock only affects later imports, so the statically imported createSortableRoot would have pulled in the real auto-scroll module first and the mock would have stopped applying silently. A type import plus a lazy binding restores the ordering in both engine specs, and drops vi.hoisted with it. The preview spec also loses the file-split rationale that cited a shared module registry, which isolate:true removed. The split itself stands: the sibling renders previews for real. --- .../sortable-lists-engine.preview.spec.ts | 26 ++++++++------ .../sortable-lists-engine.spec.ts | 34 +++++++++++-------- 2 files changed, 36 insertions(+), 24 deletions(-) diff --git a/frontend/src/common/drag-and-drop/sortable-lists-engine.preview.spec.ts b/frontend/src/common/drag-and-drop/sortable-lists-engine.preview.spec.ts index 22ead4d7e03f..ce1b72862165 100644 --- a/frontend/src/common/drag-and-drop/sortable-lists-engine.preview.spec.ts +++ b/frontend/src/common/drag-and-drop/sortable-lists-engine.preview.spec.ts @@ -29,21 +29,21 @@ // The drag preview's `getOffset` decides where the pointer sits on the // preview, and Pragmatic only hands it to the real `setCustomNativeDragPreview` // — nothing observable from the outside. So this file mocks that module and -// reads the options back, which the sibling engine spec cannot do: it renders -// previews for real. Separate file rather than a mock added there, because the -// suite runs with `isolate: false` and shares one module registry. +// reads the options back. It stays a separate file from the sibling engine +// spec because that one renders previews for real, and one file cannot both +// stub and exercise the same module. import { vi } from 'vitest'; import { NativeDragSimulation } from 'core-common/drag-and-drop/testing/native-drag-simulation'; -import { createSortableRoot } from './sortable-lists-engine'; +import type { createSortableRoot as createSortableRootFn } from './sortable-lists-engine'; -const { previewCalls } = vi.hoisted(() => ({ - previewCalls: [] as { - getOffset?:(args:{ container:HTMLElement }) => { x:number; y:number }; - }[], -})); +// `doMock` is not hoisted, so this initialises before the factory below runs +// and a plain const does the job `vi.hoisted()` used to. +const previewCalls:{ + getOffset?:(args:{ container:HTMLElement }) => { x:number; y:number }; +}[] = []; -vi.mock('@atlaskit/pragmatic-drag-and-drop/element/set-custom-native-drag-preview', () => ({ +vi.doMock('@atlaskit/pragmatic-drag-and-drop/element/set-custom-native-drag-preview', () => ({ setCustomNativeDragPreview: (options:{ getOffset?:(args:{ container:HTMLElement }) => { x:number; y:number }; }) => { @@ -51,9 +51,15 @@ vi.mock('@atlaskit/pragmatic-drag-and-drop/element/set-custom-native-drag-previe }, })); +let createSortableRoot:typeof createSortableRootFn; + describe('createSortableRoot drag preview offset', () => { let cleanupFns:(() => void)[] = []; + beforeAll(async () => { + ({ createSortableRoot } = await import('./sortable-lists-engine')); + }); + beforeEach(() => { previewCalls.length = 0; }); afterEach(() => { diff --git a/frontend/src/common/drag-and-drop/sortable-lists-engine.spec.ts b/frontend/src/common/drag-and-drop/sortable-lists-engine.spec.ts index f515b3b28b43..ed0ddcf6e350 100644 --- a/frontend/src/common/drag-and-drop/sortable-lists-engine.spec.ts +++ b/frontend/src/common/drag-and-drop/sortable-lists-engine.spec.ts @@ -32,23 +32,23 @@ import { centerOf, towardsEdgeOf, } from 'core-common/drag-and-drop/testing/native-drag-simulation'; -import { - createSortableRoot, - type SortableDropIntent, - type SortableDropTransaction, - type SortableSource, +import type { + createSortableRoot as createSortableRootFn, + SortableDropIntent, + SortableDropTransaction, + SortableSource, } from './sortable-lists-engine'; -const { autoScrollRegistrations } = vi.hoisted(() => ({ - autoScrollRegistrations: [] as { - element:Element; - getAllowedAxis:() => string; - cleanup:() => void; - cleaned:boolean; - }[], -})); +// `doMock` is not hoisted, so this initialises before the factory below runs +// and a plain const does the job `vi.hoisted()` used to. +const autoScrollRegistrations:{ + element:Element; + getAllowedAxis:() => string; + cleanup:() => void; + cleaned:boolean; +}[] = []; -vi.mock('@atlaskit/pragmatic-drag-and-drop-auto-scroll/element', () => ({ +vi.doMock('@atlaskit/pragmatic-drag-and-drop-auto-scroll/element', () => ({ autoScrollForElements: (args:{ element:Element; getAllowedAxis:() => string }) => { const entry = { element: args.element, @@ -62,6 +62,8 @@ vi.mock('@atlaskit/pragmatic-drag-and-drop-auto-scroll/element', () => ({ }, })); +let createSortableRoot:typeof createSortableRootFn; + const liveRegistrations = () => autoScrollRegistrations.filter((r) => !r.cleaned); function buildList(items:string[]):{ root:HTMLElement; rows:HTMLElement[] } { @@ -145,6 +147,10 @@ function buildCardGrid(items:string[], columns:number):{ root:HTMLElement; cards describe('createSortableRoot', () => { let cleanupFns:(() => void)[] = []; + beforeAll(async () => { + ({ createSortableRoot } = await import('./sortable-lists-engine')); + }); + beforeEach(() => { autoScrollRegistrations.length = 0; }); afterEach(() => { From 0899bb2422bedcf5590aa08ca39b04abc1aaeb6d Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Wed, 5 Aug 2026 20:46:41 +0200 Subject: [PATCH 3/3] Model a scroll container in the DnD fixtures Pragmatic warns when auto-scroll attaches to an element whose computed overflow is visible. Production always resolves a real scroll ancestor, so the warning was accurate: the fixtures had none. A wrapper carrying overflow but no height satisfies the check without shifting the geometry these specs measure drop edges from. Where the overflow belongs differs. The directive walks up to the closest scrollable ancestor, so its hosts and the card-view root get a wrapper; the drag service and the preview spec register on the element they are handed, so that element takes it directly. Nested-root fixtures need a second, inner wrapper. Two independent root engines resolving one shared host collide on Pragmatic's element-keyed registry, which dedupes per root and not across them. The opSortableListsScrollContainer suite keeps its unscrollable hosts. It asserts that warning on purpose. --- .../wp-card-drag-and-drop.service.spec.ts | 8 +- .../sortable-lists.directive.behavior.spec.ts | 237 ++++++++++-------- .../drag-and-drop.service.spec.ts | 7 +- .../sortable-lists-engine.preview.spec.ts | 4 + 4 files changed, 143 insertions(+), 113 deletions(-) diff --git a/frontend/src/app/features/work-packages/components/wp-card-view/services/wp-card-drag-and-drop.service.spec.ts b/frontend/src/app/features/work-packages/components/wp-card-view/services/wp-card-drag-and-drop.service.spec.ts index abee1dce23fd..beda94c82f85 100644 --- a/frontend/src/app/features/work-packages/components/wp-card-view/services/wp-card-drag-and-drop.service.spec.ts +++ b/frontend/src/app/features/work-packages/components/wp-card-view/services/wp-card-drag-and-drop.service.spec.ts @@ -1002,9 +1002,11 @@ describe('WorkPackageCardDragAndDropService — real two-list directive fixture' standalone: true, imports: [OpSortableListsDirective, TestCardListHostComponent], template: ` -
- - +
+
+ + +
`, }) diff --git a/frontend/src/app/shared/directives/sortable-lists/sortable-lists.directive.behavior.spec.ts b/frontend/src/app/shared/directives/sortable-lists/sortable-lists.directive.behavior.spec.ts index cd78031781fc..4e5dca23b015 100644 --- a/frontend/src/app/shared/directives/sortable-lists/sortable-lists.directive.behavior.spec.ts +++ b/frontend/src/app/shared/directives/sortable-lists/sortable-lists.directive.behavior.spec.ts @@ -41,17 +41,9 @@ import { type SortableListsRemovedEvent, } from './sortable-lists.directive'; -// Deliberately does NOT `vi.mock` Pragmatic's autoscroll module: under the -// repo's `isolate:false` vitest config, spec files share a module registry -// within a worker, and this specifier is also `vi.mock`-ed (independently) -// by `sortable-lists-engine.spec.ts` for its own, unrelated engine-level -// tests. Two spec files independently mocking the same specifier crashes the -// shared worker (an unhandled "error when mocking a module" rejection, -// reproducible regardless of factory shape); the mock is also effectively -// global for the whole worker once registered, so even a "compatible" mock -// here would fight with that file's synthetic (non-real) replacement in -// combined runs. Instead this suite observes the REAL implementation's own -// documented side effects directly: +// Deliberately does NOT `vi.mock` Pragmatic's autoscroll module: this suite +// observes the REAL implementation's own documented side effects, which is +// what makes the scroll-container assertions below meaningful. // - the scroll-fallback tests below assert on `data-auto-scrollable`, the // attribute Pragmatic's real `autoScrollForElements` sets on registration // and removes on cleanup (see `@atlaskit/pragmatic-drag-and-drop-auto-scroll`'s @@ -71,20 +63,22 @@ describe('sortable-lists directive group behavior', () => { @Component({ imports: [OpSortableListsDirective, OpSortableListsItemDirective], template: ` -
- @for (entry of items(); track entry.id) { -
{{ entry.id }}
- } +
+
+ @for (entry of items(); track entry.id) { +
{{ entry.id }}
+ } +
`, }) @@ -225,10 +219,12 @@ describe('sortable-lists directive group behavior', () => { @Component({ imports: [OpSortableListsDirective, OpSortableListsListDirective, OpSortableListsItemDirective], template: ` -
- @for (id of ids(); track id) { -
{{ id }}
- } +
+
+ @for (id of ids(); track id) { +
{{ id }}
+ } +
`, }) @@ -291,29 +287,31 @@ describe('sortable-lists directive group behavior', () => { @Component({ imports: [OpSortableListsDirective, OpSortableListsListDirective, OpSortableListsItemDirective], template: ` -
-
- @for (id of idsA(); track id) { -
{{ id }}
- } -
+
+
+
+ @for (id of idsA(); track id) { +
{{ id }}
+ } +
-
- @for (id of idsB(); track id) { -
{{ id }}
- } +
+ @for (id of idsB(); track id) { +
{{ id }}
+ } +
`, @@ -409,22 +407,24 @@ describe('sortable-lists directive group behavior', () => { @Component({ imports: [OpSortableListsDirective, OpSortableListsListDirective, OpSortableListsItemDirective], template: ` -
- @if (explicit()) { -
+
+
+ @if (explicit()) { +
+ @for (id of ids(); track id) { +
{{ id }}
+ } +
+ } @else { @for (id of ids(); track id) {
{{ id }}
} -
- } @else { - @for (id of ids(); track id) { -
{{ id }}
} - } +
`, }) @@ -495,16 +495,23 @@ describe('sortable-lists directive group behavior', () => { }); describe('nested roots', () => { + // Two independent roots each need their own closest-scrollable-ancestor, + // or both walks terminate on the same element and Pragmatic ends up with + // two registrations on one node — see sortable-lists-engine.ts:142-149. @Component({ imports: [OpSortableListsDirective, OpSortableListsItemDirective], template: ` -
-
outer-a
- -
- @for (id of innerIds(); track id) { -
{{ id }}
- } +
+
+
outer-a
+ +
+
+ @for (id of innerIds(); track id) { +
{{ id }}
+ } +
+
`, @@ -548,27 +555,35 @@ describe('sortable-lists directive group behavior', () => { // list's id — DI's nearest-list lookup does not stop at the inner root, // so without the ownership check it would register against the inner // engine under an id that engine never registered a list for. + // + // The inner wrapper also gives the inner root its own scrollable + // ancestor, distinct from the outer root's, so the two don't collide on + // one Pragmatic registration — see sortable-lists-engine.ts:142-149. @Component({ imports: [OpSortableListsDirective, OpSortableListsListDirective, OpSortableListsItemDirective], template: ` -
-
+
+
- @for (id of innerIds(); track id) { -
{{ id }}
- } +
+
+ @for (id of innerIds(); track id) { +
{{ id }}
+ } +
+
@@ -637,10 +652,12 @@ describe('sortable-lists directive group behavior', () => { @Component({ imports: [OpSortableListsDirective, OpSortableListsListDirective, OpSortableListsItemDirective], template: ` -
- @for (id of ids(); track id) { -
{{ id }}
- } +
+
+ @for (id of ids(); track id) { +
{{ id }}
+ } +
`, }) @@ -778,20 +795,22 @@ describe('sortable-lists directive group behavior', () => { @Component({ imports: [OpSortableListsDirective, OpSortableListsItemDirective], template: ` -
- @for (id of ids(); track id) { -
{{ id }}
- } +
+
+ @for (id of ids(); track id) { +
{{ id }}
+ } +
`, }) @@ -804,10 +823,12 @@ describe('sortable-lists directive group behavior', () => { @Component({ imports: [OpSortableListsDirective, OpSortableListsItemDirective], template: ` -
- @for (id of ids(); track id) { -
{{ id }}
- } +
+
+ @for (id of ids(); track id) { +
{{ id }}
+ } +
`, }) diff --git a/frontend/src/app/shared/helpers/drag-and-drop/drag-and-drop.service.spec.ts b/frontend/src/app/shared/helpers/drag-and-drop/drag-and-drop.service.spec.ts index 90732497014a..381b7e72e4ec 100644 --- a/frontend/src/app/shared/helpers/drag-and-drop/drag-and-drop.service.spec.ts +++ b/frontend/src/app/shared/helpers/drag-and-drop/drag-and-drop.service.spec.ts @@ -60,7 +60,10 @@ function buildRow(id:string, opts:{ handle?:boolean } = {}):HTMLElement { function buildContainer(ids:string[], opts:{ handle?:boolean } = {}):{ container:HTMLElement; rows:HTMLElement[] } { const container = document.createElement('div'); - container.style.cssText = 'width:200px;'; + // `overflow-y` with no height keeps the container a scroll container for + // Pragmatic's computed-overflow check. This also computes overflow-x to + // `auto`, but rows are exactly 200px wide in a 200px container. + container.style.cssText = 'width:200px; overflow-y:auto;'; const rows = ids.map((id) => { const row = buildRow(id, opts); container.appendChild(row); @@ -159,7 +162,7 @@ describe('DragAndDropService', () => { describe('container append', () => { it('reports a drop below the rows as a null-target append intent', async () => { const { container, rows } = buildContainer(['a0', 'a1']); - container.style.cssText = 'width:200px; padding-bottom:40px;'; + container.style.cssText = 'width:200px; padding-bottom:40px; overflow-y:auto;'; const onMoved = vi.fn(); service.register(buildMember(container, { onMoved })); diff --git a/frontend/src/common/drag-and-drop/sortable-lists-engine.preview.spec.ts b/frontend/src/common/drag-and-drop/sortable-lists-engine.preview.spec.ts index ce1b72862165..2654b58a332b 100644 --- a/frontend/src/common/drag-and-drop/sortable-lists-engine.preview.spec.ts +++ b/frontend/src/common/drag-and-drop/sortable-lists-engine.preview.spec.ts @@ -70,6 +70,10 @@ describe('createSortableRoot drag preview offset', () => { function setup():{ rows:HTMLElement[] } { const root = document.createElement('div'); + // The engine attaches auto-scroll to this element directly, with no + // ancestor walk, so the overflow has to sit here. This also computes + // overflow-x to auto; 600px rows just happen to fit the viewport. + root.style.cssText = 'overflow-y:auto;'; const rows = ['a', 'b'].map((id) => { const row = document.createElement('div'); row.style.cssText = 'height:40px; width:600px;';