-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Potentially unintentionally large AoE breaking changing to typescript MockedResponse
in type 3.9
#11842
Comments
Yeah, unfortunately Generally: while we try to guarantee "no breaking changes" for runtime code, that is not true for TypeScript annotations - sometimes we just have to break a type's behaviour to enable an improvement in developer experience in the long term. But of course, we try to keep these changes to a minimum. The change here is hard to avoid, and using I'm investigating a few unorthodox solutions for this (see this tweet, but I can't guarantee that anyone will come out of it. In the meantime, can I suggest that you use a helper type like type MockedResponseArray = ReadonlyArray<MockedResponse<Record<string, any>, any>> ? That way you introduce |
Could you please try #11848 and report back if that works for you? npm i @apollo/client@0.0.0-pr-11848-20240516124842 |
Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better. |
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Issue Description
Overview
The
MockedResponse
had breaking changes to its types in 3.9 to allow the ability to dynamically match mocks.But it came with a potentially unintended side affect. Here I'd like to get the contributors thoughts on this problem and suggest a solution.
The breaking change
This change affected the usability of the
MockedResponse
type. Constructing a strongly typedMockedResponse
is useful for constructing mocked responses with type hints from the editor before supplying it into the mock link. However, you can no longer declare aMockedReponse
of a specific type:And pass it into a function that receives a generic list of
MockedResponse
This will return an error that didn't use to happen in 3.8:
Even though the
MockedResponse
is internally type-safe, typescript is unable to infer this when passing it into a generic list ofMockedResponse
. It doesn't know that we are only calling the function with the variables that we give it, which were type-safe upon construction.You can see a simplified reproduction of this typescript limitation here.
The current official migration
The official typing of the
MockLink
got around this in an unwieldy way. The constructor used to simply take a list ofMockedResponse
But now it uses
any
for the type arguments to avoid this issue.The consequences
This behavior has leaked into the public interface. API consumers must now explicitly define
MockedResponse<any, any>[]
any time they are declaring a generic list ofMockedResponse
that receive more highly specifiedMockedResponse
. Depending on the repository style guide, linter errors will need to be suppressed, or a wrapper type defined to avoid usingany
explicitly. In addition, the typescript error is confusing. Users may wonder why their more specific type doesn't get cast automatically to the looserRecord<string, any>
type of the defaultMockedResponse
(this was okay until the generic was used inside function arguments).In the spirit of encouraging type-safe code and avoiding the burden on developers to come to this conclusion on their own, I suggest the following solution.
A potential solution
To fix this this without sacrificing type safety consider changing the interface of
MockedResponse
from:to
In most cases, when users construct their own
MockedResponse
, typescript will infer the correct type from the usage ofTData
and validate that it adheres toRecord<string, any>
.While still preserving the ability to pass in responses into a
MockedResponse[]
without the need to decode the error and resort to specifyingMockedResponse<any, any>[]
Conclusion
3.9 introduced breaking API changes that subtly affect the usability of working with
MockedResponse[]
. I believe we can restore the old flexibility while still supporting the new features. I'd like to hear what the maintainers think about this issue and the potential solution, or if this is intentional and here to stay. Thank you for your feedback.Link to Reproduction
https://www.typescriptlang.org/play/?#code/ATAuE8AcFNgJWgZwK4BtQDFkDsDGoBLAe2wB4BBAGjgD5gBeYACgEMAnAcwC5hyBKBnTgBuAFCiQEGMACyRXAGtoAEwSJIJRNAoN40XETbLSiUGwLYOlYC2zga1uLoQGjJsxau37dRgG8JEBtOHnIxIOAAMxxcHjU0TBjCEgpqGnDgAF9xSShYGXAAEXlkAFtobFBdAIiWHmwygCNoNkDswKl88AA1dgIWRtQkasCQRvqmlracqKTibGBcFlRULDxEJmi8AHE2ImRIRB45RRU1DWwtAG0AXQEaoK3cXf3DgDpIwwBRFlwAC02MReB0Esx2ewOHxigPBrze7A4fD400CBkuVTYSAuWmO8iUqixmm0BWKuDKFVA1gKvXMAyGiF8wAeICePARoKYfnGwQ48MyfEoox5PD8dWAACZsiB2iAAPSy4DQABuFTAf32HD+wAA7rAFNgiNq1QREMATWA8sBECxIrBEBZcLAlis1rhEIF5WbTcttSxwKbnahPGrYE9kgttQRQFqo6bDQtIJjlNBIhYVDzyZVBXKFZ1ELhzJAqktsAByKoWW1sNUsUBvVHLVYxDZXTHqIl3YRAA
Reproduction Steps
No response
@apollo/client
version3.10.3
The text was updated successfully, but these errors were encountered: