-
Notifications
You must be signed in to change notification settings - Fork 6
TS 5.2 -> 5.8 #3410
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
TS 5.2 -> 5.8 #3410
Conversation
These are functionally equivalent (named tuples v free objects). And this is currently easier for TS/ Gel QB to express
…f currying. TS 5.5 requires this former decorator usage to be wrapped in parentheses. This isn't ideal so adjusting the signature.
📝 WalkthroughWalkthroughThis update refactors several custom decorator implementations to use a shared Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (10)
⏰ Context from checks skipped due to timeout of 90000ms (12)
✨ 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 (
|
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 (1)
src/components/partnership/partnership.gel.repository.ts (1)
40-51
: Simplified query with tuple membership filteringThe implementation has been improved to use a single
e.select
query with tuple membership filtering instead of multiple selects in a loop.Consider expanding on the TODO comment to explain why this implementation is logically equivalent to the previous one, or add a test case to verify the behavior remains the same.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (15)
package.json
(2 hunks)src/common/db-sort.decorator.ts
(2 hunks)src/common/db-unique.decorator.ts
(1 hunks)src/common/disabled.decorator.ts
(1 hunks)src/components/admin/admin.gel.repository.ts
(1 hunks)src/components/authorization/policy/executor/all-permissions-view.ts
(1 hunks)src/components/budget/handlers/update-project-current-budget-status.handler.ts
(1 hunks)src/components/engagement/engagement.gel.repository.ts
(1 hunks)src/components/partnership/partnership.gel.repository.ts
(2 hunks)src/components/project/dto/project.dto.ts
(1 hunks)src/core/database/common.repository.ts
(2 hunks)src/core/database/dto.repository.ts
(2 hunks)src/core/database/query-augmentation/pattern-formatting.ts
(1 hunks)src/core/database/query/sorting.ts
(2 hunks)test/region.e2e-spec.ts
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
test/region.e2e-spec.ts (1)
src/components/user/user.resolver.ts (1)
createPerson
(221-232)
src/core/database/query/sorting.ts (2)
src/common/db-sort.decorator.ts (1)
DbSort
(11-14)src/common/index.ts (1)
DbSort
(13-13)
src/components/project/dto/project.dto.ts (1)
src/common/disabled.decorator.ts (1)
Disabled
(8-12)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: E2E Tests (neo4j 6/6)
- GitHub Check: E2E Tests (neo4j 5/6)
- GitHub Check: E2E Tests (neo4j 4/6)
- GitHub Check: Analyze (javascript)
- GitHub Check: E2E Tests (neo4j 3/6)
- GitHub Check: E2E Tests (neo4j 2/6)
- GitHub Check: Generate (head)
- GitHub Check: E2E Tests (neo4j 1/6)
- GitHub Check: Generate (base)
- GitHub Check: Clean
- GitHub Check: Unit
- GitHub Check: lint
🔇 Additional comments (24)
package.json (1)
151-151
: TypeScript version upgrade.The TypeScript version has been upgraded from 5.2 to 5.8.3, with a corresponding update to the NestJS CLI TypeScript resolution. This aligns with the PR objective and supports the code changes in other files.
Also applies to: 161-161
src/components/budget/handlers/update-project-current-budget-status.handler.ts (1)
44-44
: Removed unnecessary non-null assertion.The
!
operator has been removed as it's no longer needed - TypeScript's control flow analysis can determine thatchange
is not null at this point because of the explicit check at line 31.src/components/authorization/policy/executor/all-permissions-view.ts (1)
56-56
: Type assertion added to handle complex dynamic typing.The
as any
type assertion was added to help TypeScript handle the complex dynamic typing in this section. This aligns with the existing comment at line 50 about "dynamic usage here is struggling" and makes the code compatible with TypeScript 5.8's stricter type checking.src/core/database/query-augmentation/pattern-formatting.ts (1)
23-23
: Grammar correction in comment.The comment was updated to correct a minor grammatical issue, changing "there's" to "there is".
src/common/disabled.decorator.ts (1)
8-12
: Streamlined decorator signature looks good!The simplified signature no longer uses generic type parameters and explicit decorator type constraints, which makes the code cleaner while maintaining the core no-op functionality.
src/components/project/dto/project.dto.ts (1)
200-205
: Updated decorator usage correctlyThe
@Disabled
decorator usage now properly matches the updated signature fromdisabled.decorator.ts
, which requires a string reason parameter to be passed with parentheses.src/components/partnership/partnership.gel.repository.ts (1)
18-19
: Standardized toe.select
instead of tuplesChanging from tuples to select objects is consistent with the broader structural updates across repositories.
Also applies to: 23-24
src/components/engagement/engagement.gel.repository.ts (1)
28-29
: Consistent structure updates usinge.select
The change from tuples to select objects for the
parent
field and its nestedproperties
aligns with the same pattern applied in the partnership repository, maintaining consistency.Also applies to: 31-32
src/components/admin/admin.gel.repository.ts (1)
32-33
: Improved query filtering syntaxThe change from
.is()
method toe.op()
for equality filtering makes the query construction more explicit and consistent with modern EdgeDB patterns.src/core/database/dto.repository.ts (2)
6-6
: Updated import to use the decorator object directlyThis change aligns with the PR's decorator refactoring pattern, replacing standalone functions with decorator objects that expose static methods.
69-69
: Refactored metadata access patternThe change from
getDbPropertyUnique()
toDbUnique.get()
is consistent with the refactoring of decorators to use a shared metadata decorator implementation.src/core/database/common.repository.ts (2)
6-6
: Updated import to use the decorator object directlyThis change aligns with the PR's decorator refactoring pattern, replacing standalone functions with decorator objects that expose static methods.
178-178
: Refactored metadata access patternThe change from
getDbPropertyUnique()
toDbUnique.get()
is consistent with the refactoring of decorators to use a shared metadata decorator implementation.src/core/database/query/sorting.ts (2)
6-6
: Updated import to use the decorator object directlyThis change aligns with the PR's decorator refactoring pattern, replacing standalone functions with decorator objects that expose static methods.
128-128
: Refactored metadata access patternThe change from
getDbSortTransformer()
toDbSort.get()
is consistent with the refactoring of decorators to use a shared metadata decorator implementation.test/region.e2e-spec.ts (2)
163-165
: Fixed reference to correct director objectGood correction to use
director.id
instead of what was likelynewDirector.id
previously. This ensures the test is using the director variable declared at the top level of the test suite, improving test consistency.
204-207
: Improved variable scoping by moving declarationGreat improvement to move the
newDirector
creation inside this specific test case rather than declaring it at the top level of the test suite. This follows best practices for test isolation by keeping variables scoped to the tests that actually use them.src/common/db-sort.decorator.ts (2)
1-1
: Clean import of decorator utilityGood addition of the
createMetadataDecorator
import to support the refactored implementation.
11-14
: Successfully refactored to use standardized decorator patternThe refactoring of
DbSort
to usecreateMetadataDecorator
is a great improvement that:
- Simplifies the implementation by removing manual metadata handling
- Provides a consistent pattern with other decorators
- Eliminates the need for a separate metadata retrieval function
The configuration correctly specifies that this works on properties and the setter correctly handles the
SortTransformer
function.src/common/db-unique.decorator.ts (5)
1-1
: Clean import of decorator utilityGood addition of the
createMetadataDecorator
import to support the refactored implementation.
6-7
: Improved documentation clarityThe updated documentation more clearly explains the purpose of the decorator.
10-14
: Well-structured refactoring of the decorator implementationGood implementation that preserves the original functionality while leveraging the new utility. The decorator still:
- Accepts an optional label parameter
- Generates a default label based on class and property name if not provided
- Applies both the unique constraint and the label decorator
16-19
: Clean implementation of inner decorator using utilityWell-structured implementation of the inner decorator using
createMetadataDecorator
. The configuration properly specifies it works on properties and the setter correctly handles the label value.
21-21
: Good API compatibility with static getterSmart approach to maintain API compatibility by exposing the inner decorator's
get
method asDbUnique.get
. This ensures that code using the previous standalonegetDbPropertyUnique
function can be easily updated to useDbUnique.get
instead.
No description provided.