Conversation
Captures the architectural decisions reached during the planning session: goal scope, public API redesign (class + typed event emitter), distribution shape (npm + unpkg/jsdelivr, no committed dist), build toolchain (Rollup, ESM-only, target ES2022), source module layout, strict mode policy, test strategy (Vitest + Playwright), branch model, and the deliverable PR sequence into the v3 branch.
Replaces the legacy build pipeline (module-generator.js + uglifyjs) with a modern TypeScript-first setup, ahead of the v3 rewrite landing in subsequent PRs into this branch. - package.json: type:module, version 3.0.0-dev, ESM-only exports map, files allow-list, new scripts (build/dev/typecheck/lint/test/test:unit/ test:e2e/format/release). - Replace deps: drop cypress, @testing-library/cypress, eslint-plugin-cypress, cypress-mcp, uglify-js. Add typescript, rollup + plugins (typescript, node-resolve, terser, dts), tslib, vitest, happy-dom, @playwright/test, typescript-eslint, @types/node. - tsconfig.json: strict + noUncheckedIndexedAccess, target ES2022, moduleResolution bundler, declaration + sourcemap, rootDir src. - rollup.config.js: two-pass (JS bundle + .d.ts bundle), banner inlined (lifted from dragster-comment.js), terser-minified ESM output. - vitest.config.ts: happy-dom env, src/**/*.test.ts. - playwright.config.ts: Chromium + Firefox + WebKit projects, webServer runs build + serve on port 8370. - eslint.config.js: ESM, drop eslint-plugin-cypress, add typescript-eslint with type-aware rules for src/, ignore legacy 2.x files until PR 9. - scripts/bump-version.js: ESM with import.meta.url for __dirname. - .husky hooks: commit-msg validates Conventional Commits format always, auto-bumps version only on master and only for feat/fix/breaking; post-commit auto-tags only on master and skips when no version change; pre-push runs typecheck + lint + unit tests (replaces removed test:headless cypress runner). - .github/workflows/ci.yml: typecheck, lint, unit, build, then Playwright cross-browser e2e on PR. - src/index.ts: stub class so the build produces a valid dist artefact; real implementation lands in PRs 2-7. - .gitignore: dist, coverage, playwright-report, .DS_Store; drop cypress paths.
Lifts the pure DOM helpers tangled inside dragster-script.js into a small
self-contained module that other PRs in the v3 series can depend on. No
module-level state; every export is either pure or a single, predictable
side effect.
Exports:
- findAncestor(start, predicate, stopAt?) — replaces getElement's recursive
walk with an iterative version. The optional stopAt boundary preserves the
"do not cross a non-drag-only region" semantics the original encoded
inline.
- insertBefore / insertAfter — null-safe wrappers around parentNode.insertBefore.
- removeNode — accepts null/undefined and detached nodes; uses Element.remove()
rather than parentNode.removeChild().
- getEventPoint(event) + isTouchEvent(event) — normalize the changedTouches[0]
vs MouseEvent branch the original repeated 3+ times in mousedown/mousemove/mouseup.
isTouchEvent is a typed user-defined type guard.
- createElement<K>(tag, { classes, dataset, style }) — eliminates the
document.createElement(DIV) + classList.add + dataset assignment boilerplate
that the placeholder/shadow/wrapper/temp-container creators all repeated.
- getVerticalMargins — parses computed marginTop + marginBottom into numbers
with a 0 fallback for NaN. Used by regions for height calculations.
Tests: 26 cases in src/dom.test.ts under happy-dom. v8 coverage on dom.ts is
100% statements / 100% branches / 100% functions / 100% lines.
Adds @vitest/coverage-v8@^2 (pinned to match vitest@2; v4 of the coverage
provider requires vitest@4 which we are not on).
Replaces the seven onBefore*/onAfter* callback options from the 2.x options
bag with a single typed pub/sub surface. Public consumers will hit this via
dragster.on('beforeDragStart', info => …) once the public class is wired in
PR 7; for now the module is internal infrastructure for state-machine.ts.
Module exports:
- DragsterEventInfo: payload shape mirroring legacy event.dragster, so
consumer code reading event.dragster.drag.node etc. ports across with
identical field paths.
- DragsterEventMap / DragsterEventName: discriminated event-name → payload
mapping. New events can be added without breaking existing subscribers.
- DragsterListener<E>: void | false return type encodes the cancellation
contract (return false from a before* listener to cancel; after* listeners
are fire-and-forget).
- createDefaultEventInfo(): factory producing a fresh nulled DragsterEventInfo
on each call. Replaces the 2.x JSON.parse(JSON.stringify(defaults)) deep
clone trick.
- EventEmitter<EventMap>: minimal generic emitter. on/off/emit/
removeAllListeners/listenerCount. Cancellation aggregates: every listener
runs regardless, emit returns false if any returned false. Iteration is
snapshot-safe — listeners added or removed during emit() do not affect the
in-flight emission.
EventMap is left unconstrained on the generic so it accepts both interface
and type-alias shapes. Constraining to Record<string, unknown> rejected
DragsterEventMap (interfaces lack implicit string index signatures).
Tests: 23 cases in src/events.test.ts. v8 coverage on events.ts is 100% on
all four metrics. The "type-level (compile-time)" describe block uses
@ts-expect-error to assert that unknown event names, wrong payload shapes,
and listeners returning non-(void|false) values fail to compile;
expectTypeOf checks payload inference inside listener bodies.
First behavioural module of the v3 rewrite. Resolves the deferred
layout-thrashing concern from the earlier audit (recorded in
project_dragster_audit memory).
The legacy updateRegionsHeight already separated reads from writes via
.map() then .forEach(), but the contract was implicit and one careless
refactor away from being broken. Hundreds of draggable elements per
region — multiplied by every pointer-move tick — make it worth pinning
the contract down with a test.
Module exports:
- RegionTracker class
- constructor(config) discovers regions and tags them with the internal
CLASS_REGION class and the instance's data-dragster-id stamp.
- refresh() re-runs discovery; nodes that no longer match the selector
get untagged, new matches get tagged, the rest stay tagged.
- getRegions() returns a readonly snapshot for consumer iteration.
- updateHeights() is the layout-thrash-safe path: phase 1 reads every
element's offsetHeight + computed margins into a buffer, phase 2
applies all style.height writes from the buffer. No DOM read happens
after the first DOM write within a single call. Empty regions keep
their existing height (matches legacy 2.x semantics).
- destroy() untags every tracked region while preserving any inline
style.height set during the lifetime, and only clears its own
data-dragster-id stamp (a different instance taking ownership of
the same node is left intact).
- isRegionBoundary(element, dragOnlyClass) — replaces the
region-but-not-drag-only stopAt check the legacy getElement walked
inline. Lets state-machine.ts pass it as the stopAt argument to
findAncestor without leaking the internal CLASS_REGION constant.
CLASS_REGION ('dragster-drag-region') stays module-private; consumers
go through isRegionBoundary or look at the tracker output.
Tests: 17 cases in src/regions.test.ts. v8 coverage on regions.ts is
100% statements / 100% functions / 100% lines / 96.3% branches (the
uncovered branch is the defensive measurements[i] !== undefined check
that the buffer length contract makes unreachable). The
"layout-thrash batching" describe block intercepts each item's
offsetHeight getter and each region's style.height setter, then
asserts lastIndexOf('read') < indexOf('write') across the full op
trace — the test fails the moment any future refactor interleaves a
read after a write.
Owns three concerns: pointer-mouse/touch normalisation, shadow element create/position/cleanup, and lifecycle event emission with cancellation. Placeholder positioning, scroll-on-edge, and the actual reorder are out of scope for this module — they wire in via the `onDrop` injection point or via subscribing to lifecycle events directly. State model is two-valued (`idle` / `dragging`). The legacy transient states (`picking`, `dropping`) are not modelled because pickup and drop are atomic in the rewrite; there is no async work between phases. 40 vitest tests cover every transition, cancellation path, touch normalisation, destroy semantics, and region rebind including mid-drag.
…croll placeholder.ts: PlaceholderManager owns the drop-slot indicator's lifetime. update() takes a pointer target + cursor Y and converges on a single DOM state — top/bottom relative to a draggable, or appended to a region. Repeat calls with the same input perform zero DOM mutations after the first insert. Cleaner than legacy: placeholder lands as a SIBLING of the wrapper rather than nested inside it, simplifying the drop reorder PR 7 will wire in. scroll.ts: autoScroll(clientY) reads window.innerHeight on each call (no cached value) and steps the viewport when the cursor sits within SCROLL_EDGE_THRESHOLD (60px) of an edge. The constants are named exports so consumers and tests can refer to them by name. Both modules deliberately leave the wiring to PR 7. Replace-elements mode is also out of scope here — it shifts the placeholder semantics enough that bundling it in would obscure the basic decision tree this PR establishes. 34 new vitest tests (25 placeholder + 9 scroll) at 100% coverage on both modules.
Composes RegionTracker, PlaceholderManager, StateMachine, and autoScroll
into the typed Dragster class promised by the §2.4 surface. Construction
wraps user items into internal dragster-draggable wrappers stamped with a
per-instance id, sets up tracking, and arms pointer-down listeners.
on()/off() delegate to the EventEmitter; update()/updateRegions() pick up
newly-added DOM; destroy() tears everything down idempotently.
Drop reorder: at pointer-up, the dragged wrapper takes the placeholder's
slot via insertBefore + clear. Cleaner than legacy 2.x — no per-mode
dispatch (move/replace/clone deferred), no nested-inside-wrapper
placeholder placement, no separate dropActions module.
Two state-machine extensions land alongside:
- onMove hook symmetric with onDrop, exposing live cursor coordinates
(the public event payload doesn't carry them) so the orchestrator
can drive elementFromPoint and autoScroll;
- defensive copy of the regions array so RegionTracker.refresh's
in-place mutation doesn't strand listener handles on removed regions.
Shadow gets pointer-events:none so elementFromPoint always sees through it.
163 vitest tests across 7 files, 100% lines coverage on every module.
index.html now loads ./dist/dragster.js as an ES module and constructs
each Dragster instance with `new Dragster(...)`. The clone-elements (d0)
and replace-elements (d3) options are dropped from the demo init since
those modes are deferred to a follow-up PR; the static markup stays in
place so the diff stays small when they land.
Replaces cypress/e2e/dragster.cy.js with tests/e2e/dragster.spec.ts:
basic-move, source-region-height, post-drop cleanup, and drag-only-region
non-acceptance — passing on all three Playwright projects (Chromium,
Firefox, WebKit). Replace and clone specs are recorded as test.fixme
markers so the matrix stays visible. The legacy Cypress directory
remains for now and gets deleted in PR 9.
Two supporting changes:
- tsconfig.json now includes tests/. Build-time config split into
tsconfig.build.json so rollup keeps its src-only rootDir.
- src/index.ts handleMove walks up from elementFromPoint to the
nearest HTMLElement. Without this, a cursor over an inline `<svg>`
descendant (which lives inside several .dragster-block items in the
demo) hands a non-HTMLElement to the placeholder logic and the drop
slot never updates.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.