Repository navigation
Upgrading three dependencies - #1303
Conversation
The upgraded typescript compiler has a different method overload resolution order. Hence, the useTranslation().t method had to be modified to ensure specificty in overload resolution.
- remove @types/testing-library__jest-dom because the above package ships it own types - there is no need to instruct tsc to explicitly import the types of @testing-library/jest-dom since the imports are self-contained during the compilation process
|
|
||
| afterEach(() => { | ||
| client.close() | ||
| // @ts-expect-error - restoring original location after test override |
There was a problem hiding this comment.
I was wondering why is this change relevant to the dependency upgrade?
There was a problem hiding this comment.
This diff is needed because of the major version upgrade of typescript dependency. The setter method for the location property has changed in the two versions.
There was a problem hiding this comment.
let me think of a more elegant way for this problem
There was a problem hiding this comment.
ok, this commit introduces a better usage of this API. It no longer resorts to evading the type-error with ts-expect-error directive.
|
|
||
| const getNetworkName = (network: string) => | ||
| t(`network_name`, { context: network }) | ||
| t(`network_name`, { context: network, defaultValue: network }) |
There was a problem hiding this comment.
I was wondering why is this change relevant to the dependency upgrade?
There was a problem hiding this comment.
Without this change, I get the following error from tsc
src/containers/Header/NetworkPicker/NetworkPicker.tsx:43:7 - error TS2345: Argument of type '["network_name", { context: string; }]' is not assignable to parameter of type '[key: "number" | "object" | "other" | "action" | "escrow" | "seconds" | "version" | "quorum" | "load" | "domain" | "fee" | "total" | "source" | "destination" | "yeas" | "nays" | "not" | ... 177 more ... | ("number" | ... 192 more ... | "prepayment")[], options?: { ...; } | undefined] | [key: ...] | [key: ...]'.
Type '["network_name", { context: string; }]' is not assignable to type '[key: string | string[], defaultValue: string, options?: ({ readonly context: string; } & $Dictionary) | undefined]'.
Type at position 1 in source is not compatible with type at position 1 in target.
Type '{ context: string; }' is not assignable to type 'string'.
43 t(`network_name`, { context: network })
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This PR upgrades the typescript compiler. The overload resolution logic has been updated because there is a major version upgrade typescript v4 -> v5.
The clue is in this message Type '["network_name", { context: string; }]' is not assignable to type '[key: string | string[], defaultValue: string, options?: ({ readonly context: string; } & $Dictionary) | undefined]'. --> If we want to specify a context value, then we must also specify a defaultValue (in case the internationalised translation is unavailable).
Apart from the complaints from tsc, I think this is a good change. It provides a necessary default, if explorer is used in a hitherto unknown language.
…t API for window.location assignment in typescript v5.9
| @@ -1,5 +1,5 @@ | |||
| import 'dotenv/config' | |||
| import '@testing-library/jest-dom/extend-expect' | |||
| import '@testing-library/jest-dom' | |||
There was a problem hiding this comment.
this subpath was removed from the @testing-library/jest-dom pacakge. These exports were provided at the root-level of the package
| "esModuleInterop": true, | ||
| "jsx": "react-jsx", | ||
| "lib": ["dom", "dom.iterable", "esnext"], | ||
| "types": ["jest", "@testing-library/jest-dom"], |
There was a problem hiding this comment.
these types had to explicitly added in prior versions of @testing-library/jest-dom during the compilation process. However, the newer release of this library self contains the types. Hence, there is no need to explicitly mention it as a config option for tsc
3 participants
High Level Overview of Change
As indicated by the commit messages, this PR upgrades three dependencies. The following extant PRs are obviated by this work:
#1242
https://github.com/ripple/explorer/pull/1221/changes
#1201
Type of Change
Test Plan
Existing tests pass with the upgraded dependencies. No functional change in the working of Explorer.