-
-
Notifications
You must be signed in to change notification settings - Fork 28
fix: custom nullable column #419
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: fdfb9cb The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
WalkthroughA changeset records a patch release for the ESLint plugin that fixes nullable custom types in insert statements. Tests add a table and cases for nullable timestamptz mapped to a custom Instant type, and the type-resolution utility now extracts and resolves non-null members of X | null unions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Call as Caller
participant Util as getPgTypeFromTsType
participant Check as checkType
Note over Util: New branch for unions containing null
Call->>Util: resolve Type (Identifier/Union)
alt Type is Union and contains null and one non-null member
Util->>Check: checkType(nonNullMember)
Check-->>Util: resolved PG type
Util-->>Call: return PG type
else Other Union
Util-->>Call: Unsupported union error / previous handling
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/eslint-plugin/src/utils/ts-pg.utils.ts (1)
323-336: LGTM! Union handling correctly supports nullable types.The logic mirrors the Identifier case (lines 269-274) to handle
X | nullunions consistently across different type resolution paths.Optional refactor: Consider extracting the
X | nullhandling into a helper function to reduce duplication:function tryExtractNonNullFromUnion(types: ts.Type[]): ts.Type | null { const nonNullTypes = types.filter((t) => t.flags !== ts.TypeFlags.Null); return nonNullTypes.length === 1 ? nonNullTypes[0] : null; }Then use it in both locations:
// In Identifier case (line 269) if (TSUtils.isTsUnionType(symbolType)) { const nonNullType = tryExtractNonNullFromUnion(symbolType.types); if (nonNullType) { return checkType({ checker, type: nonNullType, options }); } return getPgTypeFromTsTypeUnion({ types: symbolType.types }); } // In union handling (line 323) if (TSUtils.isTsUnionType(type)) { const matchingType = type.types.find((t) => t.flags in tsFlagToPgTypeMap); if (matchingType) { return E.right({ kind: "cast", cast: tsFlagToPgTypeMap[matchingType.flags] }); } const nonNullType = tryExtractNonNullFromUnion(type.types); if (nonNullType) { return checkType({ checker, type: nonNullType, options }); } return E.left("Unsupported union type"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/stale-signs-bow.md(1 hunks)packages/eslint-plugin/src/rules/check-sql.test.ts(2 hunks)packages/eslint-plugin/src/utils/ts-pg.utils.ts(2 hunks)
🧰 Additional context used
🪛 LanguageTool
.changeset/stale-signs-bow.md
[style] ~4-~4: Consider using a different verb for a more formal wording.
Context: ... "@ts-safeql/eslint-plugin": patch --- fixed an issue with nullable custom types in ...
(FIX_RESOLVE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: check (17)
🔇 Additional comments (3)
.changeset/stale-signs-bow.md (1)
1-5: LGTM!The changeset clearly documents the patch release and the fix for nullable custom types in insert statements.
packages/eslint-plugin/src/rules/check-sql.test.ts (1)
148-150: LGTM!The new test table properly sets up a nullable TIMESTAMPTZ column for testing the nullable custom type scenario.
packages/eslint-plugin/src/utils/ts-pg.utils.ts (1)
269-277: LGTM! Core fix for nullable custom types.The logic correctly handles
X | nullunions by filtering out null and delegating tocheckTypewith the single non-null type. This allows custom type overrides (liketimestamptz: "Instant") to resolve properly for nullable parameters.
Summary by CodeRabbit
Bug Fixes
Tests
Chores