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

String.split, low cost lib modification for narrowing return type of non-empty strings #56333

Closed
6 tasks done
craigphicks opened this issue Nov 7, 2023 · 7 comments
Closed
6 tasks done
Labels
Duplicate An existing issue was already created

Comments

@craigphicks
Copy link

craigphicks commented Nov 7, 2023

πŸ” Search Terms

#52299 (closed)
#30406 (marked as design limitation)

βœ… Viability Checklist

⭐ Suggestion

In es5.d.ts

before:

interface String {
    split(separator: string | RegExp, limit?: number): string[];
}

after:

interface String {
    split<S extends string>(separator: S): string extends S ? string[] : 
        S extends "" ? string[] : [string, ...string[]];
    split(separator: string | RegExp, limit?: number): string[];
}

πŸ“ƒ Motivating Example

@noUncheckedIndexAccess: true
function checkField(fieldName: string){
    let firstPathPart = fieldName;
    if (fieldName.includes('.')) {
        const partParts = fieldName.split('.');
        firstPathPart = partParts[0]; // error
    }
}

πŸ’» Use Cases

  1. What do you want to use this for?

As shown above

  1. What shortcomings exist with current approaches?

As shown above.

  1. What workarounds are you using in the meantime?
@noUncheckedIndexAccess: true

interface String {
    split<S extends string>(separator: S): string extends S ? string[] : 
        S extends "" ? string[] : [string, ...string[]];
    split(separator: string | RegExp, limit?: number): string[];
}

function checkField(fieldName: string){
    let firstPathPart = fieldName;
    if (fieldName.includes('.')) {
        const partParts = fieldName.split('.');
        firstPathPart = partParts[0]; // no error
    }
}

{
// #53362 & #56332 & #56333 all give these results
const x1 = "".split('.'); // [string, ...string[]]
const x10 = x1[0]; // string
const x11 = x1[1]; // string | undefined
const x31 = "".split('.')[0]; // string
const x32 = "".split('.')[1]; // string | undefined
const x4 = "".split(''); // string[] 
const x5 = "".split((0 as any as string)); // string[]
}

[playground](https://www.typescriptlang.org/play?#code/FDCWDsBcFMCcDMCGBjaACAypWEDmaBvYNEtAZwAcAbUSAHgzWgA8ZwATM87PAPgAoy0ColiJIAe1gAuTAEpZZHuHws2nTGgD83HCoDaAXTSzipc4zXQOXAES3tuvEZNp9SvbgA0aAHT+PZ0NDAG4zEkoaSEFhUXEpRWV8AB80ACVoXABRZgofGgBbWi1ZcABXAoAjOAUnA1DgAF8QeDLwZEhQCXA0ZAALaGQAawAxUGgqdn54ccmAOUQC6ETPOSJzKmhINBnYJQAFcT7D2G2AXh3Z9gWlsPNQeDRpq5voXwhkKjL2aDJ+AHJfP85HJCOFzL1uko0CJTidIFwLjMJtdFm9IrQAUC5HcISRdgcjvC0BdYZB4WR9AAGUJoAD0dLQ4AkTFgsCk4OazWAyCh22YAEYSWh7L4MdFAcCQvTGW5AiofP4xUkjIYeXy0MwAEzCwXU2kMuq4dXgaHMADMwtF4qxwP10sN8uNvNN-IALFbbGLqJj-lKZUajCazQBWT3eqL8fhUtCILiIcAAT1jXCdIIdjKdQaAA)

@craigphicks craigphicks reopened this Nov 7, 2023
@craigphicks craigphicks changed the title String.split, lost cost improve for narrowing return type of non-empty strings String.split, low cost lib modification for narrowing return type of non-empty strings Nov 7, 2023
@MartinJohns
Copy link
Contributor

Duplicate of #53362.

@fatcerberus
Copy link

fatcerberus commented Nov 7, 2023

How does this differ from #56332?
edit: nevermind, this was the "auxiliary proposal" from that one.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Nov 7, 2023
@craigphicks
Copy link
Author

craigphicks commented Nov 7, 2023

Why it is not a duplicate of #53362

@MartinJohns

interface String {
    split<T extends string | RegExp>(separator: T, limit?: number): T extends `${string}${infer U}` ? [string, ...string[]] : string[];
}

is not the same.

  1. The type result is same. No problem with that - the result is good. 53362 wasn't declined just because it was useful.

  2. The reason for declining 56333 was

FWIW: we have a pretty low appetite for making the standard library definitions extremely complex for the sake of NUIA, which is a flag we added with significant hesitation because it makes things like these difficult to handle.

The keyword is "extremely complex". Unfortunately it is not well defined. If that applies to this proposal we have learned something about what "extremely complex" means.

Why it is not a duplicate of #56332

@fatcerberus

This is the auxiliary proposal in 56332

interface String {
    split<S extends string>(separator: S): string extends S ? string[] : 
        S[0] extends undefined ? string[] : [string, ...string[]];
    split(separator: string | RegExp, limit?: number): string[];
}    

The auxiliary proposal 56332 has a prerequisite that the element access part of proposal #34692 be implemented, and This proposal 56333 does not.

@craigphicks
Copy link
Author

craigphicks commented Nov 8, 2023

@RyanCavanugh - This is not a duplicate. However, this could potentially declined for the same reason that #53362 was declined. In your reason for declining 53362 you used the expression extremely complex, which unfortunately is not well defined. It would helpful if you could clarify that meaning.

The evaluation extremely complex could mean using infer. Or it could mean using any conditional expression. Or it could mean making any charge to a lib solely for the sake of NUIA.

I'm inclined to believe that these would be good explicit rules:

  1. No changes to lib/*.d.ts files for the sake of NUIA (noUncheckedIndexAccess compiler option).
  2. No use of conditional types in lib/*.d.ts files at all.

@RyanCavanaugh
Copy link
Member

Complexity is relative to value provided. The value of String#split being a "provably non-empty" array is, in estimation, pretty low, in line with what you're saying.

I'm inclined to believe that these would be good explicit rules:

I'd say these are very good (especially re: NUIA), but not 100% rules (maybe 95%) ?

I don't really have a good place to write these down, but feel free to reference this comment (and I will likely do the same).

@MartinJohns
Copy link
Contributor

Why it is not a duplicate of #53362

But it is. #53362 is about improving the type definitions for String#split, and this issue is about improving the type definitions for String#split. The only difference is the approach suggested, but that does not really warrant a new issue, it can be added as a comment instead. Besides that, #53362 is not (yet) declined, it's awaiting more feedback.

@typescript-bot
Copy link
Collaborator

This issue has been marked as "Duplicate" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

5 participants