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

Void parameter still required when type extends generic #29131

Open
dested opened this issue Dec 23, 2018 · 12 comments
Open

Void parameter still required when type extends generic #29131

dested opened this issue Dec 23, 2018 · 12 comments
Labels
Bug A bug in TypeScript
Milestone

Comments

@dested
Copy link

dested commented Dec 23, 2018

TypeScript Version: 3.2.2

Search Terms:
void parameter type extends generic
Code

function works1(n: number, b: void) { }

works1(12);

function works2(n: number, b: 1 extends 1 ? void : number) { }

works2(12);

function fails<T>(n: number, b: T extends 1 ? void : number) { }

fails<2>(12, 2);  // works, requires both params
fails<1>(12);  // fails, requires 2 parameters even though second param is void

Expected behavior:
That I can ignore the second parameter since its void

Actual behavior:
Ts tells me I am missing a parameter

https://www.typescriptlang.org/play/index.html#src=function%20works1(n%3A%20number%2C%20b%3A%20void)%20%7B%20%7D%0D%0A%0D%0Aworks1(12)%3B%0D%0A%0D%0Afunction%20works2(n%3A%20number%2C%20b%3A%201%20extends%201%3F%20void%20%3A%20number)%20%7B%20%7D%0D%0A%0D%0Aworks2(12)%3B%0D%0A%0D%0Afunction%20fails%3CT%3E(n%3A%20number%2C%20b%3A%20T%20extends%201%3Fvoid%3Anumber)%20%7B%20%7D%0D%0A%0D%0Afails%3C2%3E(12%2C2)%3B%20%20%2F%2Fworks%2C%20requires%20both%20params%0D%0Afails%3C1%3E(12)%3B%20%20%2F%2F%20requires%202%20parameters%20even%20though%20second%20param%20is%20void%0D%0A

@jack-williams
Copy link
Collaborator

This is a design limitation in the way the checks are implemented.

Checking for void parameters that can be elided is done prior to generic instantiation, which means that instantiations that produce void parameters are ignored, as is the case in fails.

This choice was made to avoid having to check every generic signature twice, and there were some issues with this breaking existing overload selections.

I would be interested in understanding real-word examples of this behaviour. Currently I'm not sure whether this behaviour can be implemented using existing methods (overloads), or whether we really need to extend the void checks.

@dested
Copy link
Author

dested commented Dec 23, 2018

It was a poor mans method overloading, it just seemed very strange to me that it would not work even though the compiled method signatures were exactly the same. Now that I know when the void eliding is done I was able to solve it using the following:

function works3<T extends 1>(n: number, b: void)
function works3<T>(n: number, b: number)
function works3<T>(n: number, b: number | void) { }

works3<2>(12,2);
works3<1>(12);

Thanks for your help in understanding the situation! Feel free to close this if you see no reason to fix this as (at least in this case) it can be avoided.

@jack-williams
Copy link
Collaborator

Thanks for your help in understanding the situation! Feel free to close this if you see no reason to fix this as (at least in this case) it can be avoided.

It's not for me to open/close issues, that's for the team to decide. I would suggest leaving this open as the canonical thread that tracks use-cases that need the generic example to work too. If enough people find it useful then I'd be happy to add it.

@weswigham weswigham added the Bug A bug in TypeScript label Dec 25, 2018
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Mar 14, 2019
@lifaon74
Copy link

@jack-williams
As you asked for an example:

class MyIterator<T> implements Iterator<T> {
  protected _value: T;
  constructor(value: T) {
    this._value = value;
  }
  
  next(): IteratorResult<T> {
    return {
      done: false,
      value: this._value
    };
  }
  
  foo(value: T): void {
  }
}

const it: MyIterator<void> = new MyIterator<void>(); // error !
it.next();
it.next();
/// ...
it.foo(); // valid !

It's simply an iterator that send everytime the same value provided in its constructor.

What's the strangest: the 'foo' is perfectly valid but the 'constructor' errors.

@jack-williams
Copy link
Collaborator

Thanks for the example!

What's the strangest: the 'foo' is perfectly valid but the 'constructor' errors.

The constructor is generic at the call-site; foo is concrete at the call-site because T is known to be void.

As a workaround you can do:

function init(x: void) {
    return new MyIterator(x);
}

const it: MyIterator<void> = init();
it.next();
it.next();
/// ...
it.foo();

@parzh
Copy link

parzh commented Dec 13, 2019

Variable number of function parameters can still be achieved using tuple types (although, parameter identifiers in IntelliSense will be lost unfortunately):

declare function fn<Value extends 1 | 2>(...args: Value extends 1 ? [number] : [number, string]): void;

fn<1>(42);
fn<1>(42, "hello world"); // Error: expects 1 argument

fn<2>(42); // Error: expects 2 arguments
fn<2>(42, "hello world");

Link to playground.

@dragomirtitian
Copy link
Contributor

@parzh You can get the parameter names back (mostly) if you don't mind writing a bit more:

declare function fn<Value extends 1 | 2>(...args: Value extends 1 ?  Parameters<(x: number) =>void> : Parameters<(y:number, name: string) => void>): void;

fn<1>(42);
fn<1>(42, "hello world"); // Error: expects 1 argument

fn<2>(42); // Error: expects 2 arguments
fn<2>(42, "hello world");

Playground Link

@neopostmodern
Copy link

I have a case where I'm implicitly overloading a function, but can't do explicit overloading due to maintaining closure.
I'm trying to generically create Redux actions. The load action can have a parameter of a specifiable type, but should possibly have no parameters.

type DataStateActions<T, U> = {
  load: (parameters: U) => DataActionLoad<T, U>
  // omitted
}

const generateDataStateActions = <T, U = void>(
  type: string,
): DataStateActions<T, U> => ({
  load: (parameters): DataActionLoad<T, U> => ({
    type,
    payload: { intent: DataActionTypes.LOAD, parameters },
  }),
  // omitted
})

But then it fails at

type AiImplementationManagerContainerFunctionProps = {
  fetchCaseSetList: () => void
}
const mapDispatchToProps: AiImplementationManagerContainerFunctionProps = {
  fetchCaseSetList: caseSetListDataActions.load,
}

with

Error:(120, 3) TS2322: Type '(parameters: void) => DataActionLoad<CaseSetInfo[], void>' is not assignable to type '() => void'.

(Notice that the differing return types are completely fine – it's about the parameters: void part)

Playground

@dragomirtitian
Copy link
Contributor

@neopostmodern I think the void parameters is not required really be longs in the not deprecated but don't use category of TS features (but keep in mind I an just a random guy on the internet with an opinion 😋) . I think today, we can do much better with tuples in rest parameters (which did not exist when the void trick was implemented:

export enum DataActionTypes {
    LOAD = 'LOAD',
    // omitted
}

export type DataActionLoad<T, U  extends [undefined?] | [any] > = {
  type: string
  payload: { intent: DataActionTypes.LOAD; parameters: U[0] }
}

type DataStateActions<T, U  extends [undefined?] | [any]> = {
  load: (...parameters: U) => DataActionLoad<T, U>
  // omitted
}

const generateDataStateActions = <T, U  extends [undefined?] | [any]>(
  type: string,
): DataStateActions<T, U> => ({
  load: (...parameters: U): DataActionLoad<T, U> => ({
    type,
    payload: { intent: DataActionTypes.LOAD, parameters: parameters[0] },
  }),
  // omitted
})

const caseSetListDataActions = generateDataStateActions<number, []>('TYPE_NAME')

type AiImplementationManagerContainerFunctionProps = {
  fetchCaseSetList: () => void
}
const mapDispatchToProps: AiImplementationManagerContainerFunctionProps = {
  fetchCaseSetList: caseSetListDataActions.load,
}

Playground Link

I intentionally made this accept one parameter to keep the same API as your original, but with this approach, you could accept a variable number of parameter.

@neopostmodern
Copy link

neopostmodern commented Feb 18, 2020

I appreciate the flexibility this offers, but IMHO using void in this scenario should still work (and even seems preferable to me), for several reasons:

  • I feel like it's more explicit (void is more concise in my view than [] to indicate absence)
  • It avoids bloat:
    type DataStateActions<DataType, ParameterType = void, MetadataType = void> = {
      load: (
        ...params: MetadataType extends void
          ? ParameterType extends void
            ? []
            : Parameters<(parameters: ParameterType) => void>
          : Parameters<(parameters: ParameterType, metadata: MetadataType) => void>
      ) => DataActionLoad<ParameterType, MetadataType>
      // omitted
    }
    
    ...not nice.
  • The above (when trying to re-insert parameter names) breaks when the type passed in, for example for ParameterType, is itself MyFancyType | void to indicate omittability on a different level.
    (This might work with your approach of passing in arrays of types, but I feel it's less readable. The logic does not accept "any number of parameters" but two very specific, yet omittable, ones.)
  • Last but not least it confuses the type hints in WebStorm (and possibly other tooling): Screenshot 2020-02-18 at 09 55 56
    I guess it thinks the parameters are a mixed-array (this will probably be fixed at some point though, I'd guess)
  • Edit Plus, void as a parameter type has some tradition in C

TL;DR Yes, alternatives exist – but void parameters should work too.

@ATheCoder
Copy link

JSDocs don't work either using tuple types.

@PietroCarrara
Copy link

This feature would really help the following use-case:

Our react codebase has some route objects, of type Route<UrlData, ComponentData>, which describes the data needed do navigate to that route. It is very nice to be able to type-safely navigate around the web-app like this:

navigate(routes.createItem, { urlProps: { ... }, componentProps: { ... } }) // You can't illegally navigate now!

Some pages are very simple, and they don't need any data. In this case, the type of the second argument of navigate will expand to { urlProps?: undefined, componentProps?: undefined }. This means you navigate to them via navigate(routes.welcomePage, {}).

What we wish we could do was transform that type into void so that you can't pass any data into the page, like this:

type MakeForbiddenArgument<T> = T[Exclude<keyof T, undefined>] extends undefined
  ? void
  : T;

navigate(routes.createItem); // This should work!

So, yeah, another interesting use-case for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
None yet
Development

No branches or pull requests

9 participants