-
Notifications
You must be signed in to change notification settings - Fork 76
[WIP] use mql #2478
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
base: main
Are you sure you want to change the base?
[WIP] use mql #2478
Conversation
@@ -6,7 +6,7 @@ import { major as majorVersion } from 'semver'; | |||
import type { | |||
DownloadCenterConfig, | |||
PlatformWithPackages, | |||
} from '@mongodb-js/dl-center/dist/download-center-config'; | |||
} from '@mongodb-js/dl-center'; |
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.
The new module resolution compiler option is not liking the /dist/ hack so I changed @mongodb-js/dl-center
to just export the types.
@@ -96,6 +96,7 @@ import { HIDDEN_COMMANDS } from '@mongosh/history'; | |||
import PlanCache from './plan-cache'; | |||
import ChangeStreamCursor from './change-stream-cursor'; | |||
import { ShellApiErrors } from './error-codes'; | |||
import type * as mql from '@mongodb-js/mql-typescript/schema'; |
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 is why I'm changing the moduleResolution ts compiler option. I want to be able to import a separate export from the default one.
@@ -87,7 +87,7 @@ export class Database< | |||
> extends ShellApiWithMongoClass<M> { | |||
_mongo: Mongo<M>; | |||
_name: StringKey<M>; | |||
_collections: Record<StringKey<D>, CollectionWithSchema<M, D>>; | |||
_collections: { [k in StringKey<D>]: CollectionWithSchema<M, D, D[k], k> }; |
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.
aah this was what I was looking for!
@@ -205,21 +205,21 @@ export default class Shard< | |||
} | |||
} | |||
|
|||
async _getConfigDB(): Promise<DatabaseWithSchema<M, M[keyof M]>> { | |||
async _getConfigDB(): Promise<DatabaseWithSchema<M, M['config']>> { |
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.
I went back and forth on exactly this change and I seemed to just trade one set of errors for another. Although I agree that M['config'] is more correct - it is what I tried first.
@@ -851,7 +849,7 @@ export function addHiddenDataProperty<T = any>( | |||
|
|||
export async function iterate( | |||
results: CursorIterationResult, | |||
cursor: AbstractCursor<any, any> | ChangeStreamCursor, | |||
cursor: { isClosed(): boolean; tryNext(): Promise<Document | null> }, |
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.
Good idea to specify smaller types. Didn't consider that.
No description provided.