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

[REG 6->7] Plain fields cannot be assigned in special case (typescript error) #13529

Closed
2 tasks done
orgads opened this issue Jun 19, 2023 · 2 comments
Closed
2 tasks done
Labels
typescript Types or Types-test related issue / Pull Request
Milestone

Comments

@orgads
Copy link
Contributor

orgads commented Jun 19, 2023

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Mongoose version

7.3.0

Node.js version

18.16.0

MongoDB server version

5.x

Typescript version (if applicable)

5.1.3

Description

Directly assigning to fields of a document fails with v7 when the document is generic (special case, see example).

Steps to Reproduce

import { Model } from 'mongoose';

interface ResourceDoc {
  foo: string;
}

type ResourceIdT<ResourceIdField extends string> = {
  [key in ResourceIdField]: string;
}
type ResourceDocWithId<ResourceIdField extends string> = ResourceDoc & ResourceIdT<ResourceIdField>;

class TestClass<
  ResourceType extends string,
  DocT extends ResourceDocWithId<`${ResourceType}Id`>>
{
  constructor(dbModel: Model<DocT>) {
    const resourceDoc = new dbModel();
    resourceDoc.foo = 'bar'; // Type 'string' is not assignable to type 'Require_id<DocT>["foo"]'
  }
}

If I replace `${ResourceType}Id` with 'FooId' then it works.

Expected Behavior

It should work.

@vkarpov15 vkarpov15 added this to the 7.3.2 milestone Jun 19, 2023
@vkarpov15 vkarpov15 added the typescript Types or Types-test related issue / Pull Request label Jun 19, 2023
@vkarpov15
Copy link
Collaborator

As far as I can tell, the script as written should throw an error. ResourceType can be any string, so there's no guarantee that there's a foo property on resourceDoc.

@vkarpov15 vkarpov15 removed this from the 7.3.2 milestone Jul 3, 2023
@vkarpov15 vkarpov15 added help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary needs clarification This issue doesn't have enough information to be actionable. Close after 14 days of inactivity and removed typescript Types or Types-test related issue / Pull Request labels Jul 3, 2023
@orgads
Copy link
Contributor Author

orgads commented Jul 4, 2023

@vkarpov15 Notice that ResourceDocWithId is an intersection of ResourceDoc (which has foo) and ResourceIdT<ResourceIdField> (which adds the id field).

This is caused by Omit. Not sure what has changed between 6 and 7, but #13573 fixes it (but breaks other things 🙄).

orgads added a commit to orgads/mongoose that referenced this issue Jul 4, 2023
orgads added a commit to orgads/mongoose that referenced this issue Jul 4, 2023
@vkarpov15 vkarpov15 added typescript Types or Types-test related issue / Pull Request and removed help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary needs clarification This issue doesn't have enough information to be actionable. Close after 14 days of inactivity labels Jul 4, 2023
@vkarpov15 vkarpov15 modified the milestones: 7.3.2, 7.3.3 Jul 4, 2023
vkarpov15 added a commit that referenced this issue Jul 10, 2023
types: avoid unnecessary MergeType<> if TOverrides not set, clean up statics and insertMany() type issues
@vkarpov15 vkarpov15 reopened this Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typescript Types or Types-test related issue / Pull Request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants