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

Some signatures should use union instead of overloads #5766

Closed
falsandtru opened this issue Nov 24, 2015 · 7 comments
Closed

Some signatures should use union instead of overloads #5766

falsandtru opened this issue Nov 24, 2015 · 7 comments
Labels
Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue Help Wanted You can do this Suggestion An idea for TypeScript

Comments

@falsandtru
Copy link
Contributor

I think, this code should be correct code by fixes of lib.d.ts or type system.

var separator: string|RegExp;
'str'.split(separator); // type error

Manual: Replace to union types

Resolve by type definition.

from:

// lib.d.ts
    split(separator: string, limit?: number): string[];
    split(separator: RegExp, limit?: number): string[];

to:

// lib.d.ts
    split(separator: string|RegExp, limit?: number): string[];

Automatic: Generate union types

Resolve by auto generated merged signature from overloads.

from:

// lib.d.ts
    split(separator: string, limit?: number): string[];
    split(separator: RegExp, limit?: number): string[];

Generate the merged signature internally, implicitly.

to:

// in memory
    split(separator: string, limit?: number): string[];
    split(separator: RegExp, limit?: number): string[];
    split(separator: string|RegExp, limit?: number): string[]; // generated
@ahejlsberg
Copy link
Member

Agreed, we should fix the declaration of split to take a union type for the first parameter.

@ahejlsberg ahejlsberg added the Suggestion An idea for TypeScript label Nov 24, 2015
@falsandtru
Copy link
Contributor Author

Thanks! Should I create a list of all integration targets? Such as String#match/replace/search, and more many types. This is steady manual works.

@RyanCavanaugh RyanCavanaugh added Help Wanted You can do this Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript labels Nov 24, 2015
@RyanCavanaugh
Copy link
Member

It's probably worth having a script or something to find these cases.

Unfortunately we can't perform this merging on the type system side because the two forms (two overloads vs one overload with a union) actually have rather different semantics in practice.

@falsandtru
Copy link
Contributor Author

All right, I'll leave it up to you.

@falsandtru falsandtru changed the title Integrate overloads into union types manually, or automatically Integrate overloads into union types Nov 25, 2015
@falsandtru
Copy link
Contributor Author

When TypeScript fixes this issue? I want to fix this issue myself partially if TypeScript doesn't fix this issue until next version 1.8.

@mhegazy
Copy link
Contributor

mhegazy commented Jan 16, 2016

@falsandtru feel free to send a PR for the issue.

@falsandtru
Copy link
Contributor Author

@mhegazy thanks, I'll fix it.

@mhegazy mhegazy modified the milestone: Community Sep 21, 2016
@mhegazy mhegazy changed the title Integrate overloads into union types String.split signature should use union instead of overloads Dec 14, 2016
avonwyss added a commit to avonwyss/TypeScript that referenced this issue Jan 29, 2017
Fixes the string methods which accept either a string or RegExp as pattern. Also, some help texts were fixed to represent the actual behavior as per spec. See microsoft#5766
@falsandtru falsandtru changed the title String.split signature should use union instead of overloads Some signatures should use union instead of overloads Mar 13, 2017
@mhegazy mhegazy modified the milestones: TypeScript 2.3, Community Mar 13, 2017
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Mar 13, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants