types: align with lib.dom.d.ts by removing [Symbol.toStringTag] from File/FormData #2968
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In TypeScript `lib.dom.d.ts`, the interfaces `File` and `FormData` do
not include the `[Symbol.toStringTag]` instance method.
By aligning types, can avoid type errors such as the following when
combining undici in a JSDom environment or other Browser like
environments.
```
.../tests/utils/fetch.ts:99:3 - error TS2322: Type 'Mock<(resource: RequestInfo | URL, options?: RequestInit | undefined) => Promise<Response>>' is not assignable to type '{ (input: RequestInfo | URL, init?: RequestInit | undefined): Promise<Response>; (input: string | Request | URL, init?: RequestInit | undefined): Promise<...>; }'.
Types of parameters 'options' and 'init' are incompatible.
Type 'RequestInit | undefined' is not assignable to type 'import(".../node_modules/undici/types/fetch").RequestInit | undefined'.
Type 'RequestInit' is not assignable to type 'import(".../node_modules/undici/types/fetch").RequestInit'.
Types of property 'body' are incompatible.
Type 'BodyInit | null | undefined' is not assignable to type 'BodyInit | undefined'.
Type 'FormData' is not assignable to type 'BodyInit | undefined'.
Property '[Symbol.toStringTag]' is missing in type 'FormData' but required in type 'import(".../node_modules/undici/types/formdata").FormData'.
99 globalThis.fetch = jest.fn(fetch);
~~~~~~~~~~~~~~~~
node_modules/undici/types/formdata.d.ts:107:12
107 readonly [Symbol.toStringTag]: string
~~~~~~~~~~~~~~~~~~~~
'[Symbol.toStringTag]' is declared here.
```
```
.../tests/utils/fetch.ts:99:3 - error TS2322: Type 'Mock<(resource: URL | RequestInfo, options?: RequestInit | undefined) => Promise<Response>>' is not assignable to type '{ (input: URL | RequestInfo, init?: RequestInit | undefined): Promise<Response>; (input: string | URL | Request, init?: RequestInit | undefined): Promise<...>; }'.
Types of parameters 'options' and 'init' are incompatible.
Type 'RequestInit | undefined' is not assignable to type 'import(".../node_modules/undici/types/fetch").RequestInit | undefined'.
Type 'RequestInit' is not assignable to type 'import(".../node_modules/undici/types/fetch").RequestInit'.
Types of property 'body' are incompatible.
Type 'BodyInit | null | undefined' is not assignable to type 'BodyInit | undefined'.
Type 'FormData' is not assignable to type 'BodyInit | undefined'.
Type 'FormData' is not assignable to type 'import(".../node_modules/undici/types/formdata").FormData'.
The types returned by 'get(...)' are incompatible between these types.
Type 'FormDataEntryValue | null' is not assignable to type 'import(".../node_modules/undici/types/formdata").FormDataEntryValue | null'.
Type 'File' is not assignable to type 'FormDataEntryValue | null'.
Property '[Symbol.toStringTag]' is missing in type 'File' but required in type 'import(".../node_modules/undici/types/file").File'.
99 globalThis.fetch = jest.fn(fetch);
~~~~~~~~~~~~~~~~
node_modules/undici/types/file.d.ts:38:12
38 readonly [Symbol.toStringTag]: string
~~~~~~~~~~~~~~~~~~~~
'[Symbol.toStringTag]' is declared here.
```
These types were both originally added in
5425b77 adnd then modified slightly in
609d89b.
The `[Symbol.toStringTag]` instance method is not something we expect
users to call directly.
|
Can you add a test that ensures ts allows |
So, with this change, the above expression fails with typescript: Does that mean you consider this PR incorrect? Pardon my lack of knowledge here, but what is the use case for a library consumer would call this method. IIUC, this is an internal implementation detail for use by The following does works in a browser: But this property isn't defined on typescript's lib.dom.d.ts. I guess I'm asking, where should this difference be reconciled? |
|
The webidl spec says that we should define Symbol.toStringTag on the object's prototypes. In the same way that Headers has a Symbol.iterator in its typings for example. The DOM types are wrong here. |
|
@andrewbranch wdyt? |
|
Thanks for all the feedback. I'll take a look at this upstream. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This relates to...
#1172
Rationale
In TypeScript
lib.dom.d.ts, the interfacesFileandFormDatado not include the[Symbol.toStringTag]instance method.By aligning types, can avoid type errors such as the following when combining undici in a JSDom environment or other Browser like environments.
These types were both originally added in 5425b77 adnd then modified slightly in 609d89b.
The
[Symbol.toStringTag]instance method is not something we expect users to call directly.Changes
Removed the
[Symbol.toStringTag]instance method from the File and FormData interfaces in the filesfile.d.tsandformdata.d.ts.Features
N/A
Bug Fixes
N/A
Breaking Changes and Deprecations
N/A
Status