Skip to content

Duplicated Y.Doc lookup/existence-check logic in collection-ops.ts and tokens.ts needs shared helpers #266

Description

@brylie

Two independent, unrelated instances of the same smell — lookup/traversal logic re-implemented in more than one place instead of factored into a shared helper:

  1. src/lib/data/collection-ops.ts: repairEmbeddedViewsAfterPropertyRemoval (:397-420) and repairEmbeddedViewsAfterOptionRemoval (:624-643) each independently re-implement an identical full-scan of recordsMap(doc) to find every collection_view block referencing a given collectionId — verbatim-identical guard clauses at :402-404 and :630-632, differing only in what happens after the match (filters vs. visibleProperties/groupBy/swimlaneBy repair).
  2. src/lib/services/tokens.ts: documentExists/collectionExists (:54-63) reimplements "resolve this id, check what kind of record it is" nearly identically to src/lib/data/record-ops.ts's validateReferenceTarget (:95-110), just without the access check — two independent existence-check implementations instead of one shared helper with an optional access-check parameter.

Surfaced during the 2026-09-12 architecture audit (code-level scan of src/lib/data and src/lib/services).

  • Extract a shared findCollectionViewBlocksFor(doc, collectionId) helper in collection-ops.ts, used by both repair functions
  • Factor tokens.ts's existence check to call (or share logic with) record-ops.ts's validateReferenceTarget, rather than maintaining a second implementation
  • No behavior change — existing tests for both repair functions and for token creation continue passing unmodified

Done when: neither duplicated pattern has a second, independently-maintained copy — a grep for the guard-clause pattern in collection-ops.ts, and for the existence-check logic in tokens.ts, each return exactly one implementation site.

Priority: P2 — internal consistency, no user-facing bug. Added to 0.5.0 — Stability, Reliability, and QA as part of this release's data/service-layer consolidation work alongside #212.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions