ReviewOS

stacks/bun-query-builder

Refactor: dedupe relation-normalization logic (meta.ts vs migrations.ts)

#1042
Closed glennmichael123 opened this 2 months ago ·
2 months ago

Summary

meta.ts (toRecord/toBelongsToManyRecord) and migrations.ts (normalizeBelongsTo) independently re-implement "unwrap string | { model } | array | record" for relation entries. This is the exact shape that caused #1023, and the fix had to be applied in two places; they've already diverged (meta handles all relation kinds, migrations only belongsTo), so the next such bug will likely be fixed in only one.

Suggested fix

Extract a single normalizeRelationEntry(entry): { model, foreignKey?, onDelete? } | null (+ toModelRecord) into a shared module and consume from both.

Severity: low/medium (maintainability). Found via internal code audit.

9 days ago

Fixed in 4051a01. Extracted the relation-entry unwrapping into src/relation-utils.ts (normalizeRelationEntry / normalizeRelationList) as a single source of truth; meta.ts's toRecord and migrations.ts's belongsTo normalization now both delegate to it (the #1023 fix no longer lives in two places). Behavior unchanged — existing migration/meta tests pass, plus a new unit test for the shared helpers. Full suite green.

Sign in to comment on this issue.