indexer: update drizzle and drizzle sink #107
Conversation
📝 WalkthroughWalkthroughA new JSON file has been added for the Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
fracek
force-pushed
the
update-drizzle
branch
from
0fa740c to
68700fe
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
packages/indexer/src/sinks/drizzle/insert.ts (2)
7-7: LGTM! Consider consolidating type imports.The renaming of
PgInsertValuetoDrizzleInsertValueand the separate type import forPgInsertValuelook good. These changes align well with the PR objective of updating drizzle and resolving type issues.For better code organization, consider consolidating all type imports at the top of the file. You could move the
PgInsertValuetype import to be with the other type imports from 'drizzle-orm/pg-core'.Also applies to: 12-12
38-40: LGTM! Consider using a type guard for improved type safety.The changes look good. The use of
DrizzleInsertValueis consistent with the import changes, and the multi-line format improves readability.To improve type safety, consider using a type guard function instead of a type assertion. This would ensure that the
_cursorproperty is correctly added to all values at runtime. Here's an example:function addCursor<T extends object>(value: T, cursor: string | undefined): T & { _cursor: string | undefined } { return { ...value, _cursor: cursor }; } // In the values method: const cursoredValues = (Array.isArray(values) ? values : [values]).map( (v) => addCursor(v, getDrizzleCursor(this.endCursor?.orderKey)) ); return originalInsert.values(cursoredValues);This approach eliminates the need for the type assertion and provides better runtime type checking.
packages/indexer/package.json (1)
Line range hint
1-105: Summary: drizzle-orm version updateThe changes in this file are focused on updating the drizzle-orm dependency from version ^0.33.0 to ^0.35.2 in both devDependencies and peerDependencies. These updates are consistent with the PR objective to resolve type issues in new projects using the drizzle library.
Key points:
- The updates maintain consistency across different dependency sections.
- The minor version update (0.33 to 0.35) suggests new features and backwards-compatible changes.
Consider the following to ensure a smooth transition:
- Review the changelog for any new features or deprecations that might affect your codebase.
- Update any internal documentation or guides that reference the drizzle-orm version.
- Consider running a comprehensive test suite to catch any potential compatibility issues early.
packages/indexer/src/sinks/drizzle/utils.ts (1)
Line range hint
69-75: Consider renaming the custompgTablefunction to avoid confusionRedefining the
pgTablefunction name may cause confusion with the originalpgTableimported fromdrizzle-orm/pg-core. To enhance clarity and maintainability, consider renaming the custom function to something likepgTableWithCursor.Apply this diff to rename the function:
-export const pgTable: PgTableWithCursorFn = (name, columns, extraConfig?) => { +export const pgTableWithCursor: PgTableWithCursorFn = (name, columns, extraConfig?) => { return drizzlePgTable( name, { ...columns, _cursor: int8range("_cursor").notNull(), }, extraConfig, ); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
- change/@apibara-indexer-eae11f64-2203-4964-a273-9c1536b8fcdd.json (1 hunks)
- examples/indexer/package.json (1 hunks)
- examples/starknet-indexer/package.json (1 hunks)
- package.json (1 hunks)
- packages/indexer/package.json (2 hunks)
- packages/indexer/src/sinks/drizzle/insert.ts (2 hunks)
- packages/indexer/src/sinks/drizzle/utils.ts (1 hunks)
✅ Files skipped from review due to trivial changes (2)
- change/@apibara-indexer-eae11f64-2203-4964-a273-9c1536b8fcdd.json
- examples/indexer/package.json
🧰 Additional context used
🔇 Additional comments (8)
examples/starknet-indexer/package.json (1)
25-25: LGTM! Verify consistency and test the update.The update of
drizzle-ormto version^0.35.0aligns with the PR objectives and should help resolve the type issues mentioned in the PR summary.To ensure consistency across the project, please run the following script:
After verifying consistency, please ensure to:
- Run the project's test suite to confirm there are no breaking changes.
- If there are no tests, manually test the key functionalities that depend on
drizzle-orm.- Review the changelog for
drizzle-ormbetween versions 0.33.0 and 0.35.0 for any breaking changes or new features that might affect the project.packages/indexer/src/sinks/drizzle/insert.ts (1)
Line range hint
1-41: Overall, the changes look good and align with the PR objectives.The updates to type imports and usage, particularly the renaming of
PgInsertValuetoDrizzleInsertValue, contribute to resolving type issues as mentioned in the PR objectives. The code structure has been improved, enhancing readability and maintainability.A few minor suggestions have been provided to further improve type safety and code organization. These changes, if implemented, would further enhance the quality of the code without significantly altering its functionality.
package.json (1)
8-8: Verify the impact of removing the--parallelflag from the typecheck script.The
--parallelflag has been removed from thetypecheckscript, changing it from parallel to sequential execution. While this change might be intentional to address type-related issues, it's important to consider its implications:
- This could potentially increase the time taken for the typecheck process, especially in a large monorepo with multiple packages.
- On the other hand, sequential execution might provide more reliable results by avoiding potential race conditions.
Could you please clarify the reasoning behind this change? Also, it would be beneficial to monitor the impact on typecheck performance after this change.
To help assess the impact, you can run the following script to compare the execution times of the old and new typecheck commands:
packages/indexer/package.json (2)
101-101: Approved: drizzle-orm version update in peerDependenciesThe update of drizzle-orm from ^0.33.0 to ^0.35.2 in peerDependencies is consistent with the change in devDependencies and maintains package configuration consistency.
Please ensure that any documentation or README files mentioning the required drizzle-orm version are updated accordingly. Run the following command to check for any mentions of the old version:
✅ Verification successful
Verified: No references to drizzle-orm version 0.33.0 found in documentation
All documentation files have been checked and there are no mentions of the old drizzle-orm version
0.33.0. Documentation updates related to this version change are confirmed to be complete.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for mentions of the old drizzle-orm version in documentation # Test: Search for mentions of the old version in README and documentation files rg -i '0\.33\.0' $(fd -e md -e txt)Length of output: 397
83-83: Approved: drizzle-orm version update in devDependenciesThe update of drizzle-orm from ^0.33.0 to ^0.35.2 in devDependencies aligns with the PR objective to update to the latest release. This minor version update should include new features and backwards-compatible changes.
To ensure compatibility, please verify that all tests pass with this new version. Run the following command to check for any breaking changes or deprecations:
packages/indexer/src/sinks/drizzle/utils.ts (3)
19-29: Definition ofCursorColumnBuildercorrectly configures the_cursorcolumnThe
CursorColumnBuildertype is appropriately defined with the necessary properties, ensuring that the_cursorcolumn is accurately represented as a non-null custom column of typeInt8Range.
31-55:PgTableWithCursorFnsuccessfully extendspgTableto include_cursorThe custom
PgTableWithCursorFntype correctly extends thepgTablefunction, adding the_cursorcolumn to the table schema. This ensures that all tables created using this function will consistently include the_cursorcolumn.
57-65:PgInsertValuetype correctly omits_cursorto prevent manual insertionBy omitting the
_cursorcolumn from thePgInsertValuetype, the code prevents direct insertion into the_cursorcolumn, which is appropriate if the_cursorvalue is managed internally by the system.
1 participant
Update to the latest drizzle version to solve type issues on new projects.