Update migration guide with findings#7625
Conversation
github-actions
Bot
added
the
📚 documentation
tobias-tengler
force-pushed
the
tte/update-migration-guide
branch
from
f9650ce to
26f90dc
Compare
tobias-tengler
force-pushed
the
tte/update-migration-guide
branch
from
26f90dc to
3db6a7c
Compare
There was a problem hiding this comment.
Pull request overview
Updates the Hot Chocolate v14 “migrate from 13 to 14” guide with additional upgrade findings encountered when updating internal services, filling gaps around GID/Node ID serialization, DataLoader nullability, builder APIs, test assertions, and pagination connections.
Changes:
- Documented
IIdSerializer→INodeIdSerializerrenames and related API/behavior changes. - Added notes on
DataLoader.LoadAsyncbecoming always nullable and a new unit-test assertion rename. - Documented the
Connection<T>constructor change fromgetTotalCounttototalCountand suggested using[IsSelected]to preserve conditional computation.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | Before | After | | ||
| | ----------------------------------- | --------------------------------------------------------------------------- | | ||
| | `SetQuery("{ __typename }")` | `SetDocument("{ __typename }")` | | ||
| | `AddVariableValue("name", "value")` | `AddVariableValues(new Dictionary<string, object?> { ["name"] = "value" })` | |
There was a problem hiding this comment.
The migration table lists AddVariableValues(...) as the replacement for AddVariableValue(...), but OperationRequestBuilder exposes SetVariableValues(...) overloads (including one that takes IReadOnlyDictionary<string, object?>). If AddVariableValues is not a real API, please update the table to the correct method name to avoid sending readers to a non-existent API.
| | `AddVariableValue("name", "value")` | `AddVariableValues(new Dictionary<string, object?> { ["name"] = "value" })` | | |
| | `AddVariableValue("name", "value")` | `SetVariableValues(new Dictionary<string, object?> { ["name"] = "value" })` | |
|
|
||
| The interface `IQueryResult` was replaced with `IOperationResult`. | ||
|
|
||
| ### IExecutionResult.ExpectQueryResult replaced by .ExpectOperationResult |
There was a problem hiding this comment.
The heading "IExecutionResult.ExpectQueryResult replaced by .ExpectOperationResult" is a bit inconsistent/ambiguous (leading dot without a receiver/type). Consider renaming it to either "IExecutionResult.ExpectQueryResult replaced by IExecutionResult.ExpectOperationResult" or just "ExpectQueryResult replaced by ExpectOperationResult" for clarity.
| ### IExecutionResult.ExpectQueryResult replaced by .ExpectOperationResult | |
| ### IExecutionResult.ExpectQueryResult replaced by IExecutionResult.ExpectOperationResult |
| | `.Deserialize("<gid-value>")` | `.Parse("<gid-value>", typeof(string))` where `string` is the underlying type of the GID | | ||
| | `.Serialize("MyType", "<raw-id>")` | `.Format("MyType", "<raw-id>")` | | ||
|
|
||
| The `Parse()` (previously `Deserialize()`) method has also changed its return type from `IdValue` to `NodeId`. The parsed Id value can now be accessed through the `NodeId.InternalId` instead of the `IdValue.Value` property. |
There was a problem hiding this comment.
This sentence uses "Id" in "parsed Id value"; elsewhere in the doc the acronym is written as "ID" (e.g., "node(id: ID!)"). Consider changing this to "parsed ID value" for consistency.
| The `Parse()` (previously `Deserialize()`) method has also changed its return type from `IdValue` to `NodeId`. The parsed Id value can now be accessed through the `NodeId.InternalId` instead of the `IdValue.Value` property. | |
| The `Parse()` (previously `Deserialize()`) method has also changed its return type from `IdValue` to `NodeId`. The parsed ID value can now be accessed through the `NodeId.InternalId` instead of the `IdValue.Value` property. |
| The interface `IQueryRequestBuilder` and its implementations were replaced with `OperationRequestBuilder` which now supports building standard GraphQL operation requests as well as variable batch requests. | ||
|
|
||
| The `Build()` method returns now a `IOperationRequest` which is implemented by `OperationRequest` and `VariableBatchRequest`. | ||
| The `Build()` method now returns a `IOperationRequest` which is implemented by `OperationRequest` and `VariableBatchRequest`. |
There was a problem hiding this comment.
In this sentence the article should be "an" (vowel sound) rather than "a": "an IOperationRequest".
| The `Build()` method now returns a `IOperationRequest` which is implemented by `OperationRequest` and `VariableBatchRequest`. | |
| The `Build()` method now returns an `IOperationRequest` which is implemented by `OperationRequest` and `VariableBatchRequest`. |
I've updated some internal services to HotChocolate 14 today and have documented what I've run into during the upgrade that wasn't covered by the migration guide.