-
Notifications
You must be signed in to change notification settings - Fork 6
Infer rows #183
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
Infer rows #183
Conversation
- Removed the outdated version of @sinclair/typebox from dependencies. - Added an override for @sinclair/typebox to enforce the use of version 0.34.0. - Updated bun.lockb to reflect changes in package versions.
WalkthroughThis update refactors several type definitions across the codebase to use schema-inferred types, introduces new type utilities for insert models, and removes the Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant TypeSystem
participant Schema
Developer->>TypeSystem: Define NewRow type for Table
TypeSystem->>Schema: Use InferNewRow utility with Table schema
Schema-->>TypeSystem: Return inferred type (excluding system fields)
TypeSystem-->>Developer: Provides accurate, schema-aligned type
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
- Updated ArticleFeedRow, ArticleRow, NewArticleRow, CommentRow, and NewCommentRow interfaces to utilize the new InferNewRow type for better type inference and consistency. - Removed redundant properties from ArticleFeedRow and ArticleRow to streamline the data structure. - Enhanced type definitions across various interfaces to improve clarity and maintainability.
- Updated the ArticleRow type definition to omit the 'body' property, enhancing type clarity and reducing redundancy in the interface. - Maintained the inclusion of 'author', 'favoritedBy', and 'tags' properties for comprehensive article representation.
- Deleted the import statement for createSelectSchema and the associated tagsSelectSchema export, as they were no longer needed in the tags schema file, streamlining the codebase.
- Removed default values for 'bio' and 'image' fields in the users schema to allow for more flexible data input. - Updated the NewUserRow type to omit system fields, enhancing type clarity and ensuring better compatibility with the updated schema.
…ility type - Updated the InferNewRow type to accept a configuration object for database column names and an override option, enhancing type inference capabilities. - Introduced a new Prettify utility type to improve type representation across the codebase.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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: 0
🧹 Nitpick comments (5)
package.json (1)
39-39: Consider moving@sinclair/typeboxtodevDependencies
If@sinclair/typeboxis only used for static schema validation or type-level utilities and not needed at runtime, relocating it todevDependenciescan help keep your production bundle lean.src/common/interfaces/common.interface.ts (2)
43-60: Expose system field list as a constant for re-use at runtime
SystemFieldsis only a type alias.
In several places (serialization, audit-logging, object sanitisation, etc.) we often need the same list of keys at runtime. Exporting the literal array alongside the type avoids the usual duplication /as unknown asgymnastics while keeping the existing type alias intact.+export const SYSTEM_FIELDS = ['id', 'createdAt', 'updatedAt'] as const; +export type SystemFields = typeof SYSTEM_FIELDS[number]; -export type SystemFields = 'id' | 'createdAt' | 'updatedAt';
50-59: TConfig default contains a value for an optional key
overrideis declared optional (override?: boolean) but is still given a default offalse.
That makes the question “is the caller omitting the key” indistinguishable from “the caller setfalse”, so the optionality provides no extra information. Either:
- Drop
overridefrom the default, or- Make
overriderequired in the constraint.- override?: boolean; + override: boolean; ... - override: false;Pick whichever reflects the intended API.
src/comments/interfaces/comment-row.interface.ts (1)
1-4: Prefer path-alias import for consistencyElsewhere the project imports schemas via the
@/alias (import { comments } from '@/comments/comments.schema').
Using the same style here avoids future churn if folders are moved.-import type { comments } from '../comments.schema'; +import type { comments } from '@/comments/comments.schema';src/users/interfaces/user-row.interface.ts (1)
6-8:UpdateUserRowmay omit fields you sometimes need to patchUsing
Partial<NewUserRow>blocks updates tocreatedAt/updatedAtand any future system columns by design. If you ever issue administrative updates that legitimately touch those fields (e.g. backfilling data), consider:-export type UpdateUserRow = Partial<NewUserRow>; +export type UpdateUserRow = Partial< + Omit<InferSelectModel<typeof users>, 'id'> +>;This keeps
idimmutable but allows optional modification of all other columns when necessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (12)
package.json(1 hunks)src/articles/interfaces/article-feed-row.interface.ts(1 hunks)src/articles/interfaces/article-row.interface.ts(1 hunks)src/articles/interfaces/new-article-row.interface.ts(1 hunks)src/comments/interfaces/comment-row.interface.ts(1 hunks)src/comments/interfaces/new-comment-row.interface.ts(1 hunks)src/common/interfaces/common.interface.ts(2 hunks)src/tags/interfaces/tag-row.interface.ts(1 hunks)src/tags/tags.schema.ts(0 hunks)src/users/interfaces/user-follow-row.interface.ts(1 hunks)src/users/interfaces/user-row.interface.ts(1 hunks)src/users/users.schema.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/tags/tags.schema.ts
🧰 Additional context used
🧬 Code Graph Analysis (7)
src/comments/interfaces/comment-row.interface.ts (1)
src/comments/comments.schema.ts (1)
comments(6-17)
src/comments/interfaces/new-comment-row.interface.ts (2)
src/common/interfaces/common.interface.ts (1)
InferNewRow(50-59)src/comments/comments.schema.ts (1)
comments(6-17)
src/articles/interfaces/new-article-row.interface.ts (2)
src/common/interfaces/common.interface.ts (1)
InferNewRow(50-59)src/articles/articles.schema.ts (1)
articles(14-25)
src/users/interfaces/user-follow-row.interface.ts (2)
src/users/users.schema.ts (1)
userFollows(30-43)src/common/interfaces/common.interface.ts (1)
InferNewRow(50-59)
src/tags/interfaces/tag-row.interface.ts (2)
src/tags/tags.schema.ts (2)
tags(11-15)articleTags(23-36)src/common/interfaces/common.interface.ts (1)
InferNewRow(50-59)
src/articles/interfaces/article-row.interface.ts (3)
src/articles/articles.schema.ts (2)
articles(14-25)favoriteArticles(27-40)src/profiles/interfaces/profile-row.interface.ts (1)
ProfileRow(4-6)src/tags/interfaces/tag-row.interface.ts (1)
ArticleTagRow(8-8)
src/articles/interfaces/article-feed-row.interface.ts (1)
src/articles/articles.schema.ts (1)
articles(14-25)
🔇 Additional comments (8)
package.json (1)
38-38: Validate runtime necessity ofradashi
Ensure thatradashiis actually required in your production code. If it’s only used during development (e.g., for tooling or scripts), consider moving it todevDependencies.src/comments/interfaces/new-comment-row.interface.ts (1)
1-4: LGTM – switch toInferNewRowsimplifies maintenanceThe new alias cleanly removes system columns from the insert model and avoids duplication.
No further issues spotted.src/articles/interfaces/new-article-row.interface.ts (1)
2-4: LGTM – consistent with new typing strategy
InferNewRowis applied correctly; the article insert model now omits system fields.
Nothing else to flag.src/users/interfaces/user-follow-row.interface.ts (1)
1-10: Composite-PK table benefits from the new helper
userFollowshas noidcolumn, soInferNewRowconveniently drops only the timestamps.
Change looks sound and mirrors other modules.src/users/users.schema.ts (1)
15-16: Nullablebio/image— confirm downstream handlingRemoving the defaults and the
.notNull()constraint means both columns will now resolve tostring | nullat the type level and can storeNULLin the database. Double-check:
- All serializers / DTOs tolerate
null(e.g. JSON response for profiles).- Validation layers no longer assume empty-string defaults.
- Existing data migrations set
bio/imagetoNULLinstead of''to avoid silent casting issues.If any of these surfaces still expect a plain
string, consider keeping a default or adding explicitnotNull()constraints.src/tags/interfaces/tag-row.interface.ts (1)
1-10: NewInferNewRowadoption looks goodTransitioning to
InferNewRowcleanly dropscreatedAt/updatedAtfrom insert types while retaining the requirednamePK. No further action needed.src/articles/interfaces/article-row.interface.ts (1)
6-10: Consistent infer-basedArticleRowdefinitionRefactoring to
Omit<InferSelectModel<typeof articles>, 'authorId'>avoids manual drift and keeps the extra domain fields (author,favoritedBy,tags) explicit. Nice cleanup.src/articles/interfaces/article-feed-row.interface.ts (1)
4-7: Feed row inference is concise and accurateLeveraging
InferSelectModeland omitting only the unneeded columns (id,authorId,body) keeps the type aligned with the schema while preserving previously hand-written fields. Good move.
Description
Infer rows from schema instead of manually defining them
PR Checklist
bun docsSummary by CodeRabbit
Refactor
Chores
Style
Bug Fixes