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

encodeURIComponent should accept any value type #18159

Closed
2is10 opened this issue Aug 30, 2017 · 8 comments · Fixed by #31103
Closed

encodeURIComponent should accept any value type #18159

2is10 opened this issue Aug 30, 2017 · 8 comments · Fixed by #31103
Labels
Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Help Wanted You can do this Suggestion An idea for TypeScript
Milestone

Comments

@2is10
Copy link

2is10 commented Aug 30, 2017

TypeScript Version: 2.5.2

Code

encodeURIComponent(true);

Expected behavior:
tsc compiles with no errors.

Actual behavior:
error TS2345: Argument of type 'true' is not assignable to parameter of type 'string'.

Discussion:

encodeURIComponent is currently defined liked this:

declare function encodeURIComponent(uriComponent: string): string; 

In reality, encodeURIComponent accepts any value and coerces it to a string if necessary. The first step in its definition in the ES6 spec reads:

  1. Let componentString be ToString(uriComponent).

It’s pretty common to pass true or false or a number like 0 or 10 as a query parameter, for instance. Here are two possible ways to broaden the definition:

declare function encodeURIComponent(uriComponent: any): string; 

declare function encodeURIComponent(uriComponent: string | number | boolean): string; 

The any option would be technically correct, but passing null or undefined or a non-primitive value is usually a bug (in my experience). You can use your own judgment about how broadly the uriComponent argument should be defined. I’m specifically interested in using number and boolean.

@mhegazy
Copy link
Contributor

mhegazy commented Aug 30, 2017

Similar discussion can be found in #4002.

We have chosen to be more strict with the standard library and only allow strings, not things that can be coerced into string by the engine.

@mhegazy mhegazy added the Working as Intended The behavior described is the intended behavior; this is not a bug label Aug 30, 2017
@rattrayalex-stripe
Copy link

I think this is different than #4002, since it explicitly/transparently does coercion for the return value, not merely for a comparison value.

I probably never intend to do isFinite(someBoolean), but I could intend to use encodeURIComponent(someBoolean) as a shorthand for encodeURIComponent(someBoolean.toString()) since it has the same effect

@mhegazy mhegazy added Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Help Wanted You can do this Suggestion An idea for TypeScript and removed Working as Intended The behavior described is the intended behavior; this is not a bug labels Aug 30, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Aug 30, 2017

Would take a PR for it.

@neoncube2
Copy link

I just ran into this when trying to encode numbers, too.

Doing something like the below code gives a compile-time error, which, as someone more used to plain JavaScript, was unexpected.

getUrl(column: number) { return 'http://example.com/' + encodeUriComponent(column); }

@mhegazy mhegazy added this to the Community milestone Jan 4, 2018
rstarkov pushed a commit to FignumOld/TypeGap that referenced this issue Apr 4, 2018
@jdalrymple
Copy link

Is there a way to override the declaration in the mean time?

@jdalrymple
Copy link

Figured it out:

declare global {
  function encodeURIComponent(uriComponent: string | number | boolean): string; 
}

@NN---
Copy link

NN--- commented Feb 12, 2019

@jdalrymple Do you have time to submit PR , please ?

@smockle
Copy link
Member

smockle commented Apr 24, 2019

Opened PR #31103

From this issue’s description:

It’s pretty common to pass true or false or a number like 0 or 10 as a query parameter, for instance. Here are two possible ways to broaden the definition:

declare function encodeURIComponent(uriComponent: any): string; 
declare function encodeURIComponent(uriComponent: string | number | boolean): string; 

The any option would be technically correct, but passing null or undefined or a non-primitive value is usually a bug (in my experience). You can use your own judgment about how broadly the uriComponent argument should be defined. I’m specifically interested in using number and boolean.

I opted for the first solution (using any) because it more closely matches the ES5 spec, but I see merit in @2is10’s second solution as well.

RyanCavanaugh pushed a commit that referenced this issue Apr 26, 2019
…om 'string' to 'string | number | boolean'. Fixes #18159 (#31103)

- According to the ECMAScript 5.1 spec (§15.1.3.4), 'encodeURIComponent' invokes the abstract operation 'ToString': https://www.ecma-international.org/ecma-262/5.1/#sec-15.1.3.4
- In the spec (§9.8), 'ToString' accepts an 'Undefined', 'Null', 'Boolean', 'Number', 'String' or 'Object' argument: https://www.ecma-international.org/ecma-262/5.1/#sec-9.8
- TypeScript’s 'StringConstructor' accepts an argument with type 'any': https://github.com/Microsoft/TypeScript/blob/b0100100a18d740ad0b7c626bc81b800b5273ece/lib/lib.es5.d.ts#L518
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
8 participants