Skip to content
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

USVString type definition #28775

Open
hasbel opened this issue Nov 30, 2018 · 7 comments
Open

USVString type definition #28775

hasbel opened this issue Nov 30, 2018 · 7 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Breaking Change Would introduce errors in existing code Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Suggestion An idea for TypeScript

Comments

@hasbel
Copy link

hasbel commented Nov 30, 2018

TypeScript Version: 3.2.0-dev.201xxxxx

Search Terms:
USVString

Code

function example(uri: USVString) {
    // Do some work
}

Expected behavior:
Until typescript version 3.0.3 USVString used to be an existing type defined in lib.dom.d.ts
in version 3.1, according to the breaking changes page, some vendor-specific types are removed from lib.d.ts, a full list of removed types is included.
USVString is not on that list, yet it's type definition was also removed.

USVString is defined under webIDL, as can be seen here.

Actual behavior:
error TS2304: Cannot find name 'USVString'.

@weswigham
Copy link
Member

Our IDL-based generator just interprets USVString as an alias for string, because it effectively is. Do you still need a type USVString = string alias in the lib? The lib itself stopped using it because it was rather pointless.

@weswigham weswigham added Suggestion An idea for TypeScript Breaking Change Would introduce errors in existing code In Discussion Not yet reached consensus Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature labels Nov 30, 2018
@hasbel
Copy link
Author

hasbel commented Nov 30, 2018

Some of our modules as well as some external NPM modules we are using need USVString as a type.

Even though I agree that it is rather pointless from a programming point of view as it just maps to a string, it serves a purpose as it gives developers useful informations about what is permissible.
And since it is also used by external libraries/modules that might not be updated any time soon, we need to manually remove it or declare it in those files as well (which is really not ideal).

furthermore, it is still defined by webIDL and the removal was not documented so figuring why some code suddenly refuses to compile might be a bit of a headache.

@weswigham
Copy link
Member

Cc @DanielRosenwasser

@DanielRosenwasser
Copy link
Member

Let's discuss in person tomorrow

@RyanCavanaugh RyanCavanaugh removed the In Discussion Not yet reached consensus label Mar 13, 2019
@RyanCavanaugh RyanCavanaugh removed this from the TypeScript 3.4.0 milestone Mar 13, 2019
@rodoabad
Copy link

rodoabad commented Jun 3, 2021

Hey folks. What needs to happen on this type so we can merge it? Or is USVString pretty much just a string?

@jimmywarting
Copy link
Contributor

jimmywarting commented Jun 27, 2021

I have been pretty much bothered with the way typescript handles USVString as a String
To me they don't seem to be the same thing

a USVString dose lots of conversion casting mostly anything to a string, it deals with Symbol.toStringTag, toString() and defaulting values by doing something like String(null_or_whatever)

Here are some example

// This works fine and it's totally valid in vanilla js!
async function get (req) {
    const url = new URL(req)
    url.searchParams.set('api_key', '---')
    return fetch(url)
}

TS throws for using fetch in this way...

Argument of type 'URL' is not assignable to parameter of type 'RequestInfo'.
  Type 'URL' is not assignable to type 'string'.

type RequestInfo = Request | string;

it shouldn't be a regular string. It should be a USVString instead!

Same thing with blobs!
it's totally legit to do: new Blob([{}]) if you want to do something like this, you could provide a own custom class that has a toString method and have it working

- type BlobPart = BufferSource | Blob | string;
+ type BlobPart = BufferSource | Blob | USVString;

@thewilkybarkid
Copy link

I have been pretty much bothered with the way typescript handles USVString as a String To me they don't seem to be the same thing

a USVString dose lots of conversion casting mostly anything to a string, it deals with Symbol.toStringTag, toString() and defaulting values by doing something like String(null_or_whatever)

Here are some example

// This works fine and it's totally valid in vanilla js!
async function get (req) {
    const url = new URL(req)
    url.searchParams.set('api_key', '---')
    return fetch(url)
}

TS throws for using fetch in this way...

Argument of type 'URL' is not assignable to parameter of type 'RequestInfo'.
  Type 'URL' is not assignable to type 'string'.

type RequestInfo = Request | string;

it shouldn't be a regular string. It should be a USVString instead!

I've just been caught out by this when trying to move from node-fetch 2 to 3 (refs node-fetch/node-fetch#1261 (comment)). node-fetch 2 declared its own type: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/c00dae848d207c182dff9a1dceae179e2edff15e/types/node-fetch/index.d.ts#L212. This isn't exactly the same as allowing any stringable, but covered a common use case.

cross-fetch also suffers from the same problem as it uses the DOM RequestInfo type directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Breaking Change Would introduce errors in existing code Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

8 participants