DOCSP-25723 Updates to Mutually Recursive Schema Types #459

ajhuh-mdb · 2022-12-01T20:11:50Z

Pull Request Info

PR Reviewing Guidelines

Notes

Original ticket mentions broad support for recursive schema types, but the original Node PR only reflects changes for mutually recursive schema types
Changes to be made were discussed in this Slack thread due to incorrectly documented behavior/examples in the Node release notes.

JIRA
https://jira.mongodb.org/browse/DOCSP-25723

Staging

Self-Review Checklist

Is this free of any warnings or errors in the RST?
Did you run a spell-check?
Did you run a grammar-check?
Are all the links working?

ajhuh-mdb · 2022-12-01T20:16:41Z

source/includes/limitations/limits.rst


 .. _node-driver-recursive-types-dot-notation:

 Recursive Types and Dot Notation
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

-.. important:: Impacted Versions
-
-   - 4.3


Removed this admonition since this affects nested recursive types from all versions after v4.3.1

Question: I thought v4.11 fixed this issue. What am I missing?

Ah so the "fix" I meant to refer to when we met was not type safety.

For clarification, the "issue" from 4.3.0 was that, previously, just the presence of a self-referential recursive schema type at all would cause a compilation error. The "fix" in 4.3.1 is that they allowed for the code to compile with a recursive schema type present, but their solution, in doing so, couldn't guarantee type checking (which becomes a separate "issue" from the one originally mentioned).

Sorry about that confusion 😅

source/includes/limitations/limits.rst

ajhuh-mdb · 2022-12-01T20:40:11Z

source/whats-new.txt

-TypeScript.
+The 4.11 {+driver-short+} release includes added support for **mutually 
+recursive** collection schema types. The driver also provides type safety for 
+dot notation queries up to a depth of eight in this release. Typescript compiles 


v4.11 release notes say nine, but tests of original PR say eight

Same comment as above--confirm this with devs

ajhuh-mdb · 2022-12-01T20:50:44Z

source/includes/limitations/limits.rst

      name: string;
   }
+
+The {+driver-short+} provides type safety for mutually recursive types 
+referenced through dot notation up to a depth of eight. The following code 


v4.11 release notes say nine, but tests of original PR say eight

Definitely worth confirming with the devs

The tests are correct, I'll be updating the notes

mongoKart

great job! reading through this helps me understand the subject (which i couldn't get my head around an hour ago) much better. i made some suggestions for clarity since this is weirdo technical stuff and we should be super precise. feel free to edit my suggestions as you see fit!

source/fundamentals/typescript.txt

source/includes/limitations/limits.rst

mongoKart · 2022-12-02T16:44:18Z

source/includes/limitations/limits.rst


 .. _node-driver-recursive-types-dot-notation:

 Recursive Types and Dot Notation
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

-.. important:: Impacted Versions
-
-   - 4.3


Question: I thought v4.11 fixed this issue. What am I missing?

mongoKart · 2022-12-02T16:48:16Z

source/includes/limitations/limits.rst

+In versions 4.3 through 4.10 of the {+driver-short+}, you cannot specify a 
+**mutually recursive** type as a type parameter. To specify a mutually 
+recursive type as a type parameter, use version 4.11 or newer.


Issue: I think we can remove the version note if you're going to do a separate PR for versions 4.3-4.10. To me, it's implied that older versions of a driver might not be able to do things a new version would do.

Suggestion: Delete these lines, then make "mutually recursive" bold on line 82.

While I do agree, I'm starting to skew towards keeping it. Just thinking of the case where a user using an older version (e.g. v4.3) goes to our docs site (which defaults to the current version v4.12) and sees the incorrect information without realizing they're on the incorrect version of the docs. Not sure if this is too much of an edge case, though... I'll likely bring it up as a q in #textual-dbx to get others' thoughts.

Makes sense. I'm good with whatever the team decides.

mongoKart · 2022-12-02T16:48:42Z

source/includes/limitations/limits.rst

      name: string;
   }
+
+The {+driver-short+} provides type safety for mutually recursive types 
+referenced through dot notation up to a depth of eight. The following code 


Definitely worth confirming with the devs

mongoKart · 2022-12-02T16:53:45Z

source/whats-new.txt

+dot notation queries up to a depth of eight in this release. Typescript compiles 
+your code beyond a depth of eight, but types fall back to ``any`` at this level. 


Issue: Somewhat confusing

Suggestion:

Suggested change

dot notation queries up to a depth of eight in this release. Typescript compiles

your code beyond a depth of eight, but types fall back to ``any`` at this level.

dot notation queries up to a depth of eight in this release. At a depth greater than or equal to eight, all dot-notation properties on recursive types are assigned the ``any`` type.

(Is "properties" the right word here?)

I think the important thing for users to know here is that their code won't deal with a compilation error, rather than knowing the specifics of what type they'll fall back to.

The bit about the any type assignment is more so for users to know why they won't run into a compilation error. Will try to restructure the sentence to mitigate confusion

source/whats-new.txt

ajhuh-mdb · 2022-12-02T19:07:03Z

Hi @nbbeeken and @baileympearson 👋

Whenever y'all get a chance, could I a technical review on these documentation edits referencing the changes made in NODE-3875?

Mainly looking for:

Technical accuracy.
Consistency/alignment between these docs, the v4.11 release notes on GH, the original PR, and anywhere else that may reference these behavior changes.

Thanks!

mongoKart

LGTM once the team agrees on that admonition

nbbeeken

LGTM, one small syntax fix just to get the highlighting working

source/whats-new.txt

Co-authored-by: Neal Beeken <[email protected]>

* DOCSP-25723 Updates to Mutually Recursive Schema Types * edit mutually recursive pet code block * edit collection schema name * more specific limitations menu text * MW comment edits * rephrase confusing sentence * remove warning notes for v4.3-4.10 behavior * Apply suggestions from code review Co-authored-by: Neal Beeken <[email protected]> Co-authored-by: Mike Woofter <[email protected]> Co-authored-by: Neal Beeken <[email protected]> (cherry picked from commit 72be9c1)

ajhuh-mdb added 4 commits December 1, 2022 13:54

DOCSP-25723 Updates to Mutually Recursive Schema Types

91e0f0f

edit mutually recursive pet code block

c003c63

edit collection schema name

041baff

more specific limitations menu text

f65113a

ajhuh-mdb commented Dec 1, 2022

View reviewed changes

source/includes/limitations/limits.rst Show resolved Hide resolved

ajhuh-mdb commented Dec 1, 2022

View reviewed changes

mongoKart requested changes Dec 2, 2022

View reviewed changes

MW comment edits

da3bbe3

ajhuh-mdb requested review from baileympearson and nbbeeken December 2, 2022 19:05

mongoKart approved these changes Dec 2, 2022

View reviewed changes

ajhuh-mdb added 2 commits December 2, 2022 16:26

rephrase confusing sentence

3707291

remove warning notes for v4.3-4.10 behavior

45b2fb1

nbbeeken approved these changes Dec 5, 2022

View reviewed changes

source/whats-new.txt Outdated Show resolved Hide resolved

Apply suggestions from code review

b454952

Co-authored-by: Neal Beeken <[email protected]>

mongoKart merged commit 72be9c1 into mongodb:master Dec 5, 2022

jordan-smith721 mentioned this pull request Dec 6, 2022

Selective Backport DOCSP-25723 #461

Merged

4 tasks

		dot notation queries up to a depth of eight in this release. Typescript compiles
		your code beyond a depth of eight, but types fall back to ``any`` at this level.

DOCSP-25723 Updates to Mutually Recursive Schema Types #459

DOCSP-25723 Updates to Mutually Recursive Schema Types #459

Uh oh!

Conversation

ajhuh-mdb commented Dec 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Info

Self-Review Checklist

Uh oh!

ajhuh-mdb Dec 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ajhuh-mdb Dec 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ajhuh-mdb Dec 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mongoKart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ajhuh-mdb Dec 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ajhuh-mdb Dec 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ajhuh-mdb commented Dec 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mongoKart left a comment

Choose a reason for hiding this comment

Uh oh!

nbbeeken left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ajhuh-mdb commented Dec 1, 2022 •

edited

Loading

ajhuh-mdb Dec 1, 2022 •

edited

Loading

ajhuh-mdb Dec 2, 2022 •

edited

Loading

ajhuh-mdb Dec 1, 2022 •

edited

Loading

ajhuh-mdb Dec 2, 2022 •

edited

Loading

ajhuh-mdb Dec 2, 2022 •

edited

Loading

ajhuh-mdb commented Dec 2, 2022 •

edited

Loading