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.

Sign up for GitHub

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

Jump to bottom

[⏳] Enhanced auto typed schema. #12122

Closed
wants to merge 52 commits into from
Closed

[⏳] Enhanced auto typed schema. #12122

wants to merge 52 commits into from

Conversation

mohammad0-0ahmad
Copy link
Contributor

@mohammad0-0ahmad mohammad0-0ahmad commented Jul 19, 2022 •
edited

@mohammad0-0ahmad mohammad0-0ahmad force-pushed the schema-options-coverage-auto-typed-schema branch from c1155d8 to 20a9073 Compare July 19, 2022 04:29
@mohammad0-0ahmad mohammad0-0ahmad marked this pull request as draft July 19, 2022 04:37
@mohammad0-0ahmad mohammad0-0ahmad changed the title [:hourglass_flowing_sand:] Enhanced auto typed schema. [⏳] Enhanced auto typed schema. Jul 19, 2022
@mohammad0-0ahmad mohammad0-0ahmad force-pushed the schema-options-coverage-auto-typed-schema branch 3 times, most recently from b728108 to b5fccf6 Compare July 21, 2022 11:31
mohammad0-0ahmad
mohammad0-0ahmad commented Jul 21, 2022
View reviewed changes
test/types/schema.test.ts
@@ -599,20 +600,6 @@ function gh11997() {
userSchema.index({ name: 1 }, { weights: { name: 1 } });
}

function gh12003() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test is incorrect, Auto typed schema will not work if typeKey has been changed in this way, because it will be "type" and will not reflect the correct type of the actual value.
@Uzlopak @vkarpov15 do you have any thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we can support this approach to supply schema options with auto typed schema.
It will work just if the end user use "const" assertion and not use predefined type, then the type will reflect the actual value.

@mohammad0-0ahmad mohammad0-0ahmad force-pushed the schema-options-coverage-auto-typed-schema branch from b5fccf6 to d487695 Compare July 21, 2022 23:22
@mohammad0-0ahmad mohammad0-0ahmad mentioned this pull request Jul 24, 2022
Closed
2 tasks
vkarpov15 and others added 13 commits July 25, 2022 21:48
@vkarpov15
fix(timestamps): set timestamps on child schema when child schema has…
77f671a 
… timestamps: true but parent schema does not

Fix Automattic#12119
@vkarpov15
chore: release 6.5.0
f5ee642
@vkarpov15
Merge pull request Automattic#12151 from Automattic/vkarpov15/Automat…
18d683b 
…ticgh-12119

fix(timestamps): set timestamps on child schema when child schema has timestamps: true but parent schema does not
@hasezoey
style(buffer): change ".split" to "utils.each" for defining methods
769aa21
@ramesh-km
docs(fix): fix link to next page in schema types page
af7ef3d
@hasezoey
style: change jsdoc headers to have a consistent empty jsdoc line after
63b2209
@Uzlopak
Merge pull request Automattic#12162 from hasezoey/jsdocHeaderNewLine
7dfe820 
style: change jsdoc headers to have a consistent empty jsdoc line after
@Uzlopak
Merge pull request Automattic#12160 from hasezoey/changeSplitToEach
46b629b 
style(buffer): change ".split" to "utils.each" for defining methods
@Uzlopak
Merge pull request Automattic#12161 from ramesh-km/patch-1
5846eee 
docs(fix): fix the link to next page in schema types page
@fredkilbourn
Update compatibility.md
dc43df7 
per discussion here: Automattic@f5ee642#commitcomment-79569628
@amanfrinati
Update populate.md
e5cb559 
Fix Populating Maps example
@Uzlopak
Merge pull request Automattic#12166 from amanfrinati/patch-1
0923e97 
Update populate.md
@Uzlopak
Merge pull request Automattic#12163 from fredkilbourn/patch-1
82dc7b9 
Update compatibility.md
mohammad0-0ahmad referenced this pull request in mohammad0-0ahmad-forks/mongoose Jul 28, 2022
@mohammad0-0ahmad
Fix one case of the reported issues.
d092fdc
@mohammad0-0ahmad mohammad0-0ahmad force-pushed the schema-options-coverage-auto-typed-schema branch 2 times, most recently from 70c6dc5 to 2aa4a62 Compare August 1, 2022 20:09
@dependabot
chore(deps-dev): bump eslint from 8.19.0 to 8.21.0
1f25ee5 
Bumps [eslint](https://github.com/eslint/eslint) from 8.19.0 to 8.21.0.
- [Release notes](https://github.com/eslint/eslint/releases)
- [Changelog](https://github.com/eslint/eslint/blob/main/CHANGELOG.md)
- [Commits](eslint/eslint@v8.19.0...v8.21.0)

---
updated-dependencies:
- dependency-name: eslint
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
AbdelrahmanHafez and others added 7 commits August 2, 2022 13:26
@AbdelrahmanHafez
Merge pull request Automattic#12197 from hasezoey/disableTideliftOnFork
f50181e 
chore(tidelift-alignment): run only in main repository, not in forks
@hasezoey
docs(compatibility): change to use a table
d459c8f
@vkarpov15
fix(types): allow using path with $count
7719569 
Fix Automattic#12149
@vkarpov15
fix(types): avoid baffling circular reference when using PopulatedDoc…
7ed781c 
… with a bidirectional reference

Fix Automattic#12136
Re: 11911
@vkarpov15
Merge pull request Automattic#12193 from Automattic/vkarpov15/Automat…
07e36aa 
…ticgh-12143

fix(model+query): handle populate with lean transform that deletes `_id`
@vkarpov15
fix(schema+timestamps): handle insertMany() with timestamps and dis…
25e4580 
…criminators

Fix Automattic#12150
@vkarpov15
Merge pull request Automattic#12200 from hasezoey/updateCompat
547680d 
docs(compatibility): change to use a table
@mohammad0-0ahmad mohammad0-0ahmad force-pushed the schema-options-coverage-auto-typed-schema branch from 2aa4a62 to 1397189 Compare August 3, 2022 06:46
@mohammad0-0ahmad mohammad0-0ahmad force-pushed the schema-options-coverage-auto-typed-schema branch 6 times, most recently from 611e66e to 120665e Compare August 6, 2022 23:33
ts-benchmark added 6 commits August 7, 2022 01:48
@mohammad0-0ahmad
Add new helpers to inferschematype.d.ts file
5a35b07 
 Author:    Mohammad Ahmad <kurtular.vadisi@gmail.com>
@mohammad0-0ahmad
coverage for __v path depending on schema options.
a73c87e 
 Author:    Mohammad Ahmad <kurtular.vadisi@gmail.com>
@mohammad0-0ahmad
Coverage for id virtual depending on schema options.
846a1de 
Author:    Mohammad Ahmad <kurtular.vadisi@gmail.com>
@mohammad0-0ahmad
Fix Resolve__v
af185d4 
 Author:    Mohammad Ahmad <kurtular.vadisi@gmail.com>
@mohammad0-0ahmad
Create and implement Extends to avoid circular ref error.
17ec0ad 
 Author:    Mohammad Ahmad <kurtular.vadisi@gmail.com>
@mohammad0-0ahmad
Apply Resolve_id to resolve the type of _id path using schema options
937a491 
 Author:    Mohammad Ahmad <kurtular.vadisi@gmail.com>
@mohammad0-0ahmad mohammad0-0ahmad force-pushed the schema-options-coverage-auto-typed-schema branch from 120665e to 937a491 Compare August 6, 2022 23:51
@hasezoey
Copy link
Collaborator

hasezoey commented Jan 7, 2023

@mohammad0-0ahmad is this PR still planned on being updated? because it looks rather stale to me - if this should not get a update within ~30 days, i would close it as "stale"

@mohammad0-0ahmad
Copy link
Contributor Author

@mohammad0-0ahmad is this PR still planned on being updated? because it looks rather stale to me - if this should not get a update within ~30 days, i would close it as "stale"

Hello @hasezoey !

Sorry for being inactive for a while.
I don't think as well that I will have time to continue with this one during the next month.
You can close it right now and when I have time to continue with it, then we can reopen it.
@vkarpov15 @Uzlopak might can check this PR and maybe complete it if they have time for that and if it something good to have soon.

Best regards everyone :)

@vkarpov15
Copy link
Collaborator

We're going to close this PR for now. We implemented part of this PR in #12731, but ran into some issues like #12871. Given those issues, we want to sort out getting this working for just timestamps option before expanding to other similar tasks. We need to be very cautious with big TypeScript changes, because there's always some obscure issue that pops up.

@vkarpov15 vkarpov15 closed this Jan 14, 2023
@francescov1
Copy link
Contributor

@vkarpov15 Was there any progress made on #11787? Or any relevant findings? I tried tackling this about 6 months ago but didn't have any luck, would like to give it another shot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@mohammad0-0ahmad @hasezoey @vkarpov15 @francescov1 @fredkilbourn @ramesh-km @Uzlopak @amanfrinati @JosueMolinaMorales @AbdelrahmanHafez