Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix/immutable reference #2121

Merged
merged 10 commits into from
Feb 5, 2025
Merged

fix/immutable reference #2121

merged 10 commits into from
Feb 5, 2025

Conversation

baywet
Copy link
Member

@baywet baywet commented Feb 4, 2025

makes document for references optional. Cleans up internal constructors for references, makes references immutable to avoid side effects

related #1998

Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Copy link

sonarqubecloud bot commented Feb 4, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
70.3% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@baywet
Copy link
Member Author

baywet commented Feb 4, 2025

@captainsafia this is where I wanted to get:

  • The proxy design pattern is properly implemented, one interface, two implementations. The consumers can use type assertions while reading a document to know what's mutable, what's not.
  • All reference types now offer a single public constructor, which only requires the reference ID but also accepts document, external resource. This way creating a document from the DOM remains simple/doesn't require carrying document references all over the place.
  • The target property is a pure function (no more backing field with potential side effects)
  • The reference property is immutable (no more side effects)
  • The properties of the reference type are also immutable (no more side effects)

The only drawback I can see is having to call RegisterComponents and/or SetReferenceHostDocument on the OpenAPIDocument based on what the consumer is doing. But we could "hide that" by calling those methods before serialization.

We'll try to release soon, in fact #2115 is already up so you can test that ASAP. I've noticed a much better developer experience and reliability on my end in kiota, openapi.net.odata, plugins, and API manifest libs. (linking locally)

@baywet baywet merged commit 878dd08 into dev Feb 5, 2025
13 of 14 checks passed
@baywet baywet deleted the fix/immutable-reference branch February 5, 2025 06:19
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants