Skip to content
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

Boolean fields that appear in both the query and command of a MongoDB update can cause error #227

Open
danswann opened this issue Jan 3, 2025 · 2 comments · May be fixed by #229
Open

Comments

@danswann
Copy link
Contributor

danswann commented Jan 3, 2025

I think this is a pretty normal paradigm -- take every document where value is true and make it false (or vice versa).

Ex:

function markDead(scriptName: string) {
  $db.u({ script: scriptName, live: true} , { $set: { live: false } })
}

results in

Type 'false' is not assignable to type 'true'. (ts2322)
@danswann
Copy link
Contributor Author

danswann commented Jan 3, 2025

Maybe it's both easy and correct to just uncouple the query object from the command object.
Something like:

--- a/env.d.ts
+++ b/env.d.ts
@@ -959,7 +959,7 @@ declare global {
                  * @param query Specifies deletion criteria using query operators.
                  * @param command The modifications to apply.
                  * {@link https://docs.mongodb.com/manual/reference/method/db.collection.update/#parameters} */
-               u: <T extends MongoDocument>(query: MongoQuery<T> | MongoQuery<T>[], command: MongoUpdateCommand<T>) =>
+               u: <TQuery extends MongoDocument, TCommand extends MongoDocument>(query: MongoQuery<TQuery> | MongoQuery<TQuery>[], command: MongoUpdateCommand<TCommand>) =>
                        { n: number, opTime: { t: number }, ok: 0 | 1, nModified: number }[]
 
                /** Updates one document within the collection based on the filter.

Since both the document structure and the individual key can be heterogeneous, changing types like:

$db.u({key:"string"}, {$set:{key:5}})

should be totally legal, even though I think I would personally avoid it.

I can whip up the PR if you agree.

@samualtnorman
Copy link
Owner

yeah please make a PR, I'll play with it first and if I like the new behaviour I'll merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants