Repository navigation
fix(sdk): keep .unique() on a field with a keyOnly relation - #2919
Merged
Merged
Conversation
The relation metadata overwrote the field's own unique with the value the relation type implies, so keyOnly always dropped an explicit .unique(). The field was deployed without a unique constraint, and a relation using it as toward.key was rejected as non-unique. processRelationMetadata now takes the field's modifiers and keeps an explicit unique; the toward.key check derives uniqueness the same way. Array fields keep their current behavior.
🦋 Changeset detectedLatest commit: 80f0150 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@tailor-platform/create-sdk
@tailor-platform/eslint-plugin-sdk
@tailor-platform/sdk
@tailor-platform/sdk-plugin-frontend
@tailor-platform/sdk-plugin-seed
@tailor-platform/sdk-plugin-setup
@tailor-platform/sdk-plugin-tailordb-erd
commit: |
This comment has been minimized.
This comment has been minimized.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
📖 Docs Quality & Consistency Check
✅ Recommended Actions
None — docs are consistent with the implementation and contain no user-facing internal-detail leaks.
⚠️ Consistency Issues
No consistency issues found.
📝 Reader-Perspective Issues
No reader-perspective issues found.
🔍 Details
Areas Checked
User-Facing Documentation:
packages/sdk/docs/services/tailordb/index.md(lines 218-231, 268-271).changeset/wet-days-throw.md
Implementation Files:
packages/sdk/src/parser/service/tailordb/relation.ts—processRelationMetadatasignature change and logicpackages/sdk/src/parser/service/tailordb/type-parser.ts— relation metadata applicationpackages/sdk/src/parser/service/tailordb/type-parser.test.ts— test coverage for the new behavior
Consistency Verification
Documentation Claims vs. Implementation:
-
keyOnly relations and uniqueness (docs line 229):
- Docs: "A
keyOnlyrelation does not make the field unique; add.unique()to the field to make it unique." - Implementation:
isUnique = relationTypes[rawRelation.type] === "1-1" || field.unique === true— keyOnly is not "1-1", so it only becomes unique iffield.unique === true✅
- Docs: "A
-
Nested relations in db.object() (docs line 231):
- Docs: "Inside a
db.object()that is not an array,.unique()and.index()on the inner field work as described in Index / Unique." - Implementation:
processRelationMetadatanow receivesfieldparameter with botharrayanduniqueproperties, and nested relation processing inapplyNestedRelationMetadatapreserves these modifiers ✅ - Tests:
"keeps .unique() and .index() set on the inner field"test verifies this behavior ✅
- Docs: "Inside a
-
toward.key uniqueness requirement (docs line 270):
- Docs: "A custom
toward.keymust be unique, through.unique()or a one-to-one relation." - Implementation:
validateRelationConfigcheckskeyIsUniqueby examining either the key field's relation metadata or its own.unique()modifier ✅ - Tests:
"accepts a relation to a key made unique by %s"test verifies that fields with.unique()on a keyOnly relation can be used astoward.key✅
- Docs: "A custom
-
Array fields (docs line 178, changeset):
- Docs: "Inside an array of objects (
db.object(..., { array: true })) the platform does not reject duplicate values, so both [.unique()and.index()] are rejected there with an error" - Implementation:
shouldSetUnique = !field.array && isUnique— unique is not set on array fields ✅ - Changeset: "An array field with a relation still drops
.unique()(reachable through.clone({ array: true })); this is left for a separate fix." — accurately describes current behavior ✅
- Docs: "Inside an array of objects (
-
Migration behavior (changeset):
- Changeset describes upgrade path with
MIGRATION_SCHEMA_CHECK_FAILED,tailor tailordb migration generate, andmigrate.tswithTODO()placeholders - This matches the documented migration workflow pattern in the "Index / Unique" section (line 182-184) ✅
- Changeset describes upgrade path with
Reader-Perspective Review
User-Facing Documentation Checked:
packages/sdk/docs/services/tailordb/index.md— No internal implementation details leaked; describes only user-observable behavior (relation types, modifiers, constraints).changeset/wet-days-throw.md— MentionskyselyTypePlugin,seedPlugin,db.pglite.ts,migrate.ts,MIGRATION_SCHEMA_CHECK_FAILED, andTODO(), all of which are user-facing APIs, files, and error codes that users interact with
Internal Code Changes:
- JSDoc changes in
packages/sdk/src/parser/service/tailordb/relation.tswere reviewed but are not user-facing (parser module is internal, not re-exported throughpackage.json#exports)
Example Code:
example/tailordb/salesOrder.tsuseskeyOnlyon an array field, which is unaffected by this PR
Re-run this check by adding the
docs-checklabel to the PR.
toiroakr
reviewed
Nov 3, 2026
| inner.rawRelation, | ||
| innerContext, | ||
| inner.array, | ||
| inner, |
Contributor
There was a problem hiding this comment.
This change does not alter behavior. applyNestedRelationMetadata only reads foreignKey, foreignKeyType and foreignKeyField from the result and spreads inner, so the inner unique and index are kept either way.
Reverting this call to { array: inner.array } leaves all 82 tests in type-parser.test.ts passing, including the new nested test, so that test cannot catch a regression.
Please drop this call change and the nested test, and reduce the docs paragraph (index.md:231) to one sentence: a relation adds only the foreign key to an inner field, and .unique() / .index() set on it work as in Index / Unique.
toiroakr
reviewed
Nov 3, 2026
| expect(result.Profile!.fields.userId!.config.unique).toBe(true); | ||
| }); | ||
|
|
||
| test.each([ |
Contributor
There was a problem hiding this comment.
Optional, can be a separate PR: this test only checks config.unique, and nothing checks that the PGlite DDL gets UNIQUE.
Please add a keyOnly field with .unique() to the "relations written in the table definition" block in pglite-schema.test.ts and assert that the DDL contains UNIQUE.
toiroakr
approved these changes
Nov 3, 2026
toiroakr
left a comment
toiroakr
left a comment
Contributor
There was a problem hiding this comment.
LGTM overall. See inline comments.
Contributor
Code Metrics Report (packages/sdk)Complexity change of this pull request: cognitive +1, cyclomatic +1
Compared with the base commit of this pull request.
Details | | main (04dc1c7) | #2919 (cbfc8a5) | +/- |
|--------------------|----------------|-----------------|------|
| Coverage | 89.2% | 89.2% | 0.0% |
| Files | 578 | 578 | 0 |
| Lines | 25815 | 25815 | 0 |
| Covered | 23035 | 23035 | 0 |
+ | Code to Test Ratio | 1:0.5 | 1:0.5 | +0.0 |
| Code | 212577 | 212602 | +25 |
+ | Test | 120407 | 120432 | +25 |Code coverage of files in pull request scope (90.3% → 90.3%, patch 100.0%)
SDK Configure Bundle Size
Runtime Performance
Type Performance (instantiations)
Reported by octocov |
Open
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
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.
Summary
.unique()on a field with akeyOnlyrelation was silently dropped, so the field was deployed without a unique constraint. The field now keeps it.What changes
db.uuid().unique().relation({ type: "keyOnly", ... })used to deploy the field without a unique constraint; the constraint is now applied, and the PGlite and seed schemas declare it too.toward.keypoints at such a field used to fail withreferences "User.accountID", which must be unique.; it is now accepted.keyOnlyfield without.unique()stays non-unique.Notes
keyOnlyand.unique()gains the unique constraint on its next deploy. The changeset describes the steps, includingMIGRATION_SCHEMA_CHECK_FAILEDand the generatedmigrate.tsin a namespace withmigration.directory.keyOnly+.unique()asn-1does was considered, but existing projects would then fail to load, and a foreign key on a unique field is consistent, so the field keeps.unique()..unique()(reachable through.clone({ array: true })); this is left for a separate fix.Verification
On disposable workspaces, deployed with
main(0c17d81), inserted twoMemberrows with the sameaccountID, then deployed this branch (287925a):migration.directory:tailor deployfailed withcould not create unique index "u_idx_Member_attribute_accountID" (SQLSTATE 23505). After deleting one duplicate it succeeded, and creating a duplicate then failed withkey already exists.migration.directory:tailor deploystopped withMIGRATION_SCHEMA_CHECK_FAILED(accountID: unique: false → true).tailor tailordb migration generate --yesreportedUnique constraint added to fieldand generatedmigrate.ts, and deploy then stopped withMIGRATION_SCRIPT_REVIEW_REQUIRED. WithTODO()replaced by a UUID not inAccount, the migration failed withviolates foreign key constraint; with an unusedAccountid, the deploy succeeded and a new duplicate failed withkey already exists.