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

Feature: Infer schema type automatically. #11487

Closed

Conversation

mohammad0-0ahmad
Copy link
Contributor

@mohammad0-0ahmad mohammad0-0ahmad commented Mar 6, 2022

Replaced with: #11563

@Uzlopak
Copy link
Collaborator

Uzlopak commented Mar 6, 2022

Interesting. But do we really need tsafe? We use tsd and you could use expectError from tsd to check if the type is wrong.

@mohammad0-0ahmad
Copy link
Contributor Author

Interesting. But do we really need tsafe? We use tsd and you could use expectError from tsd to check if the type is wrong.

I use tsafe to compare two types while expectError can compare one type with another type that is returned of value.

@Uzlopak
Copy link
Collaborator

Uzlopak commented Mar 6, 2022

I would actually prefer to use tsd and not introduce another package.

But it is good to see this improvement.

@mohammad0-0ahmad
Copy link
Contributor Author

I would actually prefer to use tsd and not introduce another package.

But it is good to see this improvement.

I totally agree with you. But I couldn't make this test with tsd.

// test/types/schema.test.ts
assert<Equals<InferSchemaType<typeof AutoTypedSchema>, {
    userName: string;
    description?: string;
    isNotExist: boolean;
  }>>();

If you can fix this with tsd instead, then I will delete tsafe.

Best regards!

@Uzlopak
Copy link
Collaborator

Uzlopak commented Mar 6, 2022

How about:

  interface faultyInterface {
    userName: string;
    description?: string;
    doesNotExist: boolean;
  }
  expectError<InferSchemaType<typeof AutoTypedSchema>>(
    {} as faultyInterface
  );
  expectType<InferSchemaType<typeof AutoTypedSchema>>({} as M0_0aAutoTypedSchemaType);

Btw. npm run test-tsd throws errors. I know this is a WIP, but wanted to inform you anyway :).

@mohammad0-0ahmad
Copy link
Contributor Author

How about:

  interface faultyInterface {
    userName: string;
    description?: string;
    doesNotExist: boolean;
  }
  expectError<InferSchemaType<typeof AutoTypedSchema>>(
    {} as faultyInterface
  );
  expectType<InferSchemaType<typeof AutoTypedSchema>>({} as M0_0aAutoTypedSchemaType);

Btw. npm run test-tsd throws errors. I know this is a WIP, but wanted to inform you anyway :).

That's work now, I will remove tsafe now.
This test worked.

  expectError<faultyInterface>(
    {} as InferSchemaType<typeof AutoTypedSchema>
  );

@Uzlopak
Copy link
Collaborator

Uzlopak commented Mar 6, 2022

Nice.

Btw. Your code does have linter errors. To fix them if possible automatically just do:

npm run lint -- --fix

@mohammad0-0ahmad
Copy link
Contributor Author

Hello Uzlopak !
Would you like to check the result at this phase?
Iv'e fixed type on some functions and models static functions.
I will implement same solution for instance methods.
Do you have some suggestion? or maybe some notes or thoughts u would like to share?

lib/schema.js Outdated Show resolved Hide resolved
types/index.d.ts Outdated Show resolved Hide resolved
? DoesDocTypeExist<DocType> extends true ? DocType : DocDefinition
: unknown;

export type ObtainSchemaGeneric<TSchema, name extends 'DocType' | 'M' | 'TInstanceMethods' | 'TQueryHelpers' | 'DocDefinition'| 'StaticsMethods'> = TSchema extends Schema<infer DocType, infer M, infer TInstanceMethods, infer TQueryHelpers, infer DocDefinition, infer StaticsMethods> ? {DocType:DocType, M:M, TInstanceMethods:TInstanceMethods, TQueryHelpers:TQueryHelpers, DocDefinition:DocDefinition, StaticsMethods:StaticsMethods}[name] : never;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line is humongous
maybe some newlines would make it a little bit more readable

Suggested change
export type ObtainSchemaGeneric<TSchema, name extends 'DocType' | 'M' | 'TInstanceMethods' | 'TQueryHelpers' | 'DocDefinition'| 'StaticsMethods'> = TSchema extends Schema<infer DocType, infer M, infer TInstanceMethods, infer TQueryHelpers, infer DocDefinition, infer StaticsMethods> ? {DocType:DocType, M:M, TInstanceMethods:TInstanceMethods, TQueryHelpers:TQueryHelpers, DocDefinition:DocDefinition, StaticsMethods:StaticsMethods}[name] : never;
export type ObtainSchemaGeneric<TSchema, name extends 'DocType' | 'M' | 'TInstanceMethods' | 'TQueryHelpers' | 'DocDefinition'| 'StaticsMethods'> =
TSchema extends Schema<infer DocType, infer M, infer TInstanceMethods, infer TQueryHelpers, infer DocDefinition, infer StaticsMethods>
? {DocType:DocType, M:M, TInstanceMethods:TInstanceMethods, TQueryHelpers:TQueryHelpers, DocDefinition:DocDefinition, StaticsMethods:StaticsMethods}[name]
: never;

describe('Check if statics functions that is supplied in schema option is availabe', function() {
it('should give an array back rather than undefined m0_0aAutoTyped', function M0_0aModelJS() {
const testSchema = new mongoose.Schema({}, { statics: { staticFn() { return 'Returned from staticFn'; } } });
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so do we add a new option?
Probably also means that we have to modify the docs too.

@vkarpov15
Is adding statics here ok? Or does it could it break something with loadClass or so?

@Uzlopak
Copy link
Collaborator

Uzlopak commented Mar 9, 2022

tbh the typings are not that easy to digest. But I have no overall complaint regarding the changes, which would mean that you would need to refactor everything when you are finished and I would do a review then.

I would prefer to your finished PR and then pull IT and Check the typings

@Uzlopak
Copy link
Collaborator

Uzlopak commented Mar 23, 2022

Unfortunately I am currently not in Germany but in Iran and Internet is here horrible. 64 - 128 kby/s with regular disconnects. Took me few hours to download Ubuntu. Also have my wedding on Friday. So I have actually no real testing capability till 4. - 5. April, when I am back in Germany.

There are some merge conflicts. Please solve them, to increase the merging capabilities.

I assume that we will get in the first 1-3 weeks after release of the version containing this we could get some bug reports, like we had with the PipelineStages-PR. So it would be cool if you would check the issues after the release for potential regressions.

@mohammad0-0ahmad
Copy link
Contributor Author

mohammad0-0ahmad commented Mar 23, 2022

Unfortunately I am currently not in Germany but in Iran and Internet is here horrible. 64 - 128 kby/s with regular disconnects. Took me few hours to download Ubuntu. Also have my wedding on Friday. So I have actually no real testing capability till 4. - 5. April, when I am back in Germany.

There are some merge conflicts. Please solve them, to increase the merging capabilities.

I assume that we will get in the first 1-3 weeks after release of the version containing this we could get some bug reports, like we had with the PipelineStages-PR. So it would be cool if you would check the issues after the release for potential regressions.

Congratulations, I wish you happy times. No worries, I will solve merge conflicts soon, and I will try to check related issues afterwards.

@mohammad0-0ahmad
Copy link
Contributor Author

I am closing this PR to replace it with a cleaner one.
Link: #11563

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

Successfully merging this pull request may close these issues.

6 participants