microsoft / microsoft/TypeScript

Un-feature suggestion: relax type predicate construction rule

Open
#57,685 0 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

In Discussion Suggestion
Dominant language
Go
Stars
111k
Forks
14.3k
Avg merge
1d 19h
Merged PRs (30d)
117

Description

🔍 Search Terms

#54116 - Unlike the example in this post, it doesn't show a typePredicate function which has it's own natural constraint that (in a naive implementation) collides with the current rule.

✅ Viability Checklist
⭐ Suggestion

Relax (or even remove) the type predicate construction rule 'A type predicate's type must be assignable to its parameter's type`.

Reason: A users typePredicate function may have a narrower context specific constraint which is sufficient, but conflicts with the imposed 2677 rule. There is a workaround, but it is not immediately obvious and may incur extra processing time because it requires inferring the value of substitute variable.

📃 Motivating Example

Example 1: (Naive code that doesn't work)

interface Fizz {
    id: number;
    fizz: string;
};

interface Buzz {
    id: number;
    buzz: string;
}

interface TheUnfizz {
    fizz: Buzz;
}

declare const buzz: Buzz;
declare const unfizz: TheUnfizz;

/**
 * Valid range of T1 for users implementation is T1 extends Fizz|Buzz.
 * User knos a priori TheUnfizz is not going to enter the function - it's a callback, not a general purpose function.
 */
function isFizzBad1<T1 extends Fizz|Buzz /* actual valid range for users implementation */, Fizz extends T1 /* prevents TheUnfizz, et. al, */>(x: T1): x is Fizz {
// Type parameter 'T1' has a circular constraint.ts(2313)
    return "fizz" in x;
}

function isFizzBad2<T1, Fizz extends T1 /* prevents TheUnfizz, et. al, */>(x: T1): x is Fizz {
    return "fizz" in x;
    // Type 'T1' is not assignable to type 'object'.ts(2322)
}

/**
 * This at least compiles.
 */
function isFizz<T1, Fizz extends T1 /* prevents TheUnfizz, et. al, */>(x: T1): x is Fizz {
    return typeof x === "object" && "fizz" in (x as object);
}


function tryCast<T, U extends T>(value: T, predicate: (t:T) => t is U) {
    if (predicate(value)){
        return value; // shows: (parameter) value: U extends T
    }
    return undefined;
}

/*
* isFizz and tryCast have no errors but give incorrect results for x, y below.
*/
const x: (Buzz & Fizz) | undefined = tryCast(buzz, isFizz); // Error
// Type 'Buzz | undefined' is not assignable to type '(Buzz & Fizz) | undefined'.
//   Type 'Buzz' is not assignable to type 'Buzz & Fizz'.
//     Property 'fizz' is missing in type 'Buzz' but required in type 'Fizz'.ts(2322)

const y = tryCast(buzz, isFizz); // Buzz | undefined
const z = tryCast(unfizz, isFizz); // No error, but should be error

Example 2: (A workaround that works - not too hard but not immediately obvious either)
// Using this workaround, we can avoid error and get correct error free results for x7, y7, z below.

function isFizz<T extends Fizz|Buzz>(x: T): x is T & Fizz {  // * note 1
    return 'fizz' in x;
}

function tryCast3<T, R extends T>(value: T, predicate: (t:T) => t is R) { // * note 2
    if (predicate(value)){
        return value; // shows: (parameter) value: R extends T (although not obvious, it works)
    }
    return undefined;
}

const x7: (Fizz & Buzz) | undefined  = tryCast3(buzz, isFizz); // No error
const y7 = tryCast3(buzz, isFizz); // No error

const z = tryCast3(unfizz, isFizz); // Correctly error
//                        ~~~~~~
// Argument of type '<T1 extends Buzz | Fizz>(x: T1) => x is Fizz' is not assignable to parameter of type '(t: TheUnfizz) => t is Fizz'.
//   Types of parameters 'x' and 't' are incompatible.
//     Type 'TheUnfizz' is not assignable to type 'Buzz | Fizz'.

Playground

Example 3: (This would work if the T1 extends (the "is" target)) requirement is dropped.

declare function isFizz<T1 extends Fizz|Buzz>(x: T1): x is Fizz { // no error when `T1 extends (the "is" target)` rule is dropped
    return "fizz" in x;
}

function tryCast<T, U>(value: T, predicate: (t:T) => t is U) { // no error when `T1 extends (the "is" target)` rule is dropped
    if (predicate(value)){
        return value; // shows: (parameter) value: T & U - obvious and correct
    }
    return undefined;
}
const x: (Buzz & Fizz) | undefined = tryCast(buzz, isFizz); // No error
const y = tryCast(buzz, isFizz); // No error
const z = tryCast(unfizz, isFizz); // Correctly error (*note 3)
//                        ~~~~~~
// Argument of type '<T1 extends Buzz | Fizz>(x: T1) => x is Fizz' is not assignable to parameter of type '(t: TheUnfizz) => t is Fizz'.
//   Types of parameters 'x' and 't' are incompatible.
//     Type 'TheUnfizz' is not assignable to type 'Buzz | Fizz'.

Notice that in the line marked with "note 3", the error from passing TheUnfizz type is detected correctly even without the T1 extends (the "is" target) requirement.

Discussion:

Example 1 shows a failing naive-user attempt to use a type predicate function.
Note that the obvious user required constraint conflicts with the T1 extends (rhs of "is" target) rule for type predicates.

Example 2 is the workaround. Also see:. But arguably it is more complex than necessary because:

  • Setting the intersection type Fizz & T on the rhs of is (line with "note 1") is not immediately obvious.
  • Using the substitute variable R, and knowing that it will be successfully inferred to Fizz & T (line with "note 2") is not immediately obvious.
  • There is probably some computational cost associated with inferring type R. Is it worth it?

Example 3 is the suggested behavior, and it is more simple that either of Examples 1 or 2.

  • No workaround required.
  • That substitute R variable requiring inference of its value in example 3 is not present, so maybe less computation.
Counter argument

"Some constraint has to be there to prevent the user from passing arbitrary input, so what will stop the user from using no constraints?"

As least in in this example (1 and 3), the user needs to add their case specific constraint in order to write the test function implementation without incurring errors. And that constraint is enough to detect the error in Example 3 const z = (line with note 3).

Example 3 code was actually tested by commenting out this one simple line:

if (typePredicate.type) {
    const leadingError = () => chainDiagnosticMessages(/*details*/ undefined, Diagnostics.A_type_predicate_s_type_must_be_assignable_to_its_parameter_s_type);
    // checkTypeAssignableTo(typePredicate.type, getTypeOfSymbol(signature.parameters[typePredicate.parameterIndex]), node.type, /*headMessage*/ undefined, leadingError);

i.e., removing the rule entirely, rather than just relaxing it a little. (That was simplest to test).

There were only a couple of errors in runtests (typeGuardFunctionErrors, parseInvalidNullableTypes) , but those were basic tests to ensure the rule was being enforced, and nothing to show that the current rule is really a user-helpful rule.

So there appears to be no tests in the runtests suite that support the counter argument. Although that doesn't prove there couldn't be any.

Example 2 (workaround) also still continues to work with the rule removed - so it appears to be back compatible.

💻 Use Cases

As discussed above.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reproducing Example 3 in the linked Playground and inspect the type-predicate check shown in the issue. Compare the existing runtests cases typeGuardFunctionErrors and parseInvalidNullableTypes; done means deciding the compatibility behavior, updating affected tests, and adding coverage for the demonstrated generic predicate cases.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
compilers
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.