Fix integration tests type errors on bindgen #5479
Conversation
We're now relying 100% on the "bson" package.
| @@ -24,7 +24,7 @@ export interface IPerson { | |||
| _id: Realm.BSON.ObjectId; | |||
| name: string; | |||
| age: number; | |||
| friends: Realm.List<IPerson>; | |||
| friends: Realm.List<Person>; | |||
There was a problem hiding this comment.
Doing this as a quickfix for now... pending our discussions regarding this
gagik
requested review from
kraenhansen,
kneth,
takameyer,
elle-j and
JacobOscarGunnarsson
gagik
force-pushed
the
gagik/bindgen-type-fixes
branch
from
b26886c
to
29f66cd
Compare
| @@ -9,7 +9,7 @@ | |||
| "resolveJsonModule": true, | |||
| "outDir": "dist", | |||
| "lib": [ | |||
| "es2018" | |||
| "es2020" | |||
There was a problem hiding this comment.
Fixes:
../../node_modules/@thi.ng/api/typedarray.d.ts:112:10 - error TS2583: Cannot find name 'BigUint64Array'. Do you need to change your target library? Try changing the 'lib' compiler option to 'es2020' or later.
There was a problem hiding this comment.
Looks good! We might want to bring some of those open questions up in the group.
And I'm sure there will also be a lot more of this type of work once the migrated tests land in bindgen. Might be a good opportunity to implement whatever is the answer to said open questions.
| @@ -822,7 +822,7 @@ export class Realm { | |||
| /** | |||
| * Deletes the provided Realm object, or each one inside the provided collection. | |||
| */ | |||
| delete(subject: RealmObject | RealmObject[] | List | Results): void { | |||
| delete<T = DefaultObject>(subject: RealmObject<T> | RealmObject<T>[] | List<T> | Results<T>): void { | |||
There was a problem hiding this comment.
I think this needs an overloaded definition to better depict these types, similar to create above. I don't think any is the answer
| @@ -27,6 +27,8 @@ import * as bson from "bson"; | |||
| export namespace BSON { | |||
| export const ObjectId = bson.ObjectId; | |||
| export type ObjectId = bson.ObjectId; | |||
| export const ObjectID = bson.ObjectID; | |||
There was a problem hiding this comment.
I wish it didn't exist, but we might not have a choice.
| // TODO: should this not support removeByObjectType(Dog)? | ||
| expect(mutableSubs.removeByObjectType(Dog.schema.name)).to.equal(0); |
There was a problem hiding this comment.
The API currently only exposes a method taking a string ☝️ But having an overload accepting e.g. Dog could probably be helpful (as long as the name retrieved will be the same value specified on the name property of the schema).
There was a problem hiding this comment.
We have this functionality with i.e. create so I do think this should be possible
There was a problem hiding this comment.
Yeah, it could provide a nicer consistency with both create and objects :)
| @@ -822,7 +822,7 @@ export class Realm { | |||
| /** | |||
| * Deletes the provided Realm object, or each one inside the provided collection. | |||
| */ | |||
| delete(subject: RealmObject | RealmObject[] | List | Results): void { | |||
| delete<T = DefaultObject>(subject: RealmObject<T> | RealmObject<T>[] | List<T> | Results<T>): void { | |||
There was a problem hiding this comment.
Another question, does this method need an overload that takes the generic arg T extends RealmObject?
(Update: Saw that @takameyer mentioned this ☝️ , I believe we're referring to the same thing)
Co-authored-by: Andrew Meyer <[email protected]>
|
@realm/realm-js-team what should we do with this PR? |
|
It will be complicated to rebase this branch so I am closing the PR |
What, How & Why?
Fixes all type errors in our integration tests pre-test migration.
This closes #5486
☑️ ToDos
Breakinglabel has been applied or is not necessary