-
Notifications
You must be signed in to change notification settings - Fork 1
Resolve query method deprecation #101
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
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.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
|
|
||
| func resolve(in context: Context) -> [[Entity]] { | ||
| resolveQueries().compactMap { $0.resolve() } | ||
| } |
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.
Bug: Context parameter ignored in new resolve method
The new resolve(in context:) method accepts a context parameter but doesn't use it. Instead, it calls resolveQueries() which uses the stored self.context, and then resolves each query using their own stored contexts. The provided context parameter is completely ignored, making this method functionally identical to the deprecated version and defeating the purpose of explicitly providing context.
|
|
||
| func resolve(in context: Context) -> [Entity] { | ||
| resolveQueries().resolve() | ||
| } |
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.
Bug: Context parameter ignored in new resolve method
The new resolve(in context:) method accepts a context parameter but doesn't use it. Instead, it calls resolveQueries() which uses the stored self.context, and then calls the deprecated resolve() method on the collection which uses each query's stored context. The provided context parameter is completely ignored, making this method functionally identical to the deprecated version and defeating the purpose of explicitly providing context.
|
|
||
| func resolve(in context: Context) -> Entity? { | ||
| result(context, id) | ||
| } |
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.
Bug: Key computed with wrong context in resolve
The new resolve(in context:) method passes the provided context parameter to the result closure but uses id which is computed from self.context via the key closure. This mixes two different contexts: the key is derived from the stored context while the result lookup uses the provided context. The key should be computed using the provided context parameter to maintain consistency.
|
|
||
| func resolve<Entity>(in context: Context) -> [Entity] where Element == Query<Entity> { | ||
| compactMap { $0.resolve() } | ||
| } |
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.
Bug: Collection resolve ignores context parameter
The new resolve(in context:) method for collections accepts a context parameter but calls the deprecated resolve() method without passing the context. This means the provided context is ignored and each query uses its own stored context instead of the explicitly provided one.
Note
Deprecates parameterless
resolve()and addsresolve(in:)acrossQuery,QueryList,QueryGroup, and collections of queries to require explicit context.resolve()inQuery,QueryList,QueryGroup, andCollectionofQuery.resolve(in:)counterparts requiring aContext.resolveQueries(),whenResolved, etc.) to use the new resolve flow.Written by Cursor Bugbot for commit 4e1c11b. This will update automatically on new commits. Configure here.