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

No strong types on filters #407

Closed
nunomarks opened this issue Jan 17, 2023 · 10 comments
Closed

No strong types on filters #407

nunomarks opened this issue Jan 17, 2023 · 10 comments

Comments

@nunomarks
Copy link

Hello!

When using a model function, typescript does not error when using non-existing attributes on the schema.

const Model = papr.model('collection', schema(
  {
    attributeExists: Types.objectId(),
  }));

(async () => {
  await Model.findOne({ attributeNotExists: new ObjectId() });
})();
@CarlosLozanoHealthCaters
Copy link
Collaborator

Hi there!

We would also like to have strong types. We were researching some months ago and the permissive type is coming from MongoDB NodeJS driver. You can find more info there:

Basically, they prefer to avoid narrowing the type and prevent issues when evolving MongoDB operators. Is there anything we could do from Papr? Maybe overwrite Filter?.

@avaly
Copy link
Collaborator

avaly commented Jan 23, 2023

@CarlosLozanoHealthCaters I did actually try to overwrite Filter with a custom version in Papr, but I ended up in a big rabbit hole and other errors surfacing up.

I didn't know about the PR you linked from the mongodb repo. I'll try that code and see if it has a better outcome than my previous attempt.

@CarlosLozanoHealthCaters
Copy link
Collaborator

CarlosLozanoHealthCaters commented Jan 23, 2023

The main issue is that RootFilterOperators extends from Document. If we are able to overwrite that interface without that extension, everything will work correctly.

export declare interface RootFilterOperators<TSchema> extends Document {
    $and?: Filter<TSchema>[];
    $nor?: Filter<TSchema>[];
    $or?: Filter<TSchema>[];
    $text?: {
        $search: string;
        $language?: string;
        $caseSensitive?: boolean;
        $diacriticSensitive?: boolean;
    };
    $where?: string | ((this: TSchema) => boolean);
    $comment?: string | Document;
}

export declare interface Document {
    [key: string]: any;
}

@avaly
Copy link
Collaborator

avaly commented Jan 23, 2023

I did exactly this: I copied the types from mongodb and removed the extends Document and I got so many new errors (in our private repo).

@CarlosLozanoHealthCaters
Copy link
Collaborator

It would be something like:

export declare type PaprFilter<TSchema> = Partial<TSchema> | ({
    [Property in Join<NestedPaths<WithId<TSchema>, []>, '.'>]?: Condition<PropertyType<WithId<TSchema>, Property>>;
} & PaprRootFilterOperators<WithId<TSchema>>);

export declare interface PaprRootFilterOperators<TSchema> {
    $and?: PaprFilter<TSchema>[];
    $nor?: PaprFilter<TSchema>[];
    $or?: PaprFilter<TSchema>[];
    $text?: {
        $search: string;
        $language?: string;
        $caseSensitive?: boolean;
        $diacriticSensitive?: boolean;
    };
    $where?: string | ((this: TSchema) => boolean);
    $comment?: string | Document;
}

And then change all reference of Filter by PaprFilter in all internal types in papr and your private repo.

Did you have time to check if the errors were due to wrong naming in the filter?
Can you provide with an example of the errors so we can check together and think for a solution?

@avaly
Copy link
Collaborator

avaly commented Jan 23, 2023

Here's the branch in question: https://github.com/plexinc/papr/tree/feature/papr-filter-inexistent-fields

I'll try to dig up some of those new errors.

@avaly
Copy link
Collaborator

avaly commented Jan 23, 2023

An example of the errors that show up with this branch in our private repo:

Argument of type '{ 'person._id': ObjectId; }' is not assignable to parameter of type 'PaprFilter<{ _id: ObjectId; type: CreditType; person: { _id: ObjectId; name?: string | undefined; images?: { thumbnail?: string | undefined; } | undefined; slug?: string | undefined; }; ... 6 more ...; isPrimary?: boolean | undefined; }>'.
  Object literal may only specify known properties, and ''person._id'' does not exist in type 'PaprFilter<{ _id: ObjectId; type: CreditType; person: { _id: ObjectId; name?: string | undefined; images?: { thumbnail?: string | undefined; } | undefined; slug?: string | undefined; }; ... 6 more ...; isPrimary?: boolean | undefined; }>'.

The code:

await Credit.deleteMany({ 'person._id': person._id });

Part of the schema:

const creditSchema = schema({
  // ...
  person: Types.object(
    {
      _id: Types.objectId({ required: true }),
      name: Types.string(),
      slug: Types.string()
    },
    { required: true }
  ),
});

This may be caused by the recursive protections built in the NestedPaths type helper, but I'm not entirely sure. And if it is, I'm not sure how to work around it.

@CarlosLozanoHealthCaters
Copy link
Collaborator

I will try to work with that piece of code and check if I can find a solution!

@avaly
Copy link
Collaborator

avaly commented Jan 24, 2023

Related: #410

We just found out about mongodb's plan to remove these default Filter types. So this work just became more relevant and urgent.

@avaly
Copy link
Collaborator

avaly commented Feb 22, 2023

Closed in #430

@avaly avaly closed this as completed Feb 22, 2023
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

No branches or pull requests

3 participants