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

[typescript] define or derive typings for Query.select() and Query.findX(filter, projection) args relative to the given schema #11437

Closed
1 task done
the-vampiire opened this issue Feb 21, 2022 · 15 comments
Labels
new feature This change adds new functionality, like a new method or class
Milestone

Comments

@the-vampiire
Copy link

  • using latest version of mongoose

Do you want to request a feature or report a bug?

What is the current behavior?

context

when using the projection argument of a Query.findX(filter, projection) or chaining the Query.select(arg) method i would like to have typing support.

what is happening

the current typing for both projection and select args is any

What is the expected behavior?

what i would like to happen

define the arg Type as the fields in my schema

example model

interface IUser {
  username: string;
  password: string;
}

interface IUserDocument extends IUser, Document {}

const UserSchema = new Schema<IUserDocument>({
  username: { type: String, required: true },
  password: { type: String, required: true },
});

const User = mongoose.model("User", UserSchema);

export default User;

example querying

User.findOne(
  // this is correctly picking up the fields / Types (although annoyingly mixed with Document properties...)
  { username: "some name" },
  // using projection string
  "/* intellisense for IUser fields as enum */",
)
// using select Query method
.select({ /* intellisense for IUser fields as { [field: string]: 0 | 1 */ });

example implementation (not the best at TS, probably a better way)

// derived from IUser, used for projection string
type UserFieldProjection = "username" | "password";

type QuerySelectOption = 0 | 1;

type SelectUserField {
  [field in UserFieldProjection]: QuerySelectOption;
}

What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version.

  • node: v17.3.1
  • mongoose: ^6.2.0
  • mongodb: (derived from mongoose)
@the-vampiire the-vampiire changed the title define or derive typings for Query.select() and Query.findX(filter, projection) args relative to the given schema [typescript] define or derive typings for Query.select() and Query.findX(filter, projection) args relative to the given schema Feb 21, 2022
@the-vampiire
Copy link
Author

made a bit of progress on trying to make this generic:

export type QuerySelectOption = 0 | 1;
export type PopulateOption<T> = string & keyof T;
export type SelectFields<T> = {
  [field in PopulateOption<T>]?: QuerySelectOption;
}

export type SelectUserFields = SelectFields<IUser>;

User
  .findOne({ username: "some name" })
  // manually define "as" but at least some typing is here now!
  .select({} as SelectUserFields);

unfortunately not able to get it working for the populate arg because it needs to repeat the options with a space between.

i can do it manually with template literal type but cant get it to repeat X times (X = number of fields) dynamically. nor can i figure out how to exclude an already selected field

export type PopulateOptions<T> =
  | `${PopulateOption<T>}`
  | `${PopulateOption<T>} ${PopulateOption<T>}`
  | `${PopulateOption<T>} ${PopulateOption<T>} ${PopulateOption<T>}`
  | `${PopulateOption<T>} ${PopulateOption<T>} ${PopulateOption<T>} ${PopulateOption<T>}`;
  // repeat fieldNumber of times?
  // exclude already selected option?

@Uzlopak
Copy link
Collaborator

Uzlopak commented Feb 22, 2022

which typescript version was again introducing template literals?

@AbdelrahmanHafez
Copy link
Collaborator

Having TypeScript support for .select with strings using string literals is a brilliant idea!
We may need to allow entering any as well though, it can't be strict because you selecting deeply.nested.field is a valid case, also addresses.$.cityName is also valid, supporting all use-cases with TS can be tricky.

But that's a great start.

@Uzlopak
Copy link
Collaborator

Uzlopak commented Feb 22, 2022

If you put any in there, it renders the whole typing thing basically useless. :/

@AbdelrahmanHafez
Copy link
Collaborator

It still would give Intellisense support for most of the use cases.
It's the equivalent of allowing users to add any fields to new Document, while new User({ address: { countryName: 'Egypt' } }); is valid, so is new User({ 'address.countryName': 'Egypt' });
Strictly supporting all those types can be tricky, we don't want valid JS syntax to fail because we don't have TS support for it yet.

An incremental approach would be to give users IntelliSense support, and allow them to enter whatever they want, that way the software is not perfect, but it's in a better state than it previously was (no types support at all).

Maybe someday, when we've covered all the use cases (I doubt we'll get to that day), we can make the typings strict in a major version.
Until that day, allowing users to enter anything is the only way we can incrementally improve mongoose and people can feel comfortable upgrading mongoose to newer versions without having their code incorrectly broken.

What do you think?

@the-vampiire
Copy link
Author

which typescript version was again introducing template literals?

@Uzlopak they were introduced in TS 4.1

@the-vampiire
Copy link
Author

Having TypeScript support for .select with strings using string literals is a brilliant idea! We may need to allow entering any as well though, it can't be strict because you selecting deeply.nested.field is a valid case, also addresses.$.cityName is also valid, supporting all use-cases with TS can be tricky.

But that's a great start.

@AbdelrahmanHafez just to clarify i am only able to get it working for select using the object options arg (see my second message). i cant figure out how to make it work for the strings.

i am not an expert at TS at all. i have worked with JS for years but just started with TS a few months ago. at this point im just hacking my way through it with research and trial/error.

there may be some TS wizards that could figure it out. so far my research suggests it isnt possible to do dynamically. the reason being that there are a near infinite number of combinations (growing as N fields increases).

@the-vampiire
Copy link
Author

the-vampiire commented Feb 22, 2022

i think a good starting point for this is figuring out how to correctly introspect the document and derive only the "custom fields" (non Document properties). do either of you know how to do that?

with that as a basis i think we can at least get the select options object typing working internal to mongoose instead of the as hack i demoed above.

looking ahead i believe (but again, im no expert in TS) that it should be possible to introspect for "nested paths" (object fields) and recursively introspect subdocuments.

@the-vampiire
Copy link
Author

the-vampiire commented Feb 22, 2022

i did come across this SO answer which could give a lead for generating nested path strings.

@AbdelrahmanHafez
Copy link
Collaborator

AbdelrahmanHafez commented Feb 22, 2022

just to clarify i am only able to get it working for select using the object options arg

That's fine. Knowing that it's in the realm of possibility is good enough.
Even if it's not possible to do repetition today, we can report our use case to the TypeScript team, they'd probably help us find a "hack" to do it, or actually introduce the functionality into the language.

I am not an expert at TS at all. i have worked with JS for years but just started with TS a few months ago.

Likewise, it's pretty nice.

there may be some TS wizards that could figure it out

@taxilian @ahmedelshenawy25 here's a problem you might be interested in.

@the-vampiire
Copy link
Author

the-vampiire commented Feb 22, 2022

TS is just wonderful. i really regret not having started sooner. it has completely transformed my development experience. i will never go back to JS for anything more than quick hack scripts.

i used to think JS + JSDocs was enough. and for a time it was a "TS-light" solution. but when i ran into needing to share / reuse the JSDocs i realized i was basically reinventing TS. the rest is history!

@AbdelrahmanHafez
Copy link
Collaborator

i think a good starting point for this is figuring out how to correctly introspect the document and derive only the "custom fields" (non Document properties). do either of you know how to do that?

For those reasons, mongoose suggests you don't follow the same pattern in the original post.
Instead of passing an interface that extends Document, pass the raw interface, and let mongoose wrap it in a document only when it makes sense.

looking ahead i believe (but again, im no expert in TS) that it should be possible to introspect for "nested paths"...

I agree that this might be tricky, as stated above: the goal is to improve support for types, not have a perfect state, so it should be fine to start with simple top-level-fields selection, and then we can work on improving it to handle more cases such as deeply nested paths, index operators and all the other cases that we can support.

@Uzlopak
Copy link
Collaborator

Uzlopak commented Feb 22, 2022

@webstrand
@jcalz

Can you give us your wisdom you mighty typescript wizards?

@IslandRhythms IslandRhythms added the new feature This change adds new functionality, like a new method or class label Feb 22, 2022
@the-vampiire
Copy link
Author

For those reasons, mongoose suggests you don't follow the same pattern in the original post. Instead of passing an interface that extends Document, pass the raw interface, and let mongoose wrap it in a document only when it makes sense.

ah great point man. its funny i remember seeing that when i first started using mongoose with TS a few months ago and it made no sense to me. now it finally does! man this is so much easier to work with.

this is the general pattern im using now:

// used for create (user provided input of the doc)
interface IUserInput {
  username: string;
  password: string;
}

// adds timestamp fields (maybe theres an easier way?)
interface IWithTimestamps {
  createdAt: string;
  updatedAt: string;
}

// additional fields (any extra fields not received from input)
interface IUser extends IUserInput, IWithTimestamps {
  internalField: string;
}

type SelectUserFieldOptions = SelectFieldOptions<IUser>;

const UserSchema = new Schema<IUser>({ ... }, { timestamps: true });

const User = mongoose.model("User", UserSchema);

@the-vampiire
Copy link
Author

Hey right on @vkarpov15! Thanks for the help with this guys. Great feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature This change adds new functionality, like a new method or class
Projects
None yet
Development

No branches or pull requests

5 participants