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

Link types in union with type in union of type guard #48998

Closed
5 tasks done
Fryuni opened this issue May 6, 2022 · 4 comments
Closed
5 tasks done

Link types in union with type in union of type guard #48998

Fryuni opened this issue May 6, 2022 · 4 comments
Labels
Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds

Comments

@Fryuni
Copy link

Fryuni commented May 6, 2022

Suggestion

πŸ” Search Terms

union of type guards
type guard union
list of type guards

βœ… Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code (but would allow removing workarounds)
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

Link the type of a type guard within a union with the type of the element, so they can be used with each other, even though narrowing is not possible.

πŸ“ƒ Motivating Example

type Foo = {foo: string};
type Bar = {bar: number};

type WorkTypes = Foo | Bar;

type GuardedWorker<All, T extends All> = {
  guard: (value: All) => value is T,
  process: (value: T) => void
};

type GuardedWorkers = GuardedWorker<WorkTypes, Foo> | GuardedWorker<WorkTypes, Bar>;

function doWork<T extends WorkTypes>(workers: GuardedWorkers[], value: T) {
  for (const worker of workers) {
    if (worker.guard(value)) {
      // worker is still a union and value is still T since this can't narrow the type
      // but TS knows thy are of the same Work type, whatever it is. So this works.
      worker.process(value);
    }
  }
}

πŸ’» Use Cases

The example above is a simplification of a real word example where this would be used.

Currently, we need to do a casting like this the example to work:

function doWork<T extends WorkTypes>(workers: GuardedWorkers[], value: T) {
  for (const worker of workers) {
    if (worker.guard(value)) {
      const narrowedWorker = worker as unknown as GuardedWorker<WorkTypes, T>;

      narrowedWorker.process(value);
    }
  }
}

In our real use case, we don't need the unknown because the type containing the guard is generic a string literal, and that doesn't cause a conflict. This is the real world use case that prompted this feature request:

export function resolveNode<T extends NodeTypeNames>(id: IdOfNode<T>): NodeSource<T> {
    for (const [prefix, resolution] of Object.entries(nodeResolutionMap)) {
        if (resolution.idTest(id)) {
            // TS don't narrow the type guard itself, so we need to cast the resolution.
            const typedResolution = resolution as NodeResolution<T>;

            // Do a bunch of things to validate the ID

            const node = resolution.nodeResolver(id)

            // Do a bunch of things to validate the Node

            return node;
        }
    }

    throw new Error('Unsupported ID type.');
}
@RyanCavanaugh
Copy link
Member

There's not really a sound way of doing this without creating an entirely new kind of narrowing. From TypeScript's perspective, there's not a difference between the provided code (which is OK by construction) and this code, which is broken:

function doWork<T extends WorkTypes>(workers: GuardedWorkers[], value: T) {
  const worker1 = workers[0];
  const worker2 = workers[1];
  if (worker1.guard(value)) {
    worker2.process(value); // Unsound call
  }
}
declare const gwFoo: GuardedWorker<WorkTypes, Foo>;
declare const gwBar: GuardedWorker<WorkTypes, Bar>;
doWork([gwFoo, gwBar], { foo: "" }); // Fails

So it's not enough / not correct to narrow T, it has to be a narrowing in the context of a particular value. We haven't needed anything like that yet, and even if we had it, it wouldn't really be usable on mutable bindings like worker anyway.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds labels May 6, 2022
@jcalz
Copy link
Contributor

jcalz commented May 7, 2022

Seems like another instance of #30581. If so, then the approach described in #47109 works here also, with the introduction of a helper mapping type, like this:

interface WorkMapper {
    foo: Foo;
    bar: Bar;
}

type GuardedWorkers<K extends keyof WorkMapper> =
    { [P in K]: GuardedWorker<WorkTypes, WorkMapper[P]> }[K];

function doWork<K extends keyof WorkMapper>(workers: GuardedWorkers<K>[], value: WorkTypes) {
    for (const worker of workers) {
        if (worker.guard(value)) {
            worker.process(value); // okay
        }
    }
}

declare const gwFoo: GuardedWorker<WorkTypes, Foo>;
declare const gwBar: GuardedWorker<WorkTypes, Bar>;
doWork([gwFoo, gwBar], { foo: "" });

Playground link

It's also something that existential types would help ( #14466 ) and the various emulations of existential types also apply (with various runtime refactorings if you want type safety)

const someGuardedWorker = <A, T extends A>(worker: GuardedWorker<A, T>) =>
    (value: A) => { if (worker.guard(value)) worker.process(value) }

type SomeGuardedWorker = (value: WorkTypes) => void

function doWork(workers: SomeGuardedWorker[], value: WorkTypes) {
    for (const worker of workers) {
        worker(value)
    }
}

declare const gwFoo: GuardedWorker<WorkTypes, Foo>;
declare const gwBar: GuardedWorker<WorkTypes, Bar>;
doWork([someGuardedWorker(gwFoo), someGuardedWorker(gwBar)], { foo: "" })

Playground link to code

@Fryuni
Copy link
Author

Fryuni commented May 8, 2022

So if the workers themselves were to take the generic parameter and not the value it would work? I'll try that, thanks.

@typescript-bot
Copy link
Collaborator

This issue has been marked as "Too Complex" 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 Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds
Projects
None yet
Development

No branches or pull requests

4 participants