The Wayback Machine - https://web.archive.org/web/20221127014651/https://github.com/microsoft/TypeScript/issues/12400
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

Treating undefined parameters as optional #12400

Open
unional opened this issue Nov 21, 2016 · 38 comments
Open

Treating undefined parameters as optional #12400

unional opened this issue Nov 21, 2016 · 38 comments
Labels
Committed The team has roadmapped this issue Suggestion An idea for TypeScript
Milestone

Comments

@unional
Copy link
Contributor

unional commented Nov 21, 2016

tsc: 2.2.0-dev.20161116

Discover this behavior from redux-utilities/flux-standard-action@78a9065

This is the original code:

export interface FSA<Payload, Meta> {
  ...
  payload?: Payload;
  ...
}

However, this does not work well with type guard:

function isSomeAction(action: any): action is FSA<{ message: string }, void> {
  return true;
}

let action = {...};
if (isSomeAction(action)) {
  // `action.payload` may be undefined.
  console.log(action.payload.message);
}

Since generic type can be anything, including undefined, I'm considering to remove the optional designation:

export interface FSA<Payload, Meta> {
  ...
  payload: Payload;
  ...
}

// now type guard works
let action = {...};
if (isSomeAction(action)) {
  console.log(action.payload.message);
}

However, now the creation code fail:

function createSomeAction(): FSA<undefined, undefined> {
  // error: `payload` and `meta` are required
  return { type: 'SOME_ACTION' };
}

The workaround is to depends on infer type:

function createSomeAction() {
  return { type: 'SOME_ACTION' };
}

or specify it:

function createSomeAction(): FSA<undefined, undefined> {
  return { 
    type: 'SOME_ACTION',
    payload: undefined,
    meta: undefined
  };
}

Is there a better solution? 🌷

@unional
Copy link
Contributor Author

unional commented Nov 21, 2016

I understand that this is mixing two different contexts: Type of the value and type of the reference. But it would be nice to have a better solution to describe this situation?

@unional
Copy link
Contributor Author

unional commented Nov 21, 2016

As a counter argument, it does make sense that the property should be not optional redux-utilities/flux-standard-action#45 (comment)

@unional
Copy link
Contributor Author

unional commented Jan 26, 2017

A simpler example:

function createCounter<T>(x: number) {
  return (t: T) => {
    return x + 1
  }
}

const count = createCounter<undefined>(1)
count() // error, have to do `count(undefined)`

unional referenced this issue in redux-utilities/flux-standard-action Jan 30, 2017
* Add type guard, generic, and test

* Improve generic naming

* Add ErrorFSA alias

* Write test correctly to avoid confusion

* Add `someField` validation

* Add somefield test

* Add semi-colon, move tsc to posttest

* Add 3 more semi-colon

* Remove optional designation.

Since `payload` and `meta` is generic, user can specify `void` or `undefined` to indicate it does not present.

Removing the optional designation improves type guard.

see `isCustomAction4()` for example

* Fix more linting issue

* Adding eslint for typescript. Update yarn.lock

* Change typescript-eslint-parser to devDeps

* Move eslint-plugin-typescirpt to devDeps also.
@unional
Copy link
Contributor Author

unional commented Feb 3, 2017

@RyanCavanaugh @mhegazy do you have any feedback on this? 🌷

Can this be covered in default generic type when specifying the default type is undefined?

@mhegazy
Copy link
Contributor

mhegazy commented Feb 3, 2017

There is nothing specific to generics here. for example:

declare function f(a: string | undefined): void;

f(); // Not allowed
f(undefined); // OK

@mhegazy mhegazy changed the title Accepting undefined for generic as optional Treeting undefined parameters as optional Feb 3, 2017
@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Feb 3, 2017
@unional unional changed the title Treeting undefined parameters as optional Treating undefined parameters as optional Feb 12, 2017
@unional
Copy link
Contributor Author

unional commented Apr 22, 2017

With generic defaults (#13487) landed, more people will encounter this issue. Should this be fixed?

i.e.:

export interface FSA<Payload = never> {
  payload: Payload;
}

export class SomeFSA implements FSA {
  // error. `payload` is missing
}

@deregtd
Copy link

deregtd commented Nov 8, 2017

Adding a little more fuel to the fire. We have our own Promise library (Microsoft/SyncTasks on GitHub) and are running across bugs that devs are introducing now that we've switched our codebase to strict null-compliant.

In a perfect world, we would like:

let a: Promise<number>;
a.resolve(); // not allowed
a.resolve(4); // ok

let b: Promise<void>;
b.resolve(); // ok
b.resolve(4); // not ok

let c: Promise<number|undefined>;
c.resolve() // not ok
c.resolve(4) // ok
c.resolve(undefined) // ok

But there isn't currently any SFINAE-style way to select valid methods based on T types. If T is void, then you shouldn't be passing anything to resolve, and the function will just get an undefined parameter passed.

Right now, the function signature is "resolve(param?: T)", but that lets you do:

SyncTasks.Resolved() -- which then has a resolved synctask with an undefined value, which will then explode anything downstream that doesn't allow an undefined value.

We're contemplating, for now, changing synctasks to require always taking a parameter, and just adding some void helpers to make the code less annoyingly verbose, but it's ugly compared to what we could do with method signature selectors.

@deregtd
Copy link

deregtd commented Nov 9, 2017

To be clear, I don't actually want fully SFINAE-able method selectors -- this is JS after all, you can't do that. But I think the real answer is that if you have a function parameter whose type is void, then it should auto-change the signature for that parameter to optional, and error if you pass anything other than undefined to it, if you DO pass a value.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Nov 28, 2017

Approved. This will probably be a hairy change so we'll take a first stab at it unless someone wants to jump in. We don't think there will be breaking changes but we'll need to investigate

@phaux
Copy link

phaux commented Apr 25, 2020

@lonewarrior556 Okaaaayyy... Thanks. But now I'm only even more confused about the differences between void and undefined 😅 I thought it only makes a difference as a return type.

@lonewarrior556
Copy link

lonewarrior556 commented Apr 25, 2020

Yea... Anyone know what version void got released with?
I Want to read the release notes to see if that's intentional..

in the mean time..

const foo1 = (a: string | void) => { }
const foo2 = <T>(a: T) => { }
const foo3 = <T>(a: T | void) => { }
const foo4 = <T>(...a: [T]) => { }
const foo5 = <T>(...a: [T | void]) => { }
const foo6 = <T extends any[]>(...a: T) => { }

foo1() // work
foo2<void>() // nope
foo2<[void]>() // nope
foo3<string>() // works
foo4<string>() // nope
foo4<void>() // nope
foo5<string>() //works

foo6<[string | void]>() // works!

playground

@lonewarrior556
Copy link

lonewarrior556 commented Apr 25, 2020

Anyway this can be used (abused?) to allow for making generics where the arg is optional if the object contains no required properties...

type AllArgsOptional<T> = Partial<T> extends T ? void : T

type Options = {
  a: number
}

type PartialOptions = Partial<Options>


const foo1 = (a: Options | AllArgsOptional<Options>) => { }
const foo2 = (a: PartialOptions | AllArgsOptional<PartialOptions>) => { }
const foo3 = <T>(a: T | AllArgsOptional<T>) => { }
const foo4 = <T extends any[]>(...a: T) => { }


foo1() // a required
foo1({ a: 1 }) //ok

foo2() //ok
foo2({a: 1}) //ok

foo3<PartialOptions>() //Dang it!
foo3<PartialOptions>({a: 1}) //ok

foo4<[PartialOptions | AllArgsOptional<PartialOptions>]>() //ok
foo4<[PartialOptions | AllArgsOptional<PartialOptions>]>({ a: 1 }) //ok

type MakeOptionalIfOptional<T> = [T | AllArgsOptional<T>]

foo4<MakeOptionalIfOptional<PartialOptions>>() // tada!
foo4<MakeOptionalIfOptional<Options>>() // still required :)

playground

@georgekrax
Copy link

georgekrax commented Dec 8, 2020

Any update on this issue as of TypeScript 4.1.2?

@DScheglov
Copy link

DScheglov commented Jan 12, 2021

One more solution for making undefined fields optional:

type KeysOfType<T, SelectedType> = {
  [key in keyof T]: SelectedType extends T[key] ? key : never
}[keyof T];

type Optional<T> = Partial<Pick<T, KeysOfType<T, undefined>>>;

type Required<T> = Omit<T, KeysOfType<T, undefined>>;

export type OptionalUndefined<T> = Optional<T> & Required<T>;

Playground

@georgekrax
Copy link

georgekrax commented Jan 12, 2021

Thank you!

@kylemh
Copy link

kylemh commented Jan 28, 2021

Mental note for myself and maybe others:

When this is available and working with inferred arguments we will be able to typecast return types depending on inferred generics 🥳

demo in TS playground

@PanopticaRising
Copy link

PanopticaRising commented Mar 5, 2021

This would be really helpful to make non-breaking changes to APIs when introducing generics.

Edit: Actually, I realized (after re-reading some of the Playgrounds above) that what I wanted was to use void instead of undefined, which worked in my simple case.

@SephReed
Copy link

SephReed commented May 14, 2021

I'm dealing with two classes, one where the arg for every function is required, and one where it's optional. I'd like to be able to not copy paste them, they are otherwise the same.

class Base<ARG> {
  public foo(arg: ARG){}
  public bar(arg: ARG){}
  public qux(arg: ARG){}
}

class AllRequired extends Base<string> { }
class AllOptional extends Base<string | void> { }. // <--- this doesn't make the arg optional

I can see why specifying undefined is different from an optional arg... kind of.

But why doesn't void equate to optional? They seem to be saying the exact same thing: nothing was put there.


For my case, I found a way around:

class Base<ARG extends Array> {
  public foo(...arg: ARG){}
  public bar(...arg: ARG){}
  public qux(...arg: ARG){}
}

class AllRequired extends Base<[string]> { }
class AllOptional extends Base<[string] | []> { }

const req = new AllRequired();
req.foo(); // without an arg, this will throw an error

const opt = new AllOptional();
req.opt(); // this is fine though

@DScheglov
Copy link

DScheglov commented May 14, 2021

Hey, @SephReed

your workaround is a correct approach. But with minor update:

class Base<A extends any[]> {
  public foo(...args: A){}
  public bar(...args: A){}
  public qux(...args: A){}
}

@ericwooley
Copy link

ericwooley commented May 16, 2021

There are lots of workarounds here, Maybe all that's really needed is some good documentation or utility types?

@jacob-haller-roby
Copy link

jacob-haller-roby commented Jul 1, 2021

Trivial change, but my preferred method after playing around with the suggestions in this thread:

class Base<T = any> {
  public foo(...arg: T[]){}
  public bar(...arg: T[]){}
  public qux(...arg: T[]){}
}

class AllRequired extends Base<string> { }
class AllOptional extends Base { }

const req = new AllRequired();
req.foo(); // without an arg, this will throw an error

const opt = new AllOptional();
req.opt(); // this is fine though

@freakzlike
Copy link

freakzlike commented Aug 2, 2021

I tried to use void with conditional types, but i still need to pass undefined as parameter.

interface State {
  counter: number
}

interface Mutations {
  reset: (state: State) => void
  add: (state: State, value: number) => void
}

interface ActionContext {
  commit<K extends keyof Mutations>(
    key: K,
    payload: Parameters<Mutations[K]>[1] extends undefined ? void : Parameters<Mutations[K]>[1]
  ): void
}

const action = ({ commit }: ActionContext) => {
  commit('reset') // Expected 2 arguments, but got 1.
  commit('reset', undefined) // works
  commit('add', 1) // works
}

@DScheglov
Copy link

DScheglov commented Aug 2, 2021

@freakzlike ,
for parameters it works in a little bit another way:

interface State {
  counter: number
}

interface Mutations {
  reset: (state: State) => void
  add: (state: State, value: number) => void
}

type Payload<M> = M extends (state: any, ...payload: infer P) => void ? P : never;

type ActionContext = {
    commit: <Key extends keyof Mutations>(key: Key, ...payload: Payload<Mutations[Key]>) => void;
}

const action = ({ commit }: ActionContext) => {
  commit('reset') // works
  commit('reset', undefined) // Expected 1 arguments, but got 2.
  commit('add', 1) // works
}

See in Playground

Also I suggest to make ActionContext generic because it is the same for any possible Mutations:

type ActionContext<Ms extends {}> = {
    commit: <Key extends keyof Ms>(key: Key, ...payload: Payload<Ms[Key]>) => void;
}

const action = ({ commit }: ActionContext<Mutations>) => {
  commit('reset') // works
  commit('reset', undefined) // Expected 1 arguments, but got 2.
  commit('add', 1) // works
}

@Paduado
Copy link

Paduado commented Nov 27, 2021

A simpler example:

function createCounter<T>(x: number) {
  return (t: T) => {
    return x + 1
  }
}

const count = createCounter<undefined>(1)
count() // error, have to do `count(undefined)`

Solved it like this:

function createCounter<T>(x: number) {
  return (...args: undefined extends T ? [] : [T]) => {
    return x + 1;
  };
}

const count = createCounter<undefined>(1);
count();

@data-enabler
Copy link

data-enabler commented Oct 11, 2022

I'd also like to add that being able to tell Typescript to treat properties that can be undefined as optional would be useful for interop with Closure Compiler annotations, since their type system makes no distinction between the two and doesn't support any optional property syntax.

@unional
Copy link
Contributor Author

unional commented Oct 12, 2022

After 6 years since I raise this issue, I realized that what I really want is the other way around. Instead of "treating undefined as optional", I want it to be more expressive to say when the type is optional.

i.e. something like:

type FSA<Payload, Meta> = Equal<Payload, void> 
  ? { ... }
  : { payload: Payload }

There is a semantic difference between undefined and optional.
I don't want this suggestion makes them even more muddy.

@data-enabler
Copy link

data-enabler commented Oct 12, 2022

There is a semantic difference between undefined and optional. I don't want this suggestion makes them even more muddy.

Unfortunately, every other issue asking for the behavior I'm describing has been marked as a duplicate of this one. Perhaps it would make sense to open a separate issue?

@unional
Copy link
Contributor Author

unional commented Oct 12, 2022

Unfortunately, every other issue asking for the behavior I'm describing has been marked as a duplicate of this one. Perhaps it would make sense to open a separate issue?

It is still the same issue. What I'm suggesting is that the solution should not "loosen" the TypeScript semantics. There should be an alternative to describe precisely what it is.

There are workarounds for some cases such as the one suggested here: #12400 (comment)

But it would be great if TS can provide a way to describe it out of the box without causing more confusion.

Additional food of thoughts: what is void and what is never? Does void maps to unit in Haskell and never maps to Void in Haskell?

Is there a way to use them in generics to precisely describe what we want here?

For those who are interested in type theory, I would recommend checking out Category Theory for Programmers by Bartosz Milewski

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Committed The team has roadmapped this issue Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests